Browse Source

pass dnt to pip service and sanitize error messages

pull/852/head
Stephen Hess 8 years ago
parent
commit
a7867579de
  1. 58
      service/pointinpolygon.js
  2. 174
      test/unit/service/pointinpolygon.js

58
service/pointinpolygon.js

@ -2,14 +2,51 @@ const logger = require( 'pelias-logger' ).get( 'pointinpolygon' );
const request = require('request'); const request = require('request');
const _ = require('lodash'); const _ = require('lodash');
function sanitizeUrl(requestUrl) {
return requestUrl.replace(/^(.+)\/.+\/.+$/, (match, base) => {
return `${base}/[removed]/[removed]`;
});
}
function parseErrorMessage(requestUrl, body, do_not_track) {
if (do_not_track) {
return `${sanitizeUrl(requestUrl)} returned status 200 but with non-JSON response: ${body}`;
}
return `${requestUrl} returned status 200 but with non-JSON response: ${body}`;
}
function serviceErrorMessage(requestUrl, statusCode, body, do_not_track) {
if (do_not_track) {
return `${sanitizeUrl(requestUrl)} returned status ${statusCode}: ${body}`;
} else {
return `${requestUrl} returned status ${statusCode}: ${body}`;
}
}
module.exports = (url) => { module.exports = (url) => {
if (!_.isEmpty(url)) { if (!_.isEmpty(url)) {
logger.info(`using point-in-polygon service at ${url}`); logger.info(`using point-in-polygon service at ${url}`);
return function pointinpolygon( centroid, callback ) { return function pointinpolygon( centroid, do_not_track, callback ) {
const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`; const requestUrl = `${url}/${centroid.lon}/${centroid.lat}`;
request.get(requestUrl, (err, response, body) => { const options = {
method: 'GET',
url: requestUrl
};
if (do_not_track) {
options.headers = {
dnt: '1'
};
}
request(options, (err, response, body) => {
if (err) { if (err) {
logger.error(JSON.stringify(err)); logger.error(JSON.stringify(err));
callback(err); callback(err);
@ -20,13 +57,17 @@ module.exports = (url) => {
callback(err, parsed); callback(err, parsed);
} }
catch (err) { catch (err) {
logger.error(`${requestUrl}: could not parse response body: ${body}`); const parseMsg = parseErrorMessage(requestUrl, body, do_not_track);
callback(`${requestUrl} returned status 200 but with non-JSON response: ${body}`);
logger.error(parseMsg);
callback(parseMsg);
} }
} }
else { else {
logger.error(`${requestUrl} returned status ${response.statusCode}: ${body}`); const errorMsg = serviceErrorMessage(requestUrl, response.statusCode, body, do_not_track);
callback(`${requestUrl} returned status ${response.statusCode}: ${body}`);
logger.error(errorMsg);
callback(errorMsg);
} }
}); });
@ -35,8 +76,9 @@ module.exports = (url) => {
} else { } else {
logger.warn('point-in-polygon service disabled'); logger.warn('point-in-polygon service disabled');
return (centroid, callback) => { return (centroid, do_not_track, callback) => {
callback(`point-in-polygon service disabled, unable to resolve ${JSON.stringify(centroid)}`); callback(`point-in-polygon service disabled, unable to resolve ` +
(do_not_track ? 'centroid' : JSON.stringify(centroid)));
}; };
} }

174
test/unit/service/pointinpolygon.js

@ -25,7 +25,7 @@ module.exports.tests.do_nothing_service = (test, common) => {
'pelias-logger': logger 'pelias-logger': logger
})(); })();
service({ lat: 12.121212, lon: 21.212121 }, (err) => { service({ lat: 12.121212, lon: 21.212121 }, false, (err) => {
t.deepEquals(logger.getWarnMessages(), [ t.deepEquals(logger.getWarnMessages(), [
'point-in-polygon service disabled' 'point-in-polygon service disabled'
]); ]);
@ -35,6 +35,23 @@ module.exports.tests.do_nothing_service = (test, common) => {
}); });
test('service unavailable message should not output centroid when do_not_track is true', (t) => {
const logger = require('pelias-mock-logger')();
const service = proxyquire('../../../service/pointinpolygon', {
'pelias-logger': logger
})();
service({ lat: 12.121212, lon: 21.212121 }, true, (err) => {
t.deepEquals(logger.getWarnMessages(), [
'point-in-polygon service disabled'
]);
t.equals(err, 'point-in-polygon service disabled, unable to resolve centroid');
t.end();
});
});
}; };
module.exports.tests.success = (test, common) => { module.exports.tests.success = (test, common) => {
@ -56,13 +73,81 @@ module.exports.tests.success = (test, common) => {
'pelias-logger': logger 'pelias-logger': logger
})(`http://localhost:${port}`); })(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => { service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
t.notOk(err);
t.deepEquals(results, { field: 'value' });
t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
t.notOk(logger.hasErrorMessages());
t.end();
server.close();
});
});
test('do_not_track=true should pass DNT header', (t) => {
const pipServer = require('express')();
pipServer.get('/:lon/:lat', (req, res, next) => {
t.ok(req.headers.hasOwnProperty('dnt'));
t.equals(req.params.lat, '12.121212');
t.equals(req.params.lon, '21.212121');
res.send('{ "field": "value" }');
});
const server = pipServer.listen();
const port = server.address().port;
const logger = require('pelias-mock-logger')();
const service = proxyquire('../../../service/pointinpolygon', {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
t.notOk(err);
t.deepEquals(results, { field: 'value' });
t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
t.notOk(logger.hasErrorMessages());
t.end();
server.close();
});
});
test('do_not_track=false should not pass DNT header', (t) => {
const pipServer = require('express')();
pipServer.get('/:lon/:lat', (req, res, next) => {
t.notOk(req.headers.hasOwnProperty('dnt'));
t.equals(req.params.lat, '12.121212');
t.equals(req.params.lon, '21.212121');
res.send('{ "field": "value" }');
});
const server = pipServer.listen();
const port = server.address().port;
const logger = require('pelias-mock-logger')();
const service = proxyquire('../../../service/pointinpolygon', {
'pelias-logger': logger
})(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
t.notOk(err); t.notOk(err);
t.deepEquals(results, { field: 'value' }); t.deepEquals(results, { field: 'value' });
t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`)); t.ok(logger.isInfoMessage(`using point-in-polygon service at http://localhost:${port}`));
t.notOk(logger.hasErrorMessages()); t.notOk(logger.hasErrorMessages());
t.end(); t.end();
server.close(); server.close();
@ -92,10 +177,46 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger 'pelias-logger': logger
})(`http://localhost:${port}`); })(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => { const expectedErrorMsg = `http://localhost:${port}/21.212121/12.121212 returned ` +
t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 200 but with non-JSON response: this is not JSON`); `status 200 but with non-JSON response: this is not JSON`;
service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
t.equals(err, expectedErrorMsg);
t.notOk(results); t.notOk(results);
t.ok(logger.isErrorMessage(`http://localhost:${port}/21.212121/12.121212: could not parse response body: this is not JSON`)); t.ok(logger.isErrorMessage(expectedErrorMsg));
t.end();
server.close();
});
});
test('server returning 200 & non-JSON body should log sanitized error and return no results when do_not_track', (t) => {
const pipServer = require('express')();
pipServer.get('/:lon/:lat', (req, res, next) => {
t.equals(req.params.lat, '12.121212');
t.equals(req.params.lon, '21.212121');
res.send('this is not JSON');
});
const server = pipServer.listen();
const port = server.address().port;
const logger = require('pelias-mock-logger')();
const service = proxyquire('../../../service/pointinpolygon', {
'pelias-logger': logger
})(`http://localhost:${port}`);
const expectedErrorMsg = `http://localhost:${port}/[removed]/[removed] returned ` +
`status 200 but with non-JSON response: this is not JSON`;
service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
t.equals(err, expectedErrorMsg);
t.notOk(results);
t.ok(logger.isErrorMessage(expectedErrorMsg));
t.end(); t.end();
server.close(); server.close();
@ -117,7 +238,7 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger 'pelias-logger': logger
})(`http://localhost:${port}`); })(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => { service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
t.equals(err.code, 'ECONNREFUSED'); t.equals(err.code, 'ECONNREFUSED');
t.notOk(results); t.notOk(results);
t.ok(logger.isErrorMessage(/ECONNREFUSED/), 'there should be a connection refused error message'); t.ok(logger.isErrorMessage(/ECONNREFUSED/), 'there should be a connection refused error message');
@ -144,10 +265,43 @@ module.exports.tests.failure = (test, common) => {
'pelias-logger': logger 'pelias-logger': logger
})(`http://localhost:${port}`); })(`http://localhost:${port}`);
service({ lat: 12.121212, lon: 21.212121}, (err, results) => { const expectedErrorMsg = `http://localhost:${port}/21.212121/12.121212 returned ` +
t.equals(err, `http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`); `status 400: a bad request was made`;
service({ lat: 12.121212, lon: 21.212121}, false, (err, results) => {
t.equals(err, expectedErrorMsg);
t.notOk(results);
t.ok(logger.isErrorMessage(expectedErrorMsg));
t.end();
server.close();
});
});
test('non-OK status should log sanitized error and return no results when do_not_track=true', (t) => {
const pipServer = require('express')();
pipServer.get('/:lat/:lon', (req, res, next) => {
res.status(400).send('a bad request was made');
});
const server = pipServer.listen();
const port = server.address().port;
const logger = require('pelias-mock-logger')();
const service = proxyquire('../../../service/pointinpolygon', {
'pelias-logger': logger
})(`http://localhost:${port}`);
const expectedErrorMsg = `http://localhost:${port}/[removed]/[removed] returned ` +
`status 400: a bad request was made`;
service({ lat: 12.121212, lon: 21.212121}, true, (err, results) => {
t.equals(err, expectedErrorMsg);
t.notOk(results); t.notOk(results);
t.ok(logger.isErrorMessage(`http://localhost:${port}/21.212121/12.121212 returned status 400: a bad request was made`)); t.ok(logger.isErrorMessage(expectedErrorMsg));
t.end(); t.end();
server.close(); server.close();

Loading…
Cancel
Save