From e362fca9eb9e18ea999c1f3acbd28c9b083555b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 24 Nov 2017 17:52:26 +0100 Subject: [PATCH 01/23] adding new header --- config/environments/development.js.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 91cf2a39..d58c8a97 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: false - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal From 84fd01535c6e198311feda9a8724bf1f8be0aca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 24 Nov 2017 17:53:07 +0100 Subject: [PATCH 02/23] adding errors to errors header --- lib/cartodb/middleware/error-middleware.js | 37 +++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 80457b9c..f372648d 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -18,7 +18,8 @@ module.exports = function errorMiddleware (/* options */) { if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { statusCode = 204; } - + + logErrors(allErrors, statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 @@ -31,6 +32,7 @@ module.exports = function errorMiddleware (/* options */) { errors_with_context: allErrors.map(errorMessageWithContext) }; + res.status(statusCode); if (req.query && req.query.callback) { @@ -160,3 +162,36 @@ function errorMessageWithContext(err) { return error; } + +function logErrors(errors, statusCode, res) { + console.log(' -----------------------------') + console.log('logErrors'); + console.log(' -----------------------------') + + if(!errors || !errors.length || !statusCode) { + return; + } + + const mainError = errors.shift(); + + const errorsLog = { + statusCode, + message: mainError.message, + type: mainError.type, + subtype: mainError.subtype + } + + errorsLog.moreErrors = errors.map(error => { + return { + message: error.message, + type: error.type, + subtype: error.subtype + }; + }); + + console.log(' -----------------------------') + console.log(errorsLog) + console.log(' -----------------------------') + + res.set('X-Tiler-Errors', JSON.stringify(errorsLog)); +} \ No newline at end of file From f24217a400650e07793347140ac56a86cb99296c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 24 Nov 2017 18:06:17 +0100 Subject: [PATCH 03/23] cloning object and removing logs --- lib/cartodb/middleware/error-middleware.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index f372648d..5c20179a 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -19,7 +19,7 @@ module.exports = function errorMiddleware (/* options */) { statusCode = 204; } - logErrors(allErrors, statusCode, res); + logErrors(Object.assign({}, allErrors), statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 @@ -164,10 +164,6 @@ function errorMessageWithContext(err) { } function logErrors(errors, statusCode, res) { - console.log(' -----------------------------') - console.log('logErrors'); - console.log(' -----------------------------') - if(!errors || !errors.length || !statusCode) { return; } @@ -179,7 +175,7 @@ function logErrors(errors, statusCode, res) { message: mainError.message, type: mainError.type, subtype: mainError.subtype - } + }; errorsLog.moreErrors = errors.map(error => { return { @@ -189,9 +185,5 @@ function logErrors(errors, statusCode, res) { }; }); - console.log(' -----------------------------') - console.log(errorsLog) - console.log(' -----------------------------') - res.set('X-Tiler-Errors', JSON.stringify(errorsLog)); } \ No newline at end of file From 667925c455333a36946de885395293afe458e103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 16:43:04 +0100 Subject: [PATCH 04/23] adding error name, ensuring data and moving errors copy --- lib/cartodb/middleware/error-middleware.js | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 5c20179a..52501e5d 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -15,11 +15,12 @@ module.exports = function errorMiddleware (/* options */) { var statusCode = findStatusCode(err); + logErrors(allErrors, statusCode, res); + if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { statusCode = 204; } - logErrors(Object.assign({}, allErrors), statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 @@ -164,22 +165,26 @@ function errorMessageWithContext(err) { } function logErrors(errors, statusCode, res) { - if(!errors || !errors.length || !statusCode) { + const errorsCopy = Object.assign({}, errors); + + if(!errorsCopy || !errorsCopy.length) { return; } - const mainError = errors.shift(); + const mainError = errorsCopy.shift(); const errorsLog = { - statusCode, + statusCode: statusCode || 200, message: mainError.message, + name: mainError.name, type: mainError.type, subtype: mainError.subtype }; - errorsLog.moreErrors = errors.map(error => { + errorsLog.moreErrors = errorsCopy.map(error => { return { message: error.message, + name: error.name, type: error.type, subtype: error.subtype }; From 9a8f72b8db191f9d42535a41df9cee503ac43b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 16:47:45 +0100 Subject: [PATCH 05/23] format details --- lib/cartodb/middleware/error-middleware.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 52501e5d..a17af805 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -20,7 +20,7 @@ module.exports = function errorMiddleware (/* options */) { if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { statusCode = 204; } - + debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 @@ -33,7 +33,6 @@ module.exports = function errorMiddleware (/* options */) { errors_with_context: allErrors.map(errorMessageWithContext) }; - res.status(statusCode); if (req.query && req.query.callback) { @@ -191,4 +190,4 @@ function logErrors(errors, statusCode, res) { }); res.set('X-Tiler-Errors', JSON.stringify(errorsLog)); -} \ No newline at end of file +} From 4a2950796bbf8f0c0aded97f14fbd0577908f34a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 16:48:25 +0100 Subject: [PATCH 06/23] rest of environments config --- config/environments/production.js.example | 2 +- config/environments/staging.js.example | 2 +- config/environments/test.js.example | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 7d7a00cb..4e59bde4 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: true - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 3d1098fb..8a9138b8 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: true - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms (:res[X-Tiler-Profiler]) -> :res[Content-Type]' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms (:res[X-Tiler-Profiler]) -> :res[Content-Type] !!!:res[X-Tiler-Errors]' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal diff --git a/config/environments/test.js.example b/config/environments/test.js.example index ea788a22..fdd301bb 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -54,7 +54,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: false - ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler])' + ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal From e041b5b8a97ac4cd658123a01d57738f4ee576d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 16:52:19 +0100 Subject: [PATCH 07/23] removing ~lost space --- lib/cartodb/middleware/error-middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index a17af805..9cd77313 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -20,7 +20,7 @@ module.exports = function errorMiddleware (/* options */) { if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { statusCode = 204; } - + debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 From e7b8d9b22365e177798d12bf37559767abcb7660 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 16:55:11 +0100 Subject: [PATCH 08/23] moving logErrors to right position --- lib/cartodb/middleware/error-middleware.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 9cd77313..37807d0c 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -15,12 +15,11 @@ module.exports = function errorMiddleware (/* options */) { var statusCode = findStatusCode(err); - logErrors(allErrors, statusCode, res); - if (err.message === 'Tile does not exist' && res.locals.format === 'mvt') { statusCode = 204; } - + + logErrors(allErrors, statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 From 60e4defa662dfc7eabd75d5980e56f9389ea80c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 17:04:50 +0100 Subject: [PATCH 09/23] default value in errors header --- lib/cartodb/server.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 08ec1a66..3849417a 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -409,6 +409,12 @@ function setupLogger(app, opts) { }; app.use(global.log4js.connectLogger(global.log4js.getLogger(), _.defaults(loggerOpts, {level: 'info'}))); } + + // default X-Tiler-Errors header + app.use((req, res, next) => { + res.set('X-Tiler-Errors', '{}'); + next(); + }); } function surrogateKeysCacheBackends(serverOptions) { From 605d7057c9893ea7b68ef5992bbc10b4126c5232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 18:12:44 +0100 Subject: [PATCH 10/23] fix copying array of errors and adding error.label to logs --- lib/cartodb/middleware/error-middleware.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 37807d0c..aac05eb4 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -163,18 +163,19 @@ function errorMessageWithContext(err) { } function logErrors(errors, statusCode, res) { - const errorsCopy = Object.assign({}, errors); - + const errorsCopy = errors.slice(0); + if(!errorsCopy || !errorsCopy.length) { return; } - + const mainError = errorsCopy.shift(); const errorsLog = { statusCode: statusCode || 200, message: mainError.message, name: mainError.name, + label: mainError.label, type: mainError.type, subtype: mainError.subtype }; @@ -183,10 +184,11 @@ function logErrors(errors, statusCode, res) { return { message: error.message, name: error.name, + label: error.label, type: error.type, subtype: error.subtype }; }); - + res.set('X-Tiler-Errors', JSON.stringify(errorsLog)); } From 8cf878f72393f35ba80966cf7a8d7c2cfacddbbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 18:14:02 +0100 Subject: [PATCH 11/23] testing X-Tiler-Errors existence --- test/unit/cartodb/base_controller.js | 36 +++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index 9bf21ccd..f9e6f178 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -3,7 +3,7 @@ require('../../support/test_helper.js'); var assert = require('assert'); var errorMiddleware = require('../../../lib/cartodb/middleware/error-middleware'); -describe('error-middleware', function() { +describe.only('error-middleware', function() { it('different formats for postgis plugin error returns 400 as status code', function() { @@ -20,4 +20,38 @@ describe('error-middleware', function() { "Error status code for multiline/PSQL does not match" ); }); + + it('should return a header with errors', function (done) { + const error = new Error('error test'); + + const req = {}; + const res = { + headers: {}, + set (key, value) { + this.headers[key] = value; + }, + statusCode: 0, + status (status) { + this.statusCode = status; + }, + json () {}, + send () {} + }; + + const errorHeader = { + statusCode: 400, + message: error.message, + name: error.name, + moreErrors: [] + }; + + const errorFn = errorMiddleware(); + errorFn(error, req, res); + + assert.deepEqual(res.headers, { + 'X-Tiler-Errors': JSON.stringify(errorHeader) + }); + + done(); + }); }); From 752bfe779eef4b1c496ea7cd51c830bacf0c8f39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 18:15:27 +0100 Subject: [PATCH 12/23] forgotten 'only' --- test/unit/cartodb/base_controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index f9e6f178..ca0547e3 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -3,7 +3,7 @@ require('../../support/test_helper.js'); var assert = require('assert'); var errorMiddleware = require('../../../lib/cartodb/middleware/error-middleware'); -describe.only('error-middleware', function() { +describe('error-middleware', function() { it('different formats for postgis plugin error returns 400 as status code', function() { From 100a2986b957d27378c73463bedf7eaa7389aeb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Mon, 27 Nov 2017 18:43:48 +0100 Subject: [PATCH 13/23] ensuring all properties in errors headers --- test/unit/cartodb/base_controller.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index ca0547e3..b61406c3 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -23,7 +23,12 @@ describe('error-middleware', function() { it('should return a header with errors', function (done) { const error = new Error('error test'); + error.label = 'test label'; + error.type = 'test type'; + error.subtype = 'test subtype'; + const errors = [error, error]; + const req = {}; const res = { headers: {}, @@ -42,11 +47,20 @@ describe('error-middleware', function() { statusCode: 400, message: error.message, name: error.name, - moreErrors: [] + label: error.label, + type: error.type, + subtype: error.subtype, + moreErrors: [{ + message: error.message, + name: error.name, + label: error.label, + type: error.type, + subtype: error.subtype + }] }; const errorFn = errorMiddleware(); - errorFn(error, req, res); + errorFn(errors, req, res); assert.deepEqual(res.headers, { 'X-Tiler-Errors': JSON.stringify(errorHeader) From a007fce9130f2cfe825979fb475a7b8cebc60bc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 16:02:12 +0100 Subject: [PATCH 14/23] ensuring vars --- lib/cartodb/middleware/error-middleware.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index aac05eb4..ce44daa3 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -163,7 +163,7 @@ function errorMessageWithContext(err) { } function logErrors(errors, statusCode, res) { - const errorsCopy = errors.slice(0); + let errorsCopy = errors.slice(0); if(!errorsCopy || !errorsCopy.length) { return; @@ -171,7 +171,7 @@ function logErrors(errors, statusCode, res) { const mainError = errorsCopy.shift(); - const errorsLog = { + let errorsLog = { statusCode: statusCode || 200, message: mainError.message, name: mainError.name, From 479b8be639ebd4ae3c63ab1b99ca1edddf66cb80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 17:27:05 +0100 Subject: [PATCH 15/23] ensuring errored JSONP write a error status code in log --- test/unit/cartodb/base_controller.js | 50 ++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index b61406c3..5c3e4092 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -68,4 +68,54 @@ describe('error-middleware', function() { done(); }); + + it('JSONP should return a header with error status code', function (done) { + const error = new Error('error test'); + error.label = 'test label'; + error.type = 'test type'; + error.subtype = 'test subtype'; + + const errors = [error, error]; + + const req = { + query: { callback: true } + }; + const res = { + headers: {}, + set (key, value) { + this.headers[key] = value; + }, + statusCode: 0, + status (status) { + this.statusCode = status; + }, + jsonp () {}, + send () {} + }; + + const errorHeader = { + statusCode: 400, + message: error.message, + name: error.name, + label: error.label, + type: error.type, + subtype: error.subtype, + moreErrors: [{ + message: error.message, + name: error.name, + label: error.label, + type: error.type, + subtype: error.subtype + }] + }; + + const errorFn = errorMiddleware(); + errorFn(errors, req, res); + + assert.deepEqual(res.headers, { + 'X-Tiler-Errors': JSON.stringify(errorHeader) + }); + + done(); + }); }); From 386d6bfea846f8be54260c6b9681e00b2c48e89a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 18:19:28 +0100 Subject: [PATCH 16/23] removing unneeded check --- lib/cartodb/middleware/error-middleware.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index ce44daa3..fd9892ac 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -164,11 +164,6 @@ function errorMessageWithContext(err) { function logErrors(errors, statusCode, res) { let errorsCopy = errors.slice(0); - - if(!errorsCopy || !errorsCopy.length) { - return; - } - const mainError = errorsCopy.shift(); let errorsLog = { From 555d3f558c52aa005b74960a4a8536bebff57447 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 18:22:55 +0100 Subject: [PATCH 17/23] changing error log structure --- lib/cartodb/middleware/error-middleware.js | 14 ++++++----- test/unit/cartodb/base_controller.js | 28 ++++++++++++---------- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index fd9892ac..7de64d68 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -167,12 +167,14 @@ function logErrors(errors, statusCode, res) { const mainError = errorsCopy.shift(); let errorsLog = { - statusCode: statusCode || 200, - message: mainError.message, - name: mainError.name, - label: mainError.label, - type: mainError.type, - subtype: mainError.subtype + mainError: { + statusCode: statusCode || 200, + message: mainError.message, + name: mainError.name, + label: mainError.label, + type: mainError.type, + subtype: mainError.subtype + } }; errorsLog.moreErrors = errorsCopy.map(error => { diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/base_controller.js index 5c3e4092..d8d11421 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/base_controller.js @@ -44,12 +44,14 @@ describe('error-middleware', function() { }; const errorHeader = { - statusCode: 400, - message: error.message, - name: error.name, - label: error.label, - type: error.type, - subtype: error.subtype, + mainError: { + statusCode: 400, + message: error.message, + name: error.name, + label: error.label, + type: error.type, + subtype: error.subtype, + }, moreErrors: [{ message: error.message, name: error.name, @@ -94,12 +96,14 @@ describe('error-middleware', function() { }; const errorHeader = { - statusCode: 400, - message: error.message, - name: error.name, - label: error.label, - type: error.type, - subtype: error.subtype, + mainError: { + statusCode: 400, + message: error.message, + name: error.name, + label: error.label, + type: error.type, + subtype: error.subtype, + }, moreErrors: [{ message: error.message, name: error.name, From e0d4a9e596f07d9991d92ff8a68f701190d6ff17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 30 Nov 2017 15:04:07 +0100 Subject: [PATCH 18/23] change funcion name --- lib/cartodb/middleware/error-middleware.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/middleware/error-middleware.js b/lib/cartodb/middleware/error-middleware.js index 7de64d68..f3f2eb36 100644 --- a/lib/cartodb/middleware/error-middleware.js +++ b/lib/cartodb/middleware/error-middleware.js @@ -19,7 +19,7 @@ module.exports = function errorMiddleware (/* options */) { statusCode = 204; } - logErrors(allErrors, statusCode, res); + setErrorHeader(allErrors, statusCode, res); debug('[%s ERROR] -- %d: %s, %s', label, statusCode, err, err.stack); // If a callback was requested, force status to 200 @@ -162,7 +162,7 @@ function errorMessageWithContext(err) { return error; } -function logErrors(errors, statusCode, res) { +function setErrorHeader(errors, statusCode, res) { let errorsCopy = errors.slice(0); const mainError = errorsCopy.shift(); From ba3af551e3c9e9e8942a51a6ed840edf8cc56789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 30 Nov 2017 15:04:38 +0100 Subject: [PATCH 19/23] update test file name --- .../cartodb/{base_controller.js => error-middleware.test.js} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename test/unit/cartodb/{base_controller.js => error-middleware.test.js} (98%) diff --git a/test/unit/cartodb/base_controller.js b/test/unit/cartodb/error-middleware.test.js similarity index 98% rename from test/unit/cartodb/base_controller.js rename to test/unit/cartodb/error-middleware.test.js index d8d11421..4e235c4a 100644 --- a/test/unit/cartodb/base_controller.js +++ b/test/unit/cartodb/error-middleware.test.js @@ -43,7 +43,7 @@ describe('error-middleware', function() { send () {} }; - const errorHeader = { + const errorHeader = { mainError: { statusCode: 400, message: error.message, From ed51513b5ecbf42796aea8f1edaa17f37de406ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 1 Dec 2017 17:52:20 +0100 Subject: [PATCH 20/23] adding error header acceptance test --- test/acceptance/error-middleware.js | 40 +++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 test/acceptance/error-middleware.js diff --git a/test/acceptance/error-middleware.js b/test/acceptance/error-middleware.js new file mode 100644 index 00000000..de1e6309 --- /dev/null +++ b/test/acceptance/error-middleware.js @@ -0,0 +1,40 @@ +const assert = require('../support/assert'); +const TestClient = require('../support/test-client'); + +describe('error middleware', function () { + it('should returns a errors header', function (done) { + const mapConfig = { + version: '1.6.0', + layers: [{ + type: 'mapnik', + options: {} + }] + }; + + const errorHeader = { + mainError: { + statusCode: 400, + message: "Missing cartocss for layer 0 options", + name: "Error", + label: "ANONYMOUS LAYERGROUP", + type: "layer", + }, + moreErrors: [] + }; + + this.testClient = new TestClient(mapConfig, 1234); + + const expectedResponse = { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8', + 'X-Tiler-Errors': JSON.stringify(errorHeader) + } + }; + + this.testClient.getLayergroup(expectedResponse, (err) => { + assert.ifError(err); + done(); + }); + }); +}); \ No newline at end of file From b3d790984921e3ee41ac4583b50d69318b851bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 11:03:52 +0100 Subject: [PATCH 21/23] removing default error log value --- lib/cartodb/server.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/cartodb/server.js b/lib/cartodb/server.js index 3849417a..08ec1a66 100644 --- a/lib/cartodb/server.js +++ b/lib/cartodb/server.js @@ -409,12 +409,6 @@ function setupLogger(app, opts) { }; app.use(global.log4js.connectLogger(global.log4js.getLogger(), _.defaults(loggerOpts, {level: 'info'}))); } - - // default X-Tiler-Errors header - app.use((req, res, next) => { - res.set('X-Tiler-Errors', '{}'); - next(); - }); } function surrogateKeysCacheBackends(serverOptions) { From 2db2546ccac1b6fbac65c42ed770e8a8d840fd69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 11:04:06 +0100 Subject: [PATCH 22/23] changing error log format --- config/environments/development.js.example | 2 +- config/environments/production.js.example | 2 +- config/environments/staging.js.example | 2 +- config/environments/test.js.example | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/environments/development.js.example b/config/environments/development.js.example index d58c8a97..55cda5ba 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: false - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) (:res[X-Tiler-Errors])' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 4e59bde4..3904d46e 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: true - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) (:res[X-Tiler-Errors])' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 8a9138b8..15031991 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -55,7 +55,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: true - ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms (:res[X-Tiler-Profiler]) -> :res[Content-Type] !!!:res[X-Tiler-Errors]' + ,log_format: ':req[X-Real-IP] :method :req[Host]:url :status :response-time ms (:res[X-Tiler-Profiler]) -> :res[Content-Type] (:res[X-Tiler-Errors])' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal diff --git a/config/environments/test.js.example b/config/environments/test.js.example index fdd301bb..43ebf844 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -54,7 +54,7 @@ var config = { ,socket_timeout: 600000 ,enable_cors: true ,cache_enabled: false - ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) !!!:res[X-Tiler-Errors]' + ,log_format: '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-Tiler-Profiler]) (:res[X-Tiler-Errors])' // If log_filename is given logs will be written // there, in append mode. Otherwise stdout is used (default). // Log file will be re-opened on receiving the HUP signal From 19bb11adc5fd3ba30d980e80d73e281155077c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 16:59:07 +0100 Subject: [PATCH 23/23] line at EOF --- test/acceptance/error-middleware.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/acceptance/error-middleware.js b/test/acceptance/error-middleware.js index de1e6309..3ad22774 100644 --- a/test/acceptance/error-middleware.js +++ b/test/acceptance/error-middleware.js @@ -37,4 +37,4 @@ describe('error middleware', function () { done(); }); }); -}); \ No newline at end of file +});