diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 1e5ecc9a..57a7628f 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -66,10 +66,7 @@ LayergroupController.prototype.attributes = function(req, res) { req.profiler.add(stats || {}); if (err) { - // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 - var errMsg = err.message ? ( '' + err.message ) : ( '' + err ); - var statusCode = self.app.findStatusCode(err); - res.sendError(res, { errors: [errMsg] }, statusCode, 'GET ATTRIBUTES', err); + res.sendError(err, 'GET ATTRIBUTES'); } else { self.sendResponse(req, res, [tile, 200]); } @@ -138,18 +135,18 @@ LayergroupController.prototype.finalizeGetTileOrGrid = function(err, req, res, t } } - if (err){ + if (err) { // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 var errMsg = err.message ? ( '' + err.message ) : ( '' + err ); - var statusCode = this.app.findStatusCode(err); // Rewrite mapnik parsing errors to start with layer number var matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); if (matches) { errMsg = 'style'+matches[2]+': ' + matches[1]; } + err.message = errMsg; - res.sendError(res, { errors: ['' + errMsg] }, statusCode, 'TILE RENDER', err); + res.sendError(err, 'TILE RENDER'); global.statsClient.increment('windshaft.tiles.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); } else { @@ -203,10 +200,7 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo req.profiler.add(stats || {}); if (err) { - if (!err.error) { - err.error = err.message; - } - res.sendError(res, {errors: ['' + err] }, self.app.findStatusCode(err), 'STATIC_MAP', err); + res.sendError(err, 'STATIC_MAP'); } else { res.setHeader('Content-Type', headers['Content-Type'] || 'image/' + format); self.sendResponse(req, res, [image, 200]); @@ -245,7 +239,7 @@ LayergroupController.prototype.sendResponse = function(req, res, args) { res.header('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); self.surrogateKeysCache.tag(res, tablesCacheEntry); } - res.sendResponse(res, args); + res.sendResponse(args); } ); diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 2a43a3ce..73182387 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -155,11 +155,10 @@ MapController.prototype.create = function(req, res, prepareConfigFn) { }, function finish(err, layergroup) { if (err) { - var statusCode = self.app.findStatusCode(err); - res.sendError(res, { errors: [ err.message ] }, statusCode, 'ANONYMOUS LAYERGROUP', err); + res.sendError(err, 'ANONYMOUS LAYERGROUP'); } else { res.header('X-Layergroup-Id', layergroup.layergroupid); - res.sendResponse(res, [layergroup, 200]); + res.sendResponse([layergroup, 200]); } } ); @@ -210,8 +209,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn }, function finishTemplateInstantiation(err, layergroup) { if (err) { - var statusCode = self.app.findStatusCode(err); - res.sendError(res, { errors: [ err.message ] }, statusCode, 'NAMED MAP LAYERGROUP', err); + res.sendError(err, 'NAMED MAP LAYERGROUP'); } else { var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; @@ -219,7 +217,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(res, [layergroup, 200]); + res.sendResponse([layergroup, 200]); } } ); diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index a751b5d0..c123c461 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); } } - res.sendResponse(res, [resource, 200]); + res.sendResponse([resource, 200]); } ); }; @@ -90,10 +90,7 @@ NamedMapsController.prototype.tile = function(req, res) { req.profiler.add(stats); } if (err) { - if (!err.error) { - err.error = err.message; - } - res.sendError(res, err, self.app.findStatusCode(err), 'NAMED_MAP_TILE', err); + res.sendError(err, 'NAMED_MAP_TILE'); } else { self.sendResponse(req, res, tile, headers, namedMapProvider); } @@ -182,10 +179,7 @@ NamedMapsController.prototype.staticMap = function(req, res) { } if (err) { - if (!err.error) { - err.error = err.message; - } - res.sendError(res, err, self.app.findStatusCode(err), 'STATIC_VIZ_MAP', err); + res.sendError(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 31dffcad..8e971b1f 100644 --- a/lib/cartodb/controllers/named_maps_admin.js +++ b/lib/cartodb/controllers/named_maps_admin.js @@ -48,7 +48,7 @@ NamedMapsAdminController.prototype.create = function(req, res) { assert.ifError(err); return { template_id: tpl_id }; }, - finishFn(self.app, res, 'POST TEMPLATE') + finishFn(res, 'POST TEMPLATE') ); }; @@ -76,7 +76,7 @@ NamedMapsAdminController.prototype.update = function(req, res) { return { template_id: tpl_id }; }, - finishFn(self.app, res, 'PUT TEMPLATE') + finishFn(res, 'PUT TEMPLATE') ); }; @@ -112,7 +112,7 @@ NamedMapsAdminController.prototype.retrieve = function(req, res) { delete tpl_val.auth_id; return { template: tpl_val }; }, - finishFn(self.app, res, 'GET TEMPLATE') + finishFn(res, 'GET TEMPLATE') ); }; @@ -140,7 +140,7 @@ NamedMapsAdminController.prototype.destroy = function(req, res) { assert.ifError(err); return { status: 'ok' }; }, - finishFn(self.app, res, 'DELETE TEMPLATE', ['', 204]) + finishFn(res, 'DELETE TEMPLATE', ['', 204]) ); }; @@ -166,22 +166,16 @@ NamedMapsAdminController.prototype.list = function(req, res) { assert.ifError(err); return { template_ids: tpl_ids }; }, - finishFn(self.app, res, 'GET TEMPLATE LIST') + finishFn(res, 'GET TEMPLATE LIST') ); }; -function finishFn(app, res, description, okResponse) { +function finishFn(res, description, okResponse) { return function finish(err, response){ - var statusCode = 200; if (err) { - statusCode = 400; - response = { errors: ['' + err] }; - if ( ! _.isUndefined(err.http_status) ) { - statusCode = err.http_status; - } - res.sendError(res, response, statusCode, description, err); + res.sendError(err, description); } else { - res.sendResponse(res, okResponse || [response, statusCode]); + res.sendResponse(okResponse || [response, 200]); } }; } diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 4ee6464f..9d8594a0 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -400,23 +400,22 @@ function bootstrap(opts) { res.removeHeader('x-powered-by'); - res.sendResponse = function(res, args) { + res.sendResponse = function(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) { + if (req.params && req.params.dbhost) { res.header('X-Served-By-DB-Host', req.params.dbhost); } - if (req && req.profiler ) { + if (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 ) { + if (req.profiler ) { try { // May throw due to dns, see // See http://github.com/CartoDB/Windshaft/issues/166 @@ -426,37 +425,36 @@ function bootstrap(opts) { } } }; - res.sendError = function(res, err, statusCode, label, tolog) { - res._windshaftStatusCode = statusCode; + res.sendError = function(err, label) { - var olabel = '['; - if ( label ) { - olabel += label + ' '; + label = label || 'UNKNOWN'; + + var statusCode = app.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); } - 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; - } + if (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]); + // 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(); }); diff --git a/test/acceptance/named_static_maps.js b/test/acceptance/named_static_maps.js index 3df35b22..3acd4bee 100644 --- a/test/acceptance/named_static_maps.js +++ b/test/acceptance/named_static_maps.js @@ -163,8 +163,10 @@ describe('named static maps', function() { var nonexistentName = 'nonexistent'; getStaticMap(nonexistentName, { status: 404 }, function(err, res) { assert.ok(!err); - var parsed = JSON.parse(res.body); - assert.equal(parsed.error, "Template '" + nonexistentName + "' of user '" + username + "' not found"); + assert.deepEqual( + JSON.parse(res.body), + { errors: ["Template '" + nonexistentName + "' of user '" + username + "' not found"] } + ); done(); }); }); @@ -172,8 +174,7 @@ describe('named static maps', function() { it('should return 403 if not properly authorized', function(done) { getStaticMap(tokenAuthTemplateName, { status: 403 }, function(err, res) { assert.ok(!err); - var parsed = JSON.parse(res.body); - assert.equal(parsed.error, 'Unauthorized template instantiation'); + assert.deepEqual(JSON.parse(res.body), { errors: ['Unauthorized template instantiation'] }); done(); }); });