Merge pull request #308 from CartoDB/issue-305

Named maps error responses with valid format

NEWS.md

@@ -4,6 +4,12 @@
 
 Released 2015-mm-dd
 
+Bug fixes:
+ - Named maps error responses with `{ "errors": ["message"] }` format (#305)
+
+Announcements:
+ - Upgrades windshaft
+
 Enhancements:
  - Named maps names can start with numbers and can contain dashes (-).
 

lib/cartodb/controllers/named_maps.js

@@ -315,23 +315,22 @@ NamedMapsController.prototype.instantiateTemplate = function(req, res, template_
         },
         function prepareResponse(err, layergroup) {
             if ( err ) {
-                throw err;
+                return callback(err, { errors: [''+err] });
             }
             var tplhash = self.templateMaps.fingerPrint(template).substring(0,8);
             layergroup.layergroupid = cdbuser + '@' + tplhash + '@' + layergroup.layergroupid;
 
             self.surrogateKeysCache.tag(res, new NamedMapsCacheEntry(cdbuser, template.name));
 
-            return layergroup;
-        },
-        callback
+            callback(null, layergroup);
+        }
     );
 };
 
 NamedMapsController.prototype.finish_instantiation = function(err, response, res) {
     if (err) {
         var statusCode = 400;
-        response = { error: ''+err };
+        response = { errors: [''+err] };
         if ( ! _.isUndefined(err.http_status) ) {
             statusCode = err.http_status;
         }
@@ -346,7 +345,7 @@ function finishFn(app, res, description, okResponse) {
         var statusCode = 200;
         if (err) {
             statusCode = 400;
-            response = { error: '' + err };
+            response = { errors: ['' + err] };
             if ( ! _.isUndefined(err.http_status) ) {
                 statusCode = err.http_status;
             }

npm-shrinkwrap.json

@@ -314,7 +314,7 @@
                       "dependencies": {
                         "ansi-regex": {
                           "version": "1.1.1",
-                          "from": "ansi-regex@1.1.1",
+                          "from": "ansi-regex@^1.1.0",
                           "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-1.1.1.tgz"
                         }
                       }
@@ -379,7 +379,7 @@
       }
     },
     "log4js": {
-      "version": "0.6.21",
+      "version": "0.6.25",
       "from": "https://github.com/CartoDB/log4js-node/tarball/cdb",
       "resolved": "https://github.com/CartoDB/log4js-node/tarball/cdb",
       "dependencies": {
@@ -413,6 +413,16 @@
               "from": "inherits@~2.0.1"
             }
           }
+        },
+        "semver": {
+          "version": "4.3.6",
+          "from": "semver@~4.3.3",
+          "resolved": "https://registry.npmjs.org/semver/-/semver-4.3.6.tgz"
+        },
+        "underscore": {
+          "version": "1.8.2",
+          "from": "underscore@1.8.2",
+          "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.8.2.tgz"
         }
       }
     },
@@ -476,9 +486,9 @@
       "resolved": "https://registry.npmjs.org/underscore/-/underscore-1.6.0.tgz"
     },
     "windshaft": {
-      "version": "0.44.1",
-      "from": "windshaft@0.44.1",
-      "resolved": "https://registry.npmjs.org/windshaft/-/windshaft-0.44.1.tgz",
+      "version": "0.45.0",
+      "from": "https://github.com/CartoDB/Windshaft/tarball/issue-351",
+      "resolved": "https://github.com/CartoDB/Windshaft/tarball/issue-351",
       "dependencies": {
         "chronograph": {
           "version": "0.1.0",
@@ -2329,7 +2339,7 @@
             },
             "mime": {
               "version": "1.2.11",
-              "from": "mime@~1.2.9",
+              "from": "mime@~1.2.11",
               "resolved": "https://registry.npmjs.org/mime/-/mime-1.2.11.tgz"
             }
           }

package.json

@@ -24,7 +24,7 @@
     "dependencies": {
         "underscore"  : "~1.6.0",
         "dot": "~1.0.2",
-        "windshaft": "0.44.1",
+        "windshaft": "https://github.com/CartoDB/Windshaft/tarball/issue-351",
         "step": "~0.0.5",
         "queue-async": "~1.0.7",
         "request": "~2.9.203",

test/acceptance/limits.js

@@ -174,7 +174,7 @@ describe('render limits', function() {
                             },
                             function(res) {
                                 var parsed = JSON.parse(res.body);
-                                assert.deepEqual(parsed, { error: 'Render timed out' });
+                                assert.deepEqual(parsed, { errors: ['Render timed out'] });
                                 done();
                             }
                         );

test/acceptance/multilayer.js

@@ -131,7 +131,7 @@ suite(suiteName, function() {
           }, {}, function(res) {
               assert.equal(res.statusCode, 403, res.statusCode + ':' + res.body);
               var parsed = JSON.parse(res.body);
-              var msg = parsed.error; // TODO: should it be "errors" ?
+              var msg = parsed.errors[0];
               assert.ok(msg.match(/permission denied/i), msg);
               next(err);
           });

test/acceptance/templates.js

@@ -88,8 +88,8 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 403);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'), res.body);
-          err = parsed.error;
+          assert.ok(parsed.hasOwnProperty('errors'), res.body);
+          err = parsed.errors[0];
           assert.ok(err.match(/only.*authenticated.*user/i),
             'Unexpected error response: ' + err);
           post_request_1.url += '?api_key=1234';
@@ -113,9 +113,9 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
-          assert.ok(parsedBody.error.match(/already exists/i),
-            'Unexpected error for pre-existing template name: ' + parsedBody.error);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
+          assert.ok(parsedBody.errors[0].match(/already exists/i),
+            'Unexpected error for pre-existing template name: ' + parsedBody.errors);
           return null;
         },
         function finish(err) {
@@ -172,10 +172,10 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
           var re = /invalid.*authentication.*missing/i;
-          assert.ok(parsedBody.error.match(re),
-            'Error for invalid authentication does not match ' + re + ': ' + parsedBody.error);
+          assert.ok(parsedBody.errors[0].match(re),
+            'Error for invalid authentication does not match ' + re + ': ' + parsedBody.errors);
           return null;
         },
         function postTemplate2(err)
@@ -202,10 +202,10 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
           var re = new RegExp(/invalid.*authentication.*missing/i);
-          assert.ok(parsedBody.error.match(re),
-            'Error for invalid authentication does not match ' + re + ': ' + parsedBody.error);
+          assert.ok(parsedBody.errors[0].match(re),
+            'Error for invalid authentication does not match ' + re + ': ' + parsedBody.errors);
           return null;
         },
         function postTemplateValid(err)
@@ -253,12 +253,12 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.statusCode + ": " + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
           var re = /invalid.*authentication.*missing/i;
-          assert.ok(parsed.error.match(re),
+          assert.ok(parsed.errors[0].match(re),
             'Error for invalid authentication on PUT does not match ' +
-            re + ': ' + parsed.error);
+            re + ': ' + parsed.errors);
           var del_request = {
               url: '/api/v1/map/named/' + tpl_id + '?api_key=1234',
               method: 'DELETE',
@@ -425,9 +425,9 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 403, res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
+          assert.ok(parsed.hasOwnProperty('errors'),
             'Missing error from response: ' + res.body);
-          err = parsed.error;
+          err = parsed.errors[0];
           assert.ok(err.match(/authenticated user/), err);
           var next = this;
           var get_request = {
@@ -523,9 +523,9 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.statusCode + ": " + res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
-          assert.ok(parsedBody.error.match(/cannot update name/i),
-            'Unexpected error for invalid update: ' + parsedBody.error);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
+          assert.ok(parsedBody.errors[0].match(/cannot update name/i),
+            'Unexpected error for invalid update: ' + parsedBody.errors);
           var put_request = {
               url: '/api/v1/map/named/unexistent/?api_key=1234',
               method: 'PUT',
@@ -541,9 +541,9 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 400, res.statusCode + ": " + res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
-          assert.ok(parsedBody.error.match(/cannot update name/i),
-            'Unexpected error for invalid update: ' + parsedBody.error);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
+          assert.ok(parsedBody.errors[0].match(/cannot update name/i),
+            'Unexpected error for invalid update: ' + parsedBody.errors);
           var put_request = {
               url: '/api/v1/map/named/' + tpl_id + '/?api_key=1234',
               method: 'PUT',
@@ -630,9 +630,9 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 403, res.statusCode + ": " + res.body);
           var parsedBody = JSON.parse(res.body);
-          assert.ok(parsedBody.hasOwnProperty('error'), res.body);
-          assert.ok(parsedBody.error.match(/only.*authenticated.*user/i),
-            'Unexpected error for unauthenticated template get: ' + parsedBody.error);
+          assert.ok(parsedBody.hasOwnProperty('errors'), res.body);
+          assert.ok(parsedBody.errors[0].match(/only.*authenticated.*user/i),
+            'Unexpected error for unauthenticated template get: ' + parsedBody.errors);
           var get_request = {
               url: '/api/v1/map/named/' + tpl_id + '?api_key=1234',
               method: 'GET',
@@ -735,10 +735,10 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 403, res.statusCode + ": " + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/only.*authenticated.*user/i),
-            'Unexpected error for unauthenticated template get: ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/only.*authenticated.*user/i),
+            'Unexpected error for unauthenticated template get: ' + parsed.errors);
           var del_request = {
               url: '/api/v1/map/named/' + tpl_id + '?api_key=1234',
               method: 'DELETE',
@@ -767,10 +767,10 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 404, res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/cannot find/i),
-            'Unexpected error for missing template: ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/cannot find/i),
+            'Unexpected error for missing template: ' + parsed.errors);
           return null;
         },
         function finish(err) {
@@ -863,10 +863,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/unauthorized/i),
-            'Unexpected error for unauthorized instance : ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/unauthorized/i),
+            'Unexpected error for unauthorized instance : ' + parsed.errors);
           var post_request = {
               url: '/api/v1/map/named/' + tpl_id + '?auth_token=valid2',
               method: 'POST',
@@ -882,8 +882,8 @@ describe('template_api', function() {
           if ( err ) throw err;
           assert.equal(res.statusCode, 404, res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'), "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/not found/i), 'Unexpected error for forbidden instance : ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'), "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/not found/i), 'Unexpected error for forbidden instance : ' + parsed.errors);
           var post_request = {
               url: '/api/v1/map/named/' + tpl_id + '?auth_token=valid2',
               method: 'POST',
@@ -922,10 +922,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/permission denied/i),
-            'Unexpected error for unauthorized instance (expected /permission denied/): ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/permission denied/i),
+            'Unexpected error for unauthorized instance (expected /permission denied/): ' + parsed.errors);
           var get_request = {
               url: '/api/v1/map/' + layergroupid + '/0/0/0.png?auth_token=valid1',
               method: 'GET',
@@ -962,10 +962,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403, 
             'Unexpected error for authorized instance: ' + res.statusCode + ' -- ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/cannot use/i),
-            'Unexpected error for unauthorized instance (expected /cannot use/): ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/cannot use/i),
+            'Unexpected error for unauthorized instance (expected /cannot use/): ' + parsed.errors);
           return null;
         },
         function deleteTemplate(err)
@@ -1089,10 +1089,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/unauthorized/i),
-            'Unexpected error for unauthorized instance : ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/unauthorized/i),
+            'Unexpected error for unauthorized instance : ' + parsed.errors);
           var post_request = {
               url: '/api/v1/map/named/' + tpl_id + '?auth_token=valid2',
               method: 'POST',
@@ -1131,10 +1131,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/permission denied/i),
-            'Unexpected error for unauthorized instance (expected /permission denied): ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/permission denied/i),
+            'Unexpected error for unauthorized instance (expected /permission denied): ' + parsed.errors);
           var get_request = {
               url: '/api/v1/map/' + layergroupid + ':cb1/0/0/0/0.json.torque?auth_token=valid1',
               method: 'GET',
@@ -1297,10 +1297,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Unexpected success instanciating template with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/unauthorized/i),
-            'Unexpected error for unauthorized instance : ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/unauthorized/i),
+            'Unexpected error for unauthorized instance : ' + parsed.errors);
           var post_request = {
               url: '/api/v1/map/named/' + tpl_id + '?auth_token=valid2',
               method: 'POST',
@@ -1339,10 +1339,10 @@ describe('template_api', function() {
           assert.equal(res.statusCode, 403,
             'Fetching tile with no auth: ' + res.statusCode + ': ' + res.body);
           var parsed = JSON.parse(res.body);
-          assert.ok(parsed.hasOwnProperty('error'),
-            "Missing 'error' from response body: " + res.body);
-          assert.ok(parsed.error.match(/permission denied/i),
-            'Unexpected error for unauthorized getAttributes (expected /permission denied/): ' + parsed.error);
+          assert.ok(parsed.hasOwnProperty('errors'),
+            "Missing 'errors' from response body: " + res.body);
+          assert.ok(parsed.errors[0].match(/permission denied/i),
+            'Unexpected error for unauthorized getAttributes (expected /permission denied/): ' + parsed.errors);
           var get_request = {
               url: '/api/v1/map/' + layergroupid + ':cb1/0/attributes/5?auth_token=valid2',
               method: 'GET',
@@ -1608,7 +1608,6 @@ describe('template_api', function() {
         function checkInstanciation(err, res)
         {
           if ( err ) throw err;
-            console.log(err, res.body, res.headers);
           assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body);
           // See https://github.com/CartoDB/Windshaft-cartodb/issues/176
           helper.checkCache(res);
@@ -2072,7 +2071,7 @@ describe('template_api', function() {
                     return done(err);
                 }
                 assert.deepEqual(JSON.parse(res.body), {
-                    error: "Invalid or nonexistent map configuration token '" + nonexistentToken + "'"
+                    errors: ["Invalid or nonexistent map configuration token '" + nonexistentToken + "'"]
                 });
 
                 done();