From 3e23be2087794e865b3131e54834b2bf33b2658a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 15:57:58 +0100 Subject: [PATCH 01/12] add errors header with default value --- app/server.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/server.js b/app/server.js index 4f4ecbbc..83ebcef6 100644 --- a/app/server.js +++ b/app/server.js @@ -100,6 +100,12 @@ function App(statsClient) { app.use(global.log4js.connectLogger(global.log4js.getLogger(), _.defaults(loggerOpts, {level:'info'}))); } + // default X-SQLAPI-Errors header + app.use((req, res, next) => { + res.set('X-SQLAPI-Errors', '{}'); + next(); + }); + app.use(cors()); // Use step-profiler From a17e1fc5ec08f693ce1252a628e7b8e283269158 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 15:58:14 +0100 Subject: [PATCH 02/12] logErrors function --- app/utils/error_handler.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index 9d46def5..57364f43 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -32,6 +32,8 @@ module.exports = function handleException(err, res) { res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); } + logErrors(msg, pgErrorHandler.getStatus(), res); + res.header('Content-Type', 'application/json; charset=utf-8'); res.status(getStatusError(pgErrorHandler, req)); if (req.query && req.query.callback) { @@ -56,3 +58,13 @@ function getStatusError(pgErrorHandler, req) { return statusError; } + +function logErrors(err, statusCode, res) { + let errorsLog = Object.assign({}, err); + + errorsLog.statusCode = statusCode || 200; + errorsLog.message = err.error; + delete err.error; + + res.set('X-SQLAPI-Errors', JSON.stringify(errorsLog)); +} From 4d598eacd7729f8049c867ff900ce3797729c332 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 16:11:04 +0100 Subject: [PATCH 03/12] dont modify err, works with errorsLog --- app/utils/error_handler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index 57364f43..127d6a78 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -63,8 +63,8 @@ function logErrors(err, statusCode, res) { let errorsLog = Object.assign({}, err); errorsLog.statusCode = statusCode || 200; - errorsLog.message = err.error; - delete err.error; + errorsLog.message = errorsLog.error; + delete errorsLog.error; res.set('X-SQLAPI-Errors', JSON.stringify(errorsLog)); } From 0ab87df644aa7db2691bd522ceeeeb49b8209fea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 17:19:10 +0100 Subject: [PATCH 04/12] extract error message from array --- app/utils/error_handler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index 127d6a78..a9aa43c0 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -63,7 +63,7 @@ function logErrors(err, statusCode, res) { let errorsLog = Object.assign({}, err); errorsLog.statusCode = statusCode || 200; - errorsLog.message = errorsLog.error; + errorsLog.message = errorsLog.error[0]; delete errorsLog.error; res.set('X-SQLAPI-Errors', JSON.stringify(errorsLog)); From 43598cafe6f764406e2470a71041c444e15a4c44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 17:19:48 +0100 Subject: [PATCH 05/12] test ensuring errors header --- test/unit/error_handler.test.js | 91 +++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 test/unit/error_handler.test.js diff --git a/test/unit/error_handler.test.js b/test/unit/error_handler.test.js new file mode 100644 index 00000000..cc3e0934 --- /dev/null +++ b/test/unit/error_handler.test.js @@ -0,0 +1,91 @@ +'use strict'; + +var assert = require('assert'); +var errorHandler = require('../../app/utils/error_handler'); + +describe('error-handler', function() { + it('should return a header with errors', function (done) { + const error = new Error('error test'); + error.detail = 'test detail'; + error.hint = 'test hint'; + error.context = 'test context'; + + const res = { + req: {}, + headers: {}, + set (key, value) { + this.headers[key] = value; + }, + header (key, value) { + this.set(key, value); + }, + statusCode: 0, + status (status) { + this.statusCode = status; + }, + json () {} + }; + + const errorHeader = { + detail: error.detail, + hint: error.hint, + context: error.context, + statusCode: 400, + message: error.message + }; + + errorHandler(error, res); + + assert.ok(res.headers['X-SQLAPI-Errors'].length > 0); + assert.deepEqual( + res.headers['X-SQLAPI-Errors'], + JSON.stringify(errorHeader) + ); + + done(); + }); + + it('JSONP should return a header with error statuscode', function (done) { + const error = new Error('error test'); + error.detail = 'test detail'; + error.hint = 'test hint'; + error.context = 'test context'; + + const res = { + req: { + query: { callback: true } + }, + headers: {}, + set (key, value) { + this.headers[key] = value; + }, + header (key, value) { + this.set(key, value); + }, + statusCode: 0, + status (status) { + this.statusCode = status; + }, + jsonp () {} + }; + + const errorHeader = { + detail: error.detail, + hint: error.hint, + context: error.context, + statusCode: 400, + message: error.message + }; + + errorHandler(error, res); + + assert.ok(res.headers['X-SQLAPI-Errors'].length > 0); + assert.deepEqual( + res.headers['X-SQLAPI-Errors'], + JSON.stringify(errorHeader) + ); + + done(); + }); + +}); From 1eae2e4790aced2d5374231ce4c189420f094f7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 28 Nov 2017 17:32:29 +0100 Subject: [PATCH 06/12] adding error header to environment files --- 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 43f3588e..bdc498f2 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler])'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 21726fc1..2dd1084d 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler])'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 06f0ad77..508e7fd0 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler])'; +module.exports.log_format = '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/test.js.example b/config/environments/test.js.example index 3f0fcf4d..e1aa2cf3 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler])'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-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 // module.exports.log_filename = 'logs/cartodb-sql-api.log'; From 61b351535c8dccf101bd344646dd7563e0f8036b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Thu, 30 Nov 2017 15:06:25 +0100 Subject: [PATCH 07/12] change funcion name --- app/utils/error_handler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index a9aa43c0..5aa8203a 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -32,7 +32,7 @@ module.exports = function handleException(err, res) { res.header('X-SQLAPI-Profiler', req.profiler.toJSONString()); } - logErrors(msg, pgErrorHandler.getStatus(), res); + setErrorHeader(msg, pgErrorHandler.getStatus(), res); res.header('Content-Type', 'application/json; charset=utf-8'); res.status(getStatusError(pgErrorHandler, req)); @@ -59,7 +59,7 @@ function getStatusError(pgErrorHandler, req) { return statusError; } -function logErrors(err, statusCode, res) { +function setErrorHeader(err, statusCode, res) { let errorsLog = Object.assign({}, err); errorsLog.statusCode = statusCode || 200; From 15428979d1459ab1c7ee9ef991ed7c96d76f14f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Fri, 1 Dec 2017 18:11:32 +0100 Subject: [PATCH 08/12] adding error header acceptance test --- test/acceptance/error-handler.js | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 test/acceptance/error-handler.js diff --git a/test/acceptance/error-handler.js b/test/acceptance/error-handler.js new file mode 100644 index 00000000..9a8e5329 --- /dev/null +++ b/test/acceptance/error-handler.js @@ -0,0 +1,31 @@ +var server = require('../../app/server')(); +var assert = require('../support/assert'); + +describe('error handler', function () { + it('should returns a errors header', function (done) { + const errorHeader = { + detail: undefined, + hint: undefined, + context: undefined, + statusCode: 400, + message: 'You must indicate a sql query' + }; + + assert.response(server, { + url: '/api/v1/sql', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }, + { + status: 400, + headers: { + 'Content-Type': 'application/json; charset=utf-8', + 'X-SQLAPI-Errors': JSON.stringify(errorHeader) + } + }, + function(err){ + assert.ifError(err); + done(); + }); + }); +}); \ No newline at end of file From c478e8e62ec8059d103bb4b156f77dc5a8c1f458 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 10:55:28 +0100 Subject: [PATCH 09/12] removing default error log value --- app/server.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/app/server.js b/app/server.js index 83ebcef6..4f4ecbbc 100644 --- a/app/server.js +++ b/app/server.js @@ -100,12 +100,6 @@ function App(statsClient) { app.use(global.log4js.connectLogger(global.log4js.getLogger(), _.defaults(loggerOpts, {level:'info'}))); } - // default X-SQLAPI-Errors header - app.use((req, res, next) => { - res.set('X-SQLAPI-Errors', '{}'); - next(); - }); - app.use(cors()); // Use step-profiler From 8665caa3cb04db670677df129efe96b8451129e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 10:55:54 +0100 Subject: [PATCH 10/12] 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 bdc498f2..3ed23866 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-Errors]'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) (:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 2dd1084d..5684dafe 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-Errors]'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) (:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index 508e7fd0..f078858e 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-Errors]'; +module.exports.log_format = '[:date] :req[X-Real-IP] :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) (:res[X-SQLAPI-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 module.exports.log_filename = 'logs/cartodb-sql-api.log'; diff --git a/config/environments/test.js.example b/config/environments/test.js.example index e1aa2cf3..fd822920 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -8,7 +8,7 @@ module.exports.base_url = '(?:/api/:version|/user/:user/api/:version)'; // X-SQLAPI-Profile header containing elapsed timing for various // steps taken for producing the response. module.exports.useProfiler = true; -module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) !!!:res[X-SQLAPI-Errors]'; +module.exports.log_format = '[:date] :remote-addr :method :req[Host]:url :status :response-time ms -> :res[Content-Type] (:res[X-SQLAPI-Profiler]) (:res[X-SQLAPI-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 // module.exports.log_filename = 'logs/cartodb-sql-api.log'; From 6481d1419263392ed76d2cc3e861296fc656cfc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 12:58:31 +0100 Subject: [PATCH 11/12] escape quotes for logs --- app/utils/error_handler.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index 5aa8203a..663fda11 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -64,6 +64,8 @@ function setErrorHeader(err, statusCode, res) { errorsLog.statusCode = statusCode || 200; errorsLog.message = errorsLog.error[0]; + // escape quotes for logs + errorsLog.message = errorsLog.message.replace(/"/g, ""); delete errorsLog.error; res.set('X-SQLAPI-Errors', JSON.stringify(errorsLog)); From 3fa09a007ebbcad4411b4de8034021761dd11b2e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Tue, 12 Dec 2017 17:16:40 +0100 Subject: [PATCH 12/12] line at EOF and better comment --- app/utils/error_handler.js | 2 +- test/acceptance/error-handler.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/utils/error_handler.js b/app/utils/error_handler.js index 663fda11..25c86178 100644 --- a/app/utils/error_handler.js +++ b/app/utils/error_handler.js @@ -64,7 +64,7 @@ function setErrorHeader(err, statusCode, res) { errorsLog.statusCode = statusCode || 200; errorsLog.message = errorsLog.error[0]; - // escape quotes for logs + // remove double quotes for logs errorsLog.message = errorsLog.message.replace(/"/g, ""); delete errorsLog.error; diff --git a/test/acceptance/error-handler.js b/test/acceptance/error-handler.js index 9a8e5329..4a6ea563 100644 --- a/test/acceptance/error-handler.js +++ b/test/acceptance/error-handler.js @@ -28,4 +28,4 @@ describe('error handler', function () { done(); }); }); -}); \ No newline at end of file +});