From f9f6c8b700463ec9bd966328cb23da59cb140cf6 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 17 Sep 2015 11:06:46 +0200 Subject: [PATCH 1/2] Use debug instead of console --- lib/cartodb/controllers/base.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 7860e1ed..ce005055 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -2,6 +2,7 @@ var assert = require('assert'); var _ = require('underscore'); var step = require('step'); +var debug = require('debug')('windshaft:cartodb'); var LZMA = require('lzma').LZMA; var lzmaWorker = new LZMA(); @@ -171,7 +172,7 @@ BaseController.prototype.send = function(req, res, args) { // See http://github.com/CartoDB/Windshaft/issues/166 req.profiler.sendStats(); } catch (err) { - console.error("error sending profiling stats: " + err); + debug("error sending profiling stats: " + err); } } }; @@ -181,12 +182,7 @@ BaseController.prototype.sendError = function(req, res, err, label) { var statusCode = findStatusCode(err); - // use console.log for statusCode != 500 ? - if (statusCode >= 500) { - console.error('[%s ERROR] -- %d: %s', label, statusCode, err); - } else { - console.warn('[%s WARN] -- %d: %s', label, statusCode, err); - } + debug('[%s ERROR] -- %d: %s', label, statusCode, err); // If a callback was requested, force status to 200 if (req.query && req.query.callback) { From 9139feaa30d77638c60218a84eb90eddab7fad1b Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Thu, 17 Sep 2015 12:48:29 +0200 Subject: [PATCH 2/2] Move error message handling test to unit --- lib/cartodb/controllers/base.js | 24 ++++++++++++--------- test/acceptance/ported/regressions.js | 27 ------------------------ test/unit/cartodb/error_messages.test.js | 24 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 37 deletions(-) create mode 100644 test/unit/cartodb/error_messages.test.js diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index ce005055..ac7a2e6a 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -189,20 +189,24 @@ BaseController.prototype.sendError = function(req, res, err, label) { statusCode = 200; } - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - var message = (_.isString(err) ? err : err.message) || 'Unknown error'; - // Strip connection info, if any - message = message - // See https://github.com/CartoDB/Windshaft/issues/173 - .replace(/Connection string: '[^']*'\\n/, '') - // See https://travis-ci.org/CartoDB/Windshaft/jobs/20703062#L1644 - .replace(/is the server.*encountered/im, 'encountered'); - - var errorResponseBody = { errors: [message] }; + var errorResponseBody = { errors: [errorMessage(err)] }; this.send(req, res, [errorResponseBody, statusCode]); }; +function errorMessage(err) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + var message = (_.isString(err) ? err : err.message) || 'Unknown error'; + + // Strip connection info, if any + return message + // See https://github.com/CartoDB/Windshaft/issues/173 + .replace(/Connection string: '[^']*'\n\s/im, '') + // See https://travis-ci.org/CartoDB/Windshaft/jobs/20703062#L1644 + .replace(/is the server.*encountered/im, 'encountered'); +} +module.exports.errorMessage = errorMessage; + function findStatusCode(err) { var statusCode; if ( err.http_status ) { diff --git a/test/acceptance/ported/regressions.js b/test/acceptance/ported/regressions.js index 0d7cc4f1..fa6330af 100644 --- a/test/acceptance/ported/regressions.js +++ b/test/acceptance/ported/regressions.js @@ -135,31 +135,4 @@ describe('regressions', function() { testClient.createLayergroup(mapConfig, { server: server }, completed); } }); - - // See https://github.com/CartoDB/Windshaft/issues/173 - it.skip("#173 does not send db details in connection error response", function(done) { - - var mapConfig = testClient.defaultTableMapConfig('test_table'); - - var CustomOptions = _.clone(ServerOptions); - CustomOptions.grainstore = _.clone(CustomOptions.grainstore); - CustomOptions.grainstore.datasource = _.clone(CustomOptions.grainstore.datasource); - CustomOptions.grainstore.datasource.port = '666'; - - var options = { - statusCode: 400, - serverOptions: CustomOptions - }; - - testClient.createLayergroup(mapConfig, options, function(err, res, parsedBody) { - assert.ok(parsedBody.errors); - var msg = parsedBody.errors[0]; - assert.ok(msg.match(/connect/), msg); - assert.ok(!msg.match(/666/), msg); - - done(); - }); - - }); - }); diff --git a/test/unit/cartodb/error_messages.test.js b/test/unit/cartodb/error_messages.test.js new file mode 100644 index 00000000..52441db2 --- /dev/null +++ b/test/unit/cartodb/error_messages.test.js @@ -0,0 +1,24 @@ +require('../../support/test_helper'); + +var assert = require('assert'); + +var BaseController = require('../../../lib/cartodb/controllers/base'); + +describe('error messages clean up', function() { + + // See https://github.com/CartoDB/Windshaft/issues/173 + it("#173 does not send db details in connection error response", function() { + var inMessage = [ + "Postgis Plugin: Bad connection", + "Connection string: 'host=127.0.0.1 port=5432 dbname=test_windshaft_cartodb_user_1_db " + + "user=test_windshaft_cartodb_user_1 connect_timeout=4'", + " encountered during parsing of layer 'layer0' in Layer" + ].join('\n'); + + var outMessage = BaseController.errorMessage(inMessage); + + assert.ok(outMessage.match('connect'), outMessage); + assert.ok(!outMessage.match(/666/), outMessage); + }); + +});