From daeae5d95c77b7b879f02891046a78b115ccfee0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 21 Sep 2017 11:46:31 +0200 Subject: [PATCH] Implement error-middleware to handle errors at top level --- lib/cartodb/controllers/analyses.js | 5 +- lib/cartodb/controllers/base.js | 157 ++---------------- lib/cartodb/controllers/layergroup.js | 49 +++--- lib/cartodb/controllers/map.js | 26 +-- lib/cartodb/controllers/named_maps.js | 10 +- lib/cartodb/controllers/named_maps_admin.js | 25 +-- lib/cartodb/middleware/error-middleware.js | 175 ++++++++++++++++++++ lib/cartodb/server.js | 4 + test/unit/cartodb/base_controller.js | 8 +- test/unit/cartodb/error_messages.test.js | 4 +- test/unit/cartodb/ported/tile_stats.test.js | 7 +- 11 files changed, 264 insertions(+), 206 deletions(-) create mode 100644 lib/cartodb/middleware/error-middleware.js diff --git a/lib/cartodb/controllers/analyses.js b/lib/cartodb/controllers/analyses.js index 7760cc32..6068dc42 100644 --- a/lib/cartodb/controllers/analyses.js +++ b/lib/cartodb/controllers/analyses.js @@ -27,7 +27,7 @@ AnalysesController.prototype.sendResponse = function(req, res, resource) { this.send(req, res, resource, 200); }; -AnalysesController.prototype.catalog = function(req, res) { +AnalysesController.prototype.catalog = function (req, res, next) { var self = this; var username = req.context.user; @@ -80,7 +80,8 @@ AnalysesController.prototype.catalog = function(req, res) { err = new Error('Unauthorized'); err.http_status = 401; } - self.sendError(req, res, err); + + next(req, res, err); } else { self.sendResponse(req, res, { catalog: catalogWithTables }); } diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index f74702e9..6474eecc 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -109,6 +109,11 @@ BaseController.prototype.req2params = function(req, callback){ // bring all query values onto req.params object _.extend(req.params, req.query); + // FIXME: Temporary hack to share data between middlewares. Express overrides req.params to + // parse url params to an object and it's performed after matching path and controller. + req.locals = {}; + _.extend(req.locals, req.params); + req.profiler.done('req2params.setup'); step( @@ -144,6 +149,11 @@ BaseController.prototype.req2params = function(req, callback){ dbport: global.environment.postgres.port }); + + // FIXME: Temporary hack to share data between middlewares. Express overrides req.params to + // parse url params to an object and it's performed after matching path and controller. + _.defaults(req.locals, req.params); + req.profiler.done('req2params'); callback(null, req); } @@ -184,150 +194,3 @@ BaseController.prototype.send = function(req, res, body, status, headers) { } }; // jshint maxcomplexity:6 - -BaseController.prototype.sendError = function(req, res, err, label) { - var allErrors = Array.isArray(err) ? err : [err]; - - allErrors = populateTimeoutErrors(allErrors); - - label = label || 'UNKNOWN'; - err = allErrors[0] || new Error(label); - allErrors[0] = err; - - var statusCode = findStatusCode(err); - - if (err.message === 'Tile does not exist' && req.params.format === 'mvt') { - statusCode = 204; - } - - debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); - - // If a callback was requested, force status to 200 - if (req.query && req.query.callback) { - statusCode = 200; - } - - var errorResponseBody = { - errors: allErrors.map(errorMessage), - errors_with_context: allErrors.map(errorMessageWithContext) - }; - - this.send(req, res, errorResponseBody, statusCode); -}; - -function stripConnectionInfo(message) { - // 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'); -} - -var ERROR_INFO_TO_EXPOSE = { - message: true, - layer: true, - type: true, - analysis: true, - subtype: true -}; - -function shouldBeExposed (prop) { - return !!ERROR_INFO_TO_EXPOSE[prop]; -} - -function errorMessage(err) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - var message = (_.isString(err) ? err : err.message) || 'Unknown error'; - - return stripConnectionInfo(message); -} - -function errorMessageWithContext(err) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - var message = (_.isString(err) ? err : err.message) || 'Unknown error'; - - var error = { - type: err.type || 'unknown', - message: stripConnectionInfo(message), - }; - - for (var prop in err) { - // type & message are properties from Error's prototype and will be skipped - if (err.hasOwnProperty(prop) && shouldBeExposed(prop)) { - error[prop] = err[prop]; - } - } - - return error; -} -module.exports.errorMessage = errorMessage; - -function findStatusCode(err) { - var statusCode; - if ( err.http_status ) { - statusCode = err.http_status; - } else { - statusCode = statusFromErrorMessage('' + err); - } - return statusCode; -} -module.exports.findStatusCode = findStatusCode; - -function statusFromErrorMessage(errMsg) { - // Find an appropriate statusCode based on message - // jshint maxcomplexity:7 - var statusCode = 400; - if ( -1 !== errMsg.indexOf('permission denied') ) { - statusCode = 403; - } - else if ( -1 !== errMsg.indexOf('authentication failed') ) { - statusCode = 403; - } - else if (errMsg.match(/Postgis Plugin.*[\s|\n].*column.*does not exist/)) { - statusCode = 400; - } - else if ( -1 !== errMsg.indexOf('does not exist') ) { - if ( -1 !== errMsg.indexOf(' role ') ) { - statusCode = 403; // role 'xxx' does not exist - } else if ( errMsg.match(/function .* does not exist/) ) { - statusCode = 400; // invalid SQL (SQL function does not exist) - } else { - statusCode = 404; - } - } - - return statusCode; -} - -function isRenderTimeoutError (err) { - return err.message === 'Render timed out'; -} - -function isDatasourceTimeoutError (err) { - return err.message && err.message.match(/canceling statement due to statement timeout/i); -} - -function isTimeoutError (err) { - return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); -} - -function populateTimeoutErrors (errors) { - return errors.map(function (error) { - if (isRenderTimeoutError(error)) { - error.subtype = 'render'; - } - - if (isDatasourceTimeoutError(error)) { - error.subtype = 'datasource'; - } - - if (isTimeoutError(error)) { - error.message = 'You are over platform\'s limits. Please contact us to know more details'; - error.type = 'limit'; - error.http_status = 429; - } - - return error; - }); -} diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 9644aa06..6a30b2ae 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -130,7 +130,7 @@ LayergroupController.prototype.register = function(app) { this.analysisNodeStatus.bind(this)); }; -LayergroupController.prototype.analysisNodeStatus = function(req, res) { +LayergroupController.prototype.analysisNodeStatus = function(req, res, next) { var self = this; step( @@ -145,7 +145,8 @@ LayergroupController.prototype.analysisNodeStatus = function(req, res) { req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'GET NODE STATUS'); + err.label = 'GET NODE STATUS'; + next(err); } else { self.sendResponse(req, res, nodeStatus, 200, { 'Cache-Control': 'public,max-age=5', @@ -156,7 +157,7 @@ LayergroupController.prototype.analysisNodeStatus = function(req, res) { ); }; -LayergroupController.prototype.dataview = function(req, res) { +LayergroupController.prototype.dataview = function(req, res, next) { var self = this; step( @@ -175,7 +176,8 @@ LayergroupController.prototype.dataview = function(req, res) { req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'GET DATAVIEW'); + err.label = 'GET DATAVIEW'; + next(err); } else { self.sendResponse(req, res, dataview, 200); } @@ -184,7 +186,7 @@ LayergroupController.prototype.dataview = function(req, res) { }; -LayergroupController.prototype.dataviewSearch = function(req, res) { +LayergroupController.prototype.dataviewSearch = function(req, res, next) { var self = this; step( @@ -203,7 +205,8 @@ LayergroupController.prototype.dataviewSearch = function(req, res) { req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'GET DATAVIEW SEARCH'); + err.label = 'GET DATAVIEW SEARCH'; + next(err); } else { self.sendResponse(req, res, searchResult, 200); } @@ -212,7 +215,7 @@ LayergroupController.prototype.dataviewSearch = function(req, res) { }; -LayergroupController.prototype.attributes = function(req, res) { +LayergroupController.prototype.attributes = function(req, res, next) { var self = this; req.profiler.start('windshaft.maplayer_attribute'); @@ -233,7 +236,8 @@ LayergroupController.prototype.attributes = function(req, res) { req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'GET ATTRIBUTES'); + err.label = 'GET ATTRIBUTES'; + next(err); } else { self.sendResponse(req, res, tile, 200); } @@ -243,9 +247,9 @@ LayergroupController.prototype.attributes = function(req, res) { }; // Gets a tile for a given token and set of tile ZXY coords. (OSM style) -LayergroupController.prototype.tile = function(req, res) { +LayergroupController.prototype.tile = function(req, res, next) { req.profiler.start('windshaft.map_tile'); - this.tileOrLayer(req, res); + this.tileOrLayer(req, res, next); }; // Gets a tile for a given token, layer set of tile ZXY coords. (OSM style) @@ -254,10 +258,10 @@ LayergroupController.prototype.layer = function(req, res, next) { return next(); } req.profiler.start('windshaft.maplayer_tile'); - this.tileOrLayer(req, res); + this.tileOrLayer(req, res, next); }; -LayergroupController.prototype.tileOrLayer = function (req, res) { +LayergroupController.prototype.tileOrLayer = function (req, res, next) { var self = this; step( @@ -273,14 +277,14 @@ LayergroupController.prototype.tileOrLayer = function (req, res) { }, function mapController$finalize(err, tile, headers, stats) { req.profiler.add(stats); - self.finalizeGetTileOrGrid(err, req, res, tile, headers); + self.finalizeGetTileOrGrid(err, req, res, tile, headers, next); } ); }; // This function is meant for being called as the very last // step by all endpoints serving tiles or grids -LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, tile, headers) { +LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, tile, headers, next) { var supportedFormats = { grid_json: true, json_torque: true, @@ -309,7 +313,9 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t } err.message = errMsg; - this.sendError(req, res, err, 'TILE RENDER'); + err.label = 'TILE RENDER'; + next(err); + global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); } else { @@ -319,23 +325,23 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t } }; -LayergroupController.prototype.bbox = function(req, res) { +LayergroupController.prototype.bbox = function(req, res, next) { this.staticMap(req, res, +req.params.width, +req.params.height, { west: +req.params.west, north: +req.params.north, east: +req.params.east, south: +req.params.south - }); + }, next); }; -LayergroupController.prototype.center = function(req, res) { +LayergroupController.prototype.center = function(req, res, next) { this.staticMap(req, res, +req.params.width, +req.params.height, +req.params.z, { lng: +req.params.lng, lat: +req.params.lat - }); + }, next); }; -LayergroupController.prototype.staticMap = function(req, res, width, height, zoom /* bounds */, center) { +LayergroupController.prototype.staticMap = function(req, res, width, height, zoom /* bounds */, center, next) { var format = req.params.format === 'jpg' ? 'jpeg' : 'png'; req.params.layer = 'all'; req.params.format = 'png'; @@ -363,7 +369,8 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'STATIC_MAP'); + err.label = 'STATIC_MAP'; + next(err); } else { res.set('Content-Type', headers['Content-Type'] || 'image/' + format); self.sendResponse(req, res, image, 200); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 282c3145..2286c449 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -67,7 +67,7 @@ MapController.prototype.register = function(app) { app.options(app.base_url_mapconfig, cors('Content-Type')); }; -MapController.prototype.createGet = function(req, res){ +MapController.prototype.createGet = function(req, res, next){ req.profiler.start('windshaft.createmap_get'); this.create(req, res, function createGet$prepareConfig(err, req) { @@ -76,10 +76,10 @@ MapController.prototype.createGet = function(req, res){ throw new Error('layergroup GET needs a "config" parameter'); } return JSON.parse(req.params.config); - }); + }, next); }; -MapController.prototype.createPost = function(req, res) { +MapController.prototype.createPost = function(req, res, next) { req.profiler.start('windshaft.createmap_post'); this.create(req, res, function createPost$prepareConfig(err, req) { @@ -88,10 +88,10 @@ MapController.prototype.createPost = function(req, res) { throw new Error('layergroup POST data must be of type application/json'); } return req.body; - }); + }, next); }; -MapController.prototype.instantiate = function(req, res) { +MapController.prototype.instantiate = function(req, res, next) { req.profiler.start('windshaft-cartodb.instance_template_post'); this.instantiateTemplate(req, res, function prepareTemplateParams(callback) { @@ -99,10 +99,10 @@ MapController.prototype.instantiate = function(req, res) { return callback(new Error('Template POST data must be of type application/json')); } return callback(null, req.body); - }); + }, next); }; -MapController.prototype.jsonp = function(req, res) { +MapController.prototype.jsonp = function(req, res, next) { req.profiler.start('windshaft-cartodb.instance_template_get'); this.instantiateTemplate(req, res, function prepareJsonTemplateParams(callback) { @@ -121,10 +121,10 @@ MapController.prototype.jsonp = function(req, res) { } return callback(err, templateParams); - }); + }, next); }; -MapController.prototype.create = function(req, res, prepareConfigFn) { +MapController.prototype.create = function(req, res, prepareConfigFn, next) { var self = this; var mapConfig; @@ -188,7 +188,8 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { err = error; } - self.sendError(req, res, err, 'ANONYMOUS LAYERGROUP'); + err.label = 'ANONYMOUS LAYERGROUP'; + next(err); } else { var analysesResults = context.analysesResults || []; self.addDataviewsAndWidgetsUrls(req.context.user, layergroup, mapConfig.obj()); @@ -212,7 +213,7 @@ function addContextMetadata(layergroup, mapConfig, context) { } } -MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn) { +MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn, next) { var self = this; var cdbuser = req.context.user; @@ -259,7 +260,8 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function finishTemplateInstantiation(err, layergroup) { if (err) { - self.sendError(req, res, err, 'NAMED MAP LAYERGROUP'); + err.label = 'NAMED MAP LAYERGROUP'; + next(err); } else { var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index bde3827d..d0c24ce6 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -74,7 +74,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header ); }; -NamedMapsController.prototype.tile = function(req, res) { +NamedMapsController.prototype.tile = function(req, res, next) { var self = this; var cdbUser = req.context.user; @@ -103,7 +103,8 @@ NamedMapsController.prototype.tile = function(req, res) { function handleImage(err, tile, headers, stats) { req.profiler.add(stats); if (err) { - self.sendError(req, res, err, 'NAMED_MAP_TILE'); + err.label = 'NAMED_MAP_TILE'; + next(err); } else { self.sendResponse(req, res, tile, headers, namedMapProvider); } @@ -111,7 +112,7 @@ NamedMapsController.prototype.tile = function(req, res) { ); }; -NamedMapsController.prototype.staticMap = function(req, res) { +NamedMapsController.prototype.staticMap = function(req, res, next) { var self = this; var cdbUser = req.context.user; @@ -179,7 +180,8 @@ NamedMapsController.prototype.staticMap = function(req, res) { req.profiler.add(stats || {}); if (err) { - self.sendError(req, res, err, 'STATIC_VIZ_MAP'); + err.label = 'STATIC_VIZ_MAP'; + next(err); } else { self.sendResponse(req, res, image, headers, namedMapProvider); } diff --git a/lib/cartodb/controllers/named_maps_admin.js b/lib/cartodb/controllers/named_maps_admin.js index 08ad5322..b8bd89dd 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -35,7 +35,7 @@ NamedMapsAdminController.prototype.register = function(app) { app.options(app.base_url_templated + '/:template_id', cors('Content-Type')); }; -NamedMapsAdminController.prototype.create = function(req, res) { +NamedMapsAdminController.prototype.create = function(req, res, next) { var self = this; var cdbuser = req.context.user; @@ -55,11 +55,11 @@ NamedMapsAdminController.prototype.create = function(req, res) { assert.ifError(err); return { template_id: tpl_id }; }, - finishFn(self, req, res, 'POST TEMPLATE') + finishFn(self, req, res, 'POST TEMPLATE', null, next) ); }; -NamedMapsAdminController.prototype.update = function(req, res) { +NamedMapsAdminController.prototype.update = function(req, res, next) { var self = this; var cdbuser = req.context.user; @@ -84,11 +84,11 @@ NamedMapsAdminController.prototype.update = function(req, res) { return { template_id: tpl_id }; }, - finishFn(self, req, res, 'PUT TEMPLATE') + finishFn(self, req, res, 'PUT TEMPLATE', null, next) ); }; -NamedMapsAdminController.prototype.retrieve = function(req, res) { +NamedMapsAdminController.prototype.retrieve = function(req, res, next) { var self = this; req.profiler.start('windshaft-cartodb.get_template'); @@ -118,11 +118,11 @@ NamedMapsAdminController.prototype.retrieve = function(req, res) { delete tpl_val.auth_id; return { template: tpl_val }; }, - finishFn(self, req, res, 'GET TEMPLATE') + finishFn(self, req, res, 'GET TEMPLATE', null, next) ); }; -NamedMapsAdminController.prototype.destroy = function(req, res) { +NamedMapsAdminController.prototype.destroy = function(req, res, next) { var self = this; req.profiler.start('windshaft-cartodb.delete_template'); @@ -144,11 +144,11 @@ NamedMapsAdminController.prototype.destroy = function(req, res) { assert.ifError(err); return ''; }, - finishFn(self, req, res, 'DELETE TEMPLATE', 204) + finishFn(self, req, res, 'DELETE TEMPLATE', 204, next) ); }; -NamedMapsAdminController.prototype.list = function(req, res) { +NamedMapsAdminController.prototype.list = function(req, res, next) { var self = this; req.profiler.start('windshaft-cartodb.get_template_list'); @@ -168,14 +168,15 @@ NamedMapsAdminController.prototype.list = function(req, res) { assert.ifError(err); return { template_ids: tpl_ids }; }, - finishFn(self, req, res, 'GET TEMPLATE LIST') + finishFn(self, req, res, 'GET TEMPLATE LIST', null, next) ); }; -function finishFn(controller, req, res, description, status) { +function finishFn(controller, req, res, description, status, next) { return function finish(err, response){ if (err) { - controller.sendError(req, res, err, description); + err.label = description; + next(err); } else { controller.send(req, res, response, status || 200); } diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js new file mode 100644 index 00000000..d42dcf2e --- /dev/null +++ b/lib/cartodb/middleware/error-middleware.js @@ -0,0 +1,175 @@ +const _ = require('underscore'); +const debug = require('debug')('windshaft:cartodb:error-middleware'); + +module.exports = function errorMiddleware (/* options */) { + return function (err, req, res, next) { + // jshint unused:false + // jshint maxcomplexity:9 + var allErrors = Array.isArray(err) ? err : [err]; + + allErrors = populateTimeoutErrors(allErrors); + + const label = err.label || 'UNKNOWN'; + err = allErrors[0] || new Error(label); + allErrors[0] = err; + + var statusCode = findStatusCode(err); + + if (err.message === 'Tile does not exist' && req.locals.format === 'mvt') { + statusCode = 204; + } + + debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); + + // If a callback was requested, force status to 200 + if (req.query && req.query.callback) { + statusCode = 200; + } + + var errorResponseBody = { + errors: allErrors.map(errorMessage), + errors_with_context: allErrors.map(errorMessageWithContext) + }; + + if (req.locals && req.locals.dbhost) { + res.set('X-Served-By-DB-Host', req.locals.dbhost); + } + + res.set('X-Tiler-Profiler', req.profiler.toJSONString()); + + res.status(statusCode); + + if (req.query && req.query.callback) { + res.jsonp(errorResponseBody); + } else { + res.json(errorResponseBody); + } + + try { + // May throw due to dns, see + // See http://github.com/CartoDB/Windshaft/issues/166 + req.profiler.sendStats(); + } catch (err) { + debug("error sending profiling stats: " + err); + } + }; +}; + +function isRenderTimeoutError (err) { + return err.message === 'Render timed out'; +} + +function isDatasourceTimeoutError (err) { + return err.message && err.message.match(/canceling statement due to statement timeout/i); +} + +function isTimeoutError (err) { + return isRenderTimeoutError(err) || isDatasourceTimeoutError(err); +} + +function populateTimeoutErrors (errors) { + return errors.map(function (error) { + if (isRenderTimeoutError(error)) { + error.subtype = 'render'; + } + + if (isDatasourceTimeoutError(error)) { + error.subtype = 'datasource'; + } + + if (isTimeoutError(error)) { + error.message = 'You are over platform\'s limits. Please contact us to know more details'; + error.type = 'limit'; + error.http_status = 429; + } + + return error; + }); +} + +function findStatusCode(err) { + var statusCode; + if ( err.http_status ) { + statusCode = err.http_status; + } else { + statusCode = statusFromErrorMessage('' + err); + } + return statusCode; +} + +module.exports.findStatusCode = findStatusCode; + +function statusFromErrorMessage(errMsg) { + // Find an appropriate statusCode based on message + // jshint maxcomplexity:7 + var statusCode = 400; + if ( -1 !== errMsg.indexOf('permission denied') ) { + statusCode = 403; + } + else if ( -1 !== errMsg.indexOf('authentication failed') ) { + statusCode = 403; + } + else if (errMsg.match(/Postgis Plugin.*[\s|\n].*column.*does not exist/)) { + statusCode = 400; + } + else if ( -1 !== errMsg.indexOf('does not exist') ) { + if ( -1 !== errMsg.indexOf(' role ') ) { + statusCode = 403; // role 'xxx' does not exist + } else if ( errMsg.match(/function .* does not exist/) ) { + statusCode = 400; // invalid SQL (SQL function does not exist) + } else { + statusCode = 404; + } + } + + return statusCode; +} + +function errorMessage(err) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + var message = (_.isString(err) ? err : err.message) || 'Unknown error'; + + return stripConnectionInfo(message); +} + +module.exports.errorMessage = errorMessage; + +function stripConnectionInfo(message) { + // 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'); +} + +var ERROR_INFO_TO_EXPOSE = { + message: true, + layer: true, + type: true, + analysis: true, + subtype: true +}; + +function shouldBeExposed (prop) { + return !!ERROR_INFO_TO_EXPOSE[prop]; +} + +function errorMessageWithContext(err) { + // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 + var message = (_.isString(err) ? err : err.message) || 'Unknown error'; + + var error = { + type: err.type || 'unknown', + message: stripConnectionInfo(message), + }; + + for (var prop in err) { + // type & message are properties from Error's prototype and will be skipped + if (err.hasOwnProperty(prop) && shouldBeExposed(prop)) { + error[prop] = err[prop]; + } + } + + return error; +} diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index d9302276..f3e88107 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -44,6 +44,8 @@ var MapConfigAdapter = require('./models/mapconfig/adapter'); var StatsBackend = require('./backends/stats'); +const errorMiddleware = require('./middleware/error-middleware'); + module.exports = function(serverOptions) { // Make stats client globally accessible global.statsClient = StatsClient.getInstance(serverOptions.statsd); @@ -257,6 +259,8 @@ module.exports = function(serverOptions) { * END Routing ******************************************************************************************************************/ + app.use(errorMiddleware()); + return app; }; diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index 462e1957..9bf21ccd 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -1,21 +1,21 @@ require('../../support/test_helper.js'); var assert = require('assert'); -var BaseController = require('../../../lib/cartodb/controllers/base'); +var errorMiddleware = require('../../../lib/cartodb/middleware/error-middleware'); -describe('BaseController', function() { +describe('error-middleware', function() { it('different formats for postgis plugin error returns 400 as status code', function() { var expectedStatusCode = 400; assert.equal( - BaseController.findStatusCode("Postgis Plugin: ERROR: column \"missing\" does not exist\n"), + errorMiddleware.findStatusCode("Postgis Plugin: ERROR: column \"missing\" does not exist\n"), expectedStatusCode, "Error status code for single line does not match" ); assert.equal( - BaseController.findStatusCode("Postgis Plugin: PSQL error:\nERROR: column \"missing\" does not exist\n"), + errorMiddleware.findStatusCode("Postgis Plugin: PSQL error:\nERROR: column \"missing\" does not exist\n"), expectedStatusCode, "Error status code for multiline/PSQL does not match" ); diff --git a/test/unit/cartodb/error_messages.test.js b/test/unit/cartodb/error_messages.test.js index 52441db2..bfe0b03a 100644 --- a/test/unit/cartodb/error_messages.test.js +++ b/test/unit/cartodb/error_messages.test.js @@ -2,7 +2,7 @@ require('../../support/test_helper'); var assert = require('assert'); -var BaseController = require('../../../lib/cartodb/controllers/base'); +var errorMiddleware = require('../../../lib/cartodb/middleware/error-middleware'); describe('error messages clean up', function() { @@ -15,7 +15,7 @@ describe('error messages clean up', function() { " encountered during parsing of layer 'layer0' in Layer" ].join('\n'); - var outMessage = BaseController.errorMessage(inMessage); + var outMessage = errorMiddleware.errorMessage(inMessage); assert.ok(outMessage.match('connect'), outMessage); assert.ok(!outMessage.match(/666/), outMessage); diff --git a/test/unit/cartodb/ported/tile_stats.test.js b/test/unit/cartodb/ported/tile_stats.test.js index b71f1384..3fbf93ec 100644 --- a/test/unit/cartodb/ported/tile_stats.test.js +++ b/test/unit/cartodb/ported/tile_stats.test.js @@ -41,7 +41,9 @@ describe('tile stats', function() { jsonp: function() {}, send: function() {} }; - layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, resMock, null, null); + + var next = function () {}; + layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, resMock, null, null, next); assert.ok(formatMatched, 'Format was never matched in increment method'); assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method'); @@ -74,7 +76,8 @@ describe('tile stats', function() { var layergroupController = new LayergroupController(); - layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, resMock, null, null); + var next = function () {}; + layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, resMock, null, null, next); assert.ok(formatMatched, 'Format was never matched in increment method'); assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method');