diff --git a/lib/cartodb/controllers/base.js b/lib/cartodb/controllers/base.js index 3d2fad13..7860e1ed 100644 --- a/lib/cartodb/controllers/base.js +++ b/lib/cartodb/controllers/base.js @@ -148,3 +148,94 @@ BaseController.prototype.req2params = function(req, callback){ ); }; // jshint maxcomplexity:6 + + +BaseController.prototype.send = function(req, res, args) { + if (global.environment && global.environment.api_hostname) { + res.header('X-Served-By-Host', global.environment.api_hostname); + } + + if (req.params && req.params.dbhost) { + res.header('X-Served-By-DB-Host', req.params.dbhost); + } + + if (req.profiler) { + res.header('X-Tiler-Profiler', req.profiler.toJSONString()); + } + + res.send.apply(res, args); + + if (req.profiler ) { + try { + // May throw due to dns, see + // See http://github.com/CartoDB/Windshaft/issues/166 + req.profiler.sendStats(); + } catch (err) { + console.error("error sending profiling stats: " + err); + } + } +}; + +BaseController.prototype.sendError = function(req, res, err, label) { + label = label || 'UNKNOWN'; + + 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); + } + + // If a callback was requested, force status to 200 + if (req.query && req.query.callback) { + 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] }; + + this.send(req, res, [errorResponseBody, statusCode]); +}; + +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 + 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 { + statusCode = 404; + } + } + return statusCode; +} diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index ed95dabe..59e7a98b 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -75,7 +75,7 @@ LayergroupController.prototype.attributes = function(req, res) { req.profiler.add(stats || {}); if (err) { - res.sendError(err, 'GET ATTRIBUTES'); + self.sendError(req, res, err, 'GET ATTRIBUTES'); } else { self.sendResponse(req, res, [tile, 200]); } @@ -155,7 +155,7 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t } err.message = errMsg; - res.sendError(err, 'TILE RENDER'); + this.sendError(req, res, err, 'TILE RENDER'); global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); } else { @@ -209,7 +209,7 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo req.profiler.add(stats || {}); if (err) { - res.sendError(err, 'STATIC_MAP'); + self.sendError(req, res, err, 'STATIC_MAP'); } else { res.setHeader('Content-Type', headers['Content-Type'] || 'image/' + format); self.sendResponse(req, res, [image, 200]); @@ -248,7 +248,7 @@ LayergroupController.prototype.sendResponse = function(req, res, args) { res.header('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); self.surrogateKeysCache.tag(res, tablesCacheEntry); } - res.sendResponse(args); + self.send(req, res, args); } ); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 45b2b1ce..ffa776a7 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -164,10 +164,10 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { }, function finish(err, layergroup) { if (err) { - res.sendError(err, 'ANONYMOUS LAYERGROUP'); + self.sendError(req, res, err, 'ANONYMOUS LAYERGROUP'); } else { res.header('X-Layergroup-Id', layergroup.layergroupid); - res.sendResponse([layergroup, 200]); + self.send(req, res, [layergroup, 200]); } } ); @@ -218,7 +218,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function finishTemplateInstantiation(err, layergroup) { if (err) { - res.sendError(err, 'NAMED MAP LAYERGROUP'); + self.sendError(req, res, err, 'NAMED MAP LAYERGROUP'); } else { var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; @@ -226,7 +226,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.header('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); - res.sendResponse([layergroup, 200]); + self.send(req, res, [layergroup, 200]); } } ); diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index b16f1f69..d921b956 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -68,7 +68,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header self.surrogateKeysCache.tag(res, tablesCacheEntry); } } - res.sendResponse([resource, 200]); + self.send(req, res, [resource, 200]); } ); }; @@ -98,7 +98,7 @@ NamedMapsController.prototype.tile = function(req, res) { req.profiler.add(stats); } if (err) { - res.sendError(err, 'NAMED_MAP_TILE'); + self.sendError(req, res, err, 'NAMED_MAP_TILE'); } else { self.sendResponse(req, res, tile, headers, namedMapProvider); } @@ -187,7 +187,7 @@ NamedMapsController.prototype.staticMap = function(req, res) { } if (err) { - res.sendError(err, 'STATIC_VIZ_MAP'); + self.sendError(req, res, err, 'STATIC_VIZ_MAP'); } 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 02792cf3..c393b1c3 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -54,7 +54,7 @@ NamedMapsAdminController.prototype.create = function(req, res) { assert.ifError(err); return { template_id: tpl_id }; }, - finishFn(res, 'POST TEMPLATE') + finishFn(self, req, res, 'POST TEMPLATE') ); }; @@ -82,7 +82,7 @@ NamedMapsAdminController.prototype.update = function(req, res) { return { template_id: tpl_id }; }, - finishFn(res, 'PUT TEMPLATE') + finishFn(self, req, res, 'PUT TEMPLATE') ); }; @@ -118,7 +118,7 @@ NamedMapsAdminController.prototype.retrieve = function(req, res) { delete tpl_val.auth_id; return { template: tpl_val }; }, - finishFn(res, 'GET TEMPLATE') + finishFn(self, req, res, 'GET TEMPLATE') ); }; @@ -146,7 +146,7 @@ NamedMapsAdminController.prototype.destroy = function(req, res) { assert.ifError(err); return { status: 'ok' }; }, - finishFn(res, 'DELETE TEMPLATE', ['', 204]) + finishFn(self, req, res, 'DELETE TEMPLATE', ['', 204]) ); }; @@ -172,16 +172,16 @@ NamedMapsAdminController.prototype.list = function(req, res) { assert.ifError(err); return { template_ids: tpl_ids }; }, - finishFn(res, 'GET TEMPLATE LIST') + finishFn(self, req, res, 'GET TEMPLATE LIST') ); }; -function finishFn(res, description, okResponse) { +function finishFn(controller, req, res, description, okResponse) { return function finish(err, response){ if (err) { - res.sendError(err, description); + controller.sendError(req, res, err, description); } else { - res.sendResponse(okResponse || [response, 200]); + controller.send(req, res, okResponse || [response, 200]); } }; } diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index b2f69c6d..8f1e3de8 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -255,62 +255,6 @@ function bootstrap(opts) { res.removeHeader('x-powered-by'); - res.sendResponse = function(args) { - if (global.environment && global.environment.api_hostname) { - res.header('X-Served-By-Host', global.environment.api_hostname); - } - - if (req.params && req.params.dbhost) { - res.header('X-Served-By-DB-Host', req.params.dbhost); - } - - if (req.profiler) { - res.header('X-Tiler-Profiler', req.profiler.toJSONString()); - } - - res.send.apply(res, args); - - if (req.profiler ) { - try { - // May throw due to dns, see - // See http://github.com/CartoDB/Windshaft/issues/166 - req.profiler.sendStats(); - } catch (err) { - console.error("error sending profiling stats: " + err); - } - } - }; - res.sendError = function(err, label) { - - label = label || 'UNKNOWN'; - - 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); - } - - // If a callback was requested, force status to 200 - if (req.query.callback) { - 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] }; - - res.sendResponse([errorResponseBody, statusCode]); - }; next(); }); @@ -371,39 +315,6 @@ function surrogateKeysCacheBackends(serverOptions) { return cacheBackends; } -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 - 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 { - statusCode = 404; - } - } - return statusCode; -} - function mapnikVersion(opts) { return opts.grainstore.mapnik_version || mapnik.versions.mapnik; } diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js new file mode 100644 index 00000000..462e1957 --- /dev/null +++ b/test/unit/cartodb/base_controller.js @@ -0,0 +1,23 @@ +require('../../support/test_helper.js'); + +var assert = require('assert'); +var BaseController = require('../../../lib/cartodb/controllers/base'); + +describe('BaseController', 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"), + 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"), + expectedStatusCode, + "Error status code for multiline/PSQL does not match" + ); + }); +}); diff --git a/test/unit/cartodb/ported/tile_stats.test.js b/test/unit/cartodb/ported/tile_stats.test.js index b0bbf79c..318c7194 100644 --- a/test/unit/cartodb/ported/tile_stats.test.js +++ b/test/unit/cartodb/ported/tile_stats.test.js @@ -36,7 +36,7 @@ describe('tile stats', function() { } }; var resMock = { - sendError: function() {} + send: function() {} }; layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, resMock, null, null); @@ -61,7 +61,7 @@ describe('tile stats', function() { } }; var resMock = { - sendError: function() {} + send: function() {} }; var layergroupController = new LayergroupController(cartodbServer(serverOptions)); diff --git a/test/unit/cartodb/ported/windshaft_server.test.js b/test/unit/cartodb/ported/windshaft_server.test.js index 62b428cb..c7acdceb 100644 --- a/test/unit/cartodb/ported/windshaft_server.test.js +++ b/test/unit/cartodb/ported/windshaft_server.test.js @@ -41,20 +41,4 @@ describe('windshaft', function() { assert.equal(ws.base_url, '/tiles/:table'); }); - it('different formats for postgis plugin error returns 400 as status code', function() { - - var expectedStatusCode = 400; - assert.equal( - cartodbServer.findStatusCode("Postgis Plugin: ERROR: column \"missing\" does not exist\n"), - expectedStatusCode, - "Error status code for single line does not match" - ); - - assert.equal( - cartodbServer.findStatusCode("Postgis Plugin: PSQL error:\nERROR: column \"missing\" does not exist\n"), - expectedStatusCode, - "Error status code for multiline/PSQL does not match" - ); - }); - });