Moves sendError and sendResponse to Base controller

Test for findStatusCode moved to controller

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;
+}

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);
         }
     );
 

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]);
             }
         }
     );

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);
             }

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]);
         }
     };
 }

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;
 }

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"
+        );
+    });
+});

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));

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"
-        );
-    });
-
 });