New sendResponse and sendError methods

- fixes response for static named map error cases
This commit is contained in:
Raul Ochoa 2015-09-16 01:36:51 +02:00
parent fba5a35514
commit 72a0c4a487
6 changed files with 55 additions and 76 deletions

View File

@ -66,10 +66,7 @@ LayergroupController.prototype.attributes = function(req, res) {
req.profiler.add(stats || {}); req.profiler.add(stats || {});
if (err) { if (err) {
// See https://github.com/Vizzuality/Windshaft-cartodb/issues/68 res.sendError(err, 'GET ATTRIBUTES');
var errMsg = err.message ? ( '' + err.message ) : ( '' + err );
var statusCode = self.app.findStatusCode(err);
res.sendError(res, { errors: [errMsg] }, statusCode, 'GET ATTRIBUTES', err);
} else { } else {
self.sendResponse(req, res, [tile, 200]); 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 // See https://github.com/Vizzuality/Windshaft-cartodb/issues/68
var errMsg = err.message ? ( '' + err.message ) : ( '' + err ); var errMsg = err.message ? ( '' + err.message ) : ( '' + err );
var statusCode = this.app.findStatusCode(err);
// Rewrite mapnik parsing errors to start with layer number // Rewrite mapnik parsing errors to start with layer number
var matches = errMsg.match("(.*) in style 'layer([0-9]+)'"); var matches = errMsg.match("(.*) in style 'layer([0-9]+)'");
if (matches) { if (matches) {
errMsg = 'style'+matches[2]+': ' + matches[1]; 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.error');
global.statsClient.increment('windshaft.tiles.' + formatStat + '.error'); global.statsClient.increment('windshaft.tiles.' + formatStat + '.error');
} else { } else {
@ -203,10 +200,7 @@ LayergroupController.prototype.staticMap = function(req, res, width, height, zoo
req.profiler.add(stats || {}); req.profiler.add(stats || {});
if (err) { if (err) {
if (!err.error) { res.sendError(err, 'STATIC_MAP');
err.error = err.message;
}
res.sendError(res, {errors: ['' + err] }, self.app.findStatusCode(err), 'STATIC_MAP', err);
} else { } else {
res.setHeader('Content-Type', headers['Content-Type'] || 'image/' + format); res.setHeader('Content-Type', headers['Content-Type'] || 'image/' + format);
self.sendResponse(req, res, [image, 200]); self.sendResponse(req, res, [image, 200]);
@ -245,7 +239,7 @@ LayergroupController.prototype.sendResponse = function(req, res, args) {
res.header('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); res.header('X-Cache-Channel', tablesCacheEntry.getCacheChannel());
self.surrogateKeysCache.tag(res, tablesCacheEntry); self.surrogateKeysCache.tag(res, tablesCacheEntry);
} }
res.sendResponse(res, args); res.sendResponse(args);
} }
); );

View File

@ -155,11 +155,10 @@ MapController.prototype.create = function(req, res, prepareConfigFn) {
}, },
function finish(err, layergroup) { function finish(err, layergroup) {
if (err) { if (err) {
var statusCode = self.app.findStatusCode(err); res.sendError(err, 'ANONYMOUS LAYERGROUP');
res.sendError(res, { errors: [ err.message ] }, statusCode, 'ANONYMOUS LAYERGROUP', err);
} else { } else {
res.header('X-Layergroup-Id', layergroup.layergroupid); 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) { function finishTemplateInstantiation(err, layergroup) {
if (err) { if (err) {
var statusCode = self.app.findStatusCode(err); res.sendError(err, 'NAMED MAP LAYERGROUP');
res.sendError(res, { errors: [ err.message ] }, statusCode, 'NAMED MAP LAYERGROUP', err);
} else { } else {
var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8); var templateHash = self.templateMaps.fingerPrint(mapConfigProvider.template).substring(0, 8);
layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid; layergroup.layergroupid = cdbuser + '@' + templateHash + '@' + layergroup.layergroupid;
@ -219,7 +217,7 @@ MapController.prototype.instantiateTemplate = function(req, res, prepareParamsFn
res.header('X-Layergroup-Id', layergroup.layergroupid); res.header('X-Layergroup-Id', layergroup.layergroupid);
self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName())); self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, mapConfigProvider.getTemplateName()));
res.sendResponse(res, [layergroup, 200]); res.sendResponse([layergroup, 200]);
} }
} }
); );

View File

@ -60,7 +60,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header
self.surrogateKeysCache.tag(res, tablesCacheEntry); 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); req.profiler.add(stats);
} }
if (err) { if (err) {
if (!err.error) { res.sendError(err, 'NAMED_MAP_TILE');
err.error = err.message;
}
res.sendError(res, err, self.app.findStatusCode(err), 'NAMED_MAP_TILE', err);
} else { } else {
self.sendResponse(req, res, tile, headers, namedMapProvider); self.sendResponse(req, res, tile, headers, namedMapProvider);
} }
@ -182,10 +179,7 @@ NamedMapsController.prototype.staticMap = function(req, res) {
} }
if (err) { if (err) {
if (!err.error) { res.sendError(err, 'STATIC_VIZ_MAP');
err.error = err.message;
}
res.sendError(res, err, self.app.findStatusCode(err), 'STATIC_VIZ_MAP', err);
} else { } else {
self.sendResponse(req, res, image, headers, namedMapProvider); self.sendResponse(req, res, image, headers, namedMapProvider);
} }

View File

@ -48,7 +48,7 @@ NamedMapsAdminController.prototype.create = function(req, res) {
assert.ifError(err); assert.ifError(err);
return { template_id: tpl_id }; 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 }; 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; delete tpl_val.auth_id;
return { template: tpl_val }; 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); assert.ifError(err);
return { status: 'ok' }; 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); assert.ifError(err);
return { template_ids: tpl_ids }; 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){ return function finish(err, response){
var statusCode = 200;
if (err) { if (err) {
statusCode = 400; res.sendError(err, description);
response = { errors: ['' + err] };
if ( ! _.isUndefined(err.http_status) ) {
statusCode = err.http_status;
}
res.sendError(res, response, statusCode, description, err);
} else { } else {
res.sendResponse(res, okResponse || [response, statusCode]); res.sendResponse(okResponse || [response, 200]);
} }
}; };
} }

View File

@ -400,23 +400,22 @@ function bootstrap(opts) {
res.removeHeader('x-powered-by'); res.removeHeader('x-powered-by');
res.sendResponse = function(res, args) { res.sendResponse = function(args) {
if (global.environment && global.environment.api_hostname) { if (global.environment && global.environment.api_hostname) {
res.header('X-Served-By-Host', 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); 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.header('X-Tiler-Profiler', req.profiler.toJSONString());
} }
// res.send(body|status[, headers|status[, status]])
res.send.apply(res, args); res.send.apply(res, args);
if ( req && req.profiler ) { if (req.profiler ) {
try { try {
// May throw due to dns, see // May throw due to dns, see
// See http://github.com/CartoDB/Windshaft/issues/166 // See http://github.com/CartoDB/Windshaft/issues/166
@ -426,37 +425,36 @@ function bootstrap(opts) {
} }
} }
}; };
res.sendError = function(res, err, statusCode, label, tolog) { res.sendError = function(err, label) {
res._windshaftStatusCode = statusCode;
var olabel = '['; label = label || 'UNKNOWN';
if ( label ) {
olabel += label + ' '; 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 a callback was requested, force status to 200
if ( res.req ) { if (req.query.callback) {
// NOTE: res.req can be undefined when we fake a call to statusCode = 200;
// 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]); // 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(); next();
}); });

View File

@ -163,8 +163,10 @@ describe('named static maps', function() {
var nonexistentName = 'nonexistent'; var nonexistentName = 'nonexistent';
getStaticMap(nonexistentName, { status: 404 }, function(err, res) { getStaticMap(nonexistentName, { status: 404 }, function(err, res) {
assert.ok(!err); assert.ok(!err);
var parsed = JSON.parse(res.body); assert.deepEqual(
assert.equal(parsed.error, "Template '" + nonexistentName + "' of user '" + username + "' not found"); JSON.parse(res.body),
{ errors: ["Template '" + nonexistentName + "' of user '" + username + "' not found"] }
);
done(); done();
}); });
}); });
@ -172,8 +174,7 @@ describe('named static maps', function() {
it('should return 403 if not properly authorized', function(done) { it('should return 403 if not properly authorized', function(done) {
getStaticMap(tokenAuthTemplateName, { status: 403 }, function(err, res) { getStaticMap(tokenAuthTemplateName, { status: 403 }, function(err, res) {
assert.ok(!err); assert.ok(!err);
var parsed = JSON.parse(res.body); assert.deepEqual(JSON.parse(res.body), { errors: ['Unauthorized template instantiation'] });
assert.equal(parsed.error, 'Unauthorized template instantiation');
done(); done();
}); });
}); });