From fba5a3551469b62271c7e2b1751cf493d66e4cfa Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Tue, 15 Sep 2015 19:28:02 +0200 Subject: [PATCH] Move sendResponse and sendError to response object --- lib/cartodb/controllers/layergroup.js | 8 +- lib/cartodb/controllers/map.js | 8 +- lib/cartodb/controllers/named_maps.js | 6 +- lib/cartodb/controllers/named_maps_admin.js | 4 +- lib/cartodb/server.js | 148 ++++++++++---------- test/unit/cartodb/ported/tile_stats.test.js | 20 +-- 6 files changed, 96 insertions(+), 98 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 1925225e..1e5ecc9a 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -69,7 +69,7 @@ LayergroupController.prototype.attributes = function(req, res) { // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 var errMsg = err.message ? ( '' + err.message ) : ( '' + err ); var statusCode = self.app.findStatusCode(err); - self.app.sendError(res, { errors: [errMsg] }, statusCode, 'GET ATTRIBUTES', err); + res.sendError(res, { errors: [errMsg] }, statusCode, 'GET ATTRIBUTES', err); } else { self.sendResponse(req, res, [tile, 200]); } @@ -149,7 +149,7 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t errMsg = 'style'+matches[2]+': ' + matches[1]; } - this.app.sendError(res, { errors: ['' + errMsg] }, statusCode, 'TILE RENDER', err); + res.sendError(res, { errors: ['' + errMsg] }, statusCode, 'TILE RENDER', err); global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); } else { @@ -206,7 +206,7 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo if (!err.error) { err.error = err.message; } - self.app.sendError(res, {errors: ['' + err] }, self.app.findStatusCode(err), 'STATIC_MAP', err); + res.sendError(res, {errors: ['' + err] }, self.app.findStatusCode(err), 'STATIC_MAP', err); } else { res.setHeader('Content-Type', headers['Content-Type'] || 'image/' + format); self.sendResponse(req, res, [image, 200]); @@ -245,7 +245,7 @@ LayergroupController.prototype.sendResponse = function(req, res, args) { res.header('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); self.surrogateKeysCache.tag(res, tablesCacheEntry); } - self.app.sendResponse(res, args); + res.sendResponse(res, args); } ); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 45f17797..2a43a3ce 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -156,10 +156,10 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { function finish(err, layergroup) { if (err) { var statusCode = self.app.findStatusCode(err); - self.app.sendError(res, { errors: [ err.message ] }, statusCode, 'ANONYMOUS LAYERGROUP', err); + res.sendError(res, { errors: [ err.message ] }, statusCode, 'ANONYMOUS LAYERGROUP', err); } else { res.header('X-Layergroup-Id', layergroup.layergroupid); - self.app.sendResponse(res, [layergroup, 200]); + res.sendResponse(res, [layergroup, 200]); } } ); @@ -211,7 +211,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn function finishTemplateInstantiation(err, layergroup) { if (err) { var statusCode = self.app.findStatusCode(err); - self.app.sendError(res, { errors: [ err.message ] }, statusCode, 'NAMED MAP LAYERGROUP', err); + res.sendError(res, { errors: [ err.message ] }, statusCode, 'NAMED MAP LAYERGROUP', err); } else { var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; @@ -219,7 +219,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn res.header('X-Layergroup-Id', layergroup.layergroupid); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); - self.app.sendResponse(res, [layergroup, 200]); + res.sendResponse(res, [layergroup, 200]); } } ); diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 8dbc27d1..a751b5d0 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -60,7 +60,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header self.surrogateKeysCache.tag(res, tablesCacheEntry); } } - self.app.sendResponse(res, [resource, 200]); + res.sendResponse(res, [resource, 200]); } ); }; @@ -93,7 +93,7 @@ NamedMapsController.prototype.tile = function(req, res) { if (!err.error) { err.error = err.message; } - self.app.sendError(res, err, self.app.findStatusCode(err), 'NAMED_MAP_TILE', err); + res.sendError(res, err, self.app.findStatusCode(err), 'NAMED_MAP_TILE', err); } else { self.sendResponse(req, res, tile, headers, namedMapProvider); } @@ -185,7 +185,7 @@ NamedMapsController.prototype.staticMap = function(req, res) { if (!err.error) { err.error = err.message; } - self.app.sendError(res, err, self.app.findStatusCode(err), 'STATIC_VIZ_MAP', err); + res.sendError(res, err, self.app.findStatusCode(err), 'STATIC_VIZ_MAP', 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 f2fb1c8a..31dffcad 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -179,9 +179,9 @@ function finishFn(app, res, description, okResponse) { if ( ! _.isUndefined(err.http_status) ) { statusCode = err.http_status; } - app.sendError(res, response, statusCode, description, err); + res.sendError(res, response, statusCode, description, err); } else { - app.sendResponse(res, okResponse || [response, statusCode]); + res.sendResponse(res, okResponse || [response, statusCode]); } }; } diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 225b61c4..4ee6464f 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -227,81 +227,6 @@ module.exports = function(serverOptions) { * END Routing ******************************************************************************************************************/ - // temporary measure until we upgrade to newer version expressjs so we can check err.status - app.use(function(err, req, res, next) { - if (err) { - if (err.name === 'SyntaxError') { - app.sendError(res, { errors: [err.name + ': ' + err.message] }, 400, 'JSON', err); - } else { - next(err); - } - } else { - next(); - } - }); - - app.sendResponse = function(res, args) { - var req = res.req; - - if (global.environment && global.environment.api_hostname) { - res.header('X-Served-By-Host', global.environment.api_hostname); - } - - if (req && req.params && req.params.dbhost) { - res.header('X-Served-By-DB-Host', req.params.dbhost); - } - - if ( req && req.profiler ) { - res.header('X-Tiler-Profiler', req.profiler.toJSONString()); - } - -// res.send(body|status[, headers|status[, status]]) - res.send.apply(res, args); - - if ( req && 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); - } - } - }; - - app.sendError = function(res, err, statusCode, label, tolog) { - res._windshaftStatusCode = statusCode; - - var olabel = '['; - if ( label ) { - olabel += label + ' '; - } - olabel += 'ERROR]'; - if ( ! tolog ) { - tolog = err; - } - var log_msg = olabel + " -- " + statusCode + ": " + tolog; - //if ( tolog.stack ) log_msg += "\n" + tolog.stack; - console.error(log_msg); // use console.log for statusCode != 500 ? - // If a callback was requested, force status to 200 - if ( res.req ) { - // NOTE: res.req can be undefined when we fake a call to - // ourself from POST to /layergroup - if ( res.req.query.callback ) { - statusCode = 200; - } - } - // Strip connection info, if any - // See https://github.com/CartoDB/Windshaft/issues/173 - err = JSON.stringify(err); - err = err.replace(/Connection string: '[^']*'\\n/, ''); - // See https://travis-ci.org/CartoDB/Windshaft/jobs/20703062#L1644 - err = err.replace(/is the server.*encountered/im, 'encountered'); - err = JSON.parse(err); - - app.sendResponse(res, [err, statusCode]); - }; - // jshint maxcomplexity:10 /** * Whitelist input and get database name & default geometry type from @@ -472,10 +397,83 @@ function bootstrap(opts) { statsd_client: global.statsClient, profile: opts.useProfiler }); + res.removeHeader('x-powered-by'); + + res.sendResponse = function(res, args) { + if (global.environment && global.environment.api_hostname) { + res.header('X-Served-By-Host', global.environment.api_hostname); + } + + if (req && req.params && req.params.dbhost) { + res.header('X-Served-By-DB-Host', req.params.dbhost); + } + + if (req && req.profiler ) { + res.header('X-Tiler-Profiler', req.profiler.toJSONString()); + } + +// res.send(body|status[, headers|status[, status]]) + res.send.apply(res, args); + + if ( req && 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(res, err, statusCode, label, tolog) { + res._windshaftStatusCode = statusCode; + + var olabel = '['; + if ( label ) { + olabel += label + ' '; + } + olabel += 'ERROR]'; + if ( ! tolog ) { + tolog = err; + } + var log_msg = olabel + " -- " + statusCode + ": " + tolog; + //if ( tolog.stack ) log_msg += "\n" + tolog.stack; + console.error(log_msg); // use console.log for statusCode != 500 ? + // If a callback was requested, force status to 200 + if ( res.req ) { + // NOTE: res.req can be undefined when we fake a call to + // ourself from POST to /layergroup + if ( res.req.query.callback ) { + statusCode = 200; + } + } + // Strip connection info, if any + // See https://github.com/CartoDB/Windshaft/issues/173 + err = JSON.stringify(err); + err = err.replace(/Connection string: '[^']*'\\n/, ''); + // See https://travis-ci.org/CartoDB/Windshaft/jobs/20703062#L1644 + err = err.replace(/is the server.*encountered/im, 'encountered'); + err = JSON.parse(err); + + res.sendResponse(res, [err, statusCode]); + }; next(); }); + // temporary measure until we upgrade to newer version expressjs so we can check err.status + app.use(function(err, req, res, next) { + if (err) { + if (err.name === 'SyntaxError') { + res.send({ errors: [err.name + ': ' + err.message] }, 400); + } else { + next(err); + } + } else { + next(); + } + }); + setupLogger(app, opts); return app; diff --git a/test/unit/cartodb/ported/tile_stats.test.js b/test/unit/cartodb/ported/tile_stats.test.js index 433daddf..b0bbf79c 100644 --- a/test/unit/cartodb/ported/tile_stats.test.js +++ b/test/unit/cartodb/ported/tile_stats.test.js @@ -28,17 +28,17 @@ describe('tile stats', function() { } }); - var ws = cartodbServer(serverOptions); - ws.sendError = function(){}; - - var layergroupController = new LayergroupController(ws, null); + var layergroupController = new LayergroupController(cartodbServer(serverOptions)); var reqMock = { params: { format: invalidFormat } }; - layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, {}, null, null); + var resMock = { + sendError: function() {} + }; + layergroupController.finalizeGetTileOrGrid('Unsupported format png2', reqMock, resMock, null, null); assert.ok(formatMatched, 'Format was never matched in increment method'); assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method'); @@ -60,13 +60,13 @@ describe('tile stats', function() { format: validFormat } }; + var resMock = { + sendError: function() {} + }; - var ws = cartodbServer(serverOptions); - ws.sendError = function(){}; + var layergroupController = new LayergroupController(cartodbServer(serverOptions)); - var layergroupController = new LayergroupController(ws, null); - - layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, {}, null, null); + layergroupController.finalizeGetTileOrGrid('Another error happened', reqMock, resMock, null, null); assert.ok(formatMatched, 'Format was never matched in increment method'); assert.equal(expectedCalls, 0, 'Unexpected number of calls to increment method');