From 8c305cb1645045b7828cc3a4fb89d8adae29352e Mon Sep 17 00:00:00 2001 From: Jeremy Trufier Date: Thu, 31 Aug 2017 14:30:15 +0200 Subject: [PATCH 1/3] feat(error debug) #125 - Return a debug key containing the stack when an unknown error occured if debug is enabled Signed-off-by: Jeremy Trufier --- README.md | 20 +++++++++++++++++ lib/errors.js | 18 +++++++++++---- test/errors.test.js | 53 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 84 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 04bf2eb..ac8819c 100644 --- a/README.md +++ b/README.md @@ -118,6 +118,7 @@ Example: "host": "https://www.mydomain.com", "enable": true, "handleErrors": true, + "errorStackInResponse": false, "exclude": [ {"model": "comment"}, {"methods": "find"}, @@ -201,6 +202,25 @@ out of the box with EmberJS. - Type: `boolean` - Default: `true` +### errorStackInResponse +Along handleErrors, When true, this option will send the error stack if available within the error +reponse. It will be stored under the `source.stack` key. + +**Be careful, this option should never be enabled in production environment. It can propagate some +sensitive datas.** + +#### example +```js +{ + ... + "errorStackInResponse": NODE_ENV === 'development', + ... +} +``` + +- Type: `boolean` +- Default: `false` + ### exclude Allows blacklisting of models and methods. Define an array of blacklist objects. Blacklist objects can contain "model" key diff --git a/lib/errors.js b/lib/errors.js index 78919bb..f60566e 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -1,10 +1,12 @@ 'use strict' var debug +var errorStackInResponse var statusCodes = require('http-status-codes') module.exports = function (app, options) { debug = options.debug + errorStackInResponse = options.errorStackInResponse if (options.handleErrors !== false) { debug( @@ -80,7 +82,7 @@ function JSONAPIErrorHandler (err, req, res, next) { } errors.push( - buildErrorResponse(statusCode, err.message, err.code, err.name) + buildErrorResponse(statusCode, err.message, err.code, err.name, null, err.stack) ) } else { debug( @@ -112,6 +114,7 @@ function JSONAPIErrorHandler (err, req, res, next) { * @param {String} errorCode internal system error code * @param {String} errorName error title for the user, human readable * @param {String} propertyName for validation errors, name of property validation refers to + * @param {String} errorStack Some debbuging informations * @return {Object} */ function buildErrorResponse ( @@ -119,13 +122,20 @@ function buildErrorResponse ( errorDetail, errorCode, errorName, - propertyName + propertyName, + errorStack ) { - return { + var out = { status: httpStatusCode || statusCodes.INTERNAL_SERVER_ERROR, - source: propertyName ? { pointer: 'data/attributes/' + propertyName } : '', + source: propertyName ? { pointer: 'data/attributes/' + propertyName } : {}, title: errorName || '', code: errorCode || '', detail: errorDetail || '' } + + if (errorStackInResponse && errorStack) { + out.source.stack = errorStack + } + + return out } diff --git a/test/errors.test.js b/test/errors.test.js index f9f93bc..97afcda 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -2,6 +2,7 @@ var request = require('supertest') var loopback = require('loopback') +var _ = require('lodash') var expect = require('chai').expect var JSONAPIComponent = require('../') var app @@ -24,7 +25,7 @@ describe('disabling loopback-component-jsonapi error handler', function () { request(app).get('/posts/100').end(function (err, res) { expect(err).to.equal(null) expect(res.body).to.have.keys('error') - expect(res.body.error).to.have.keys('name', 'message', 'statusCode') + expect(res.body.error).to.have.keys('name', 'message', 'statusCode', 'code') done() }) }) @@ -87,7 +88,7 @@ describe('loopback json api errors', function () { status: 404, code: 'MODEL_NOT_FOUND', detail: 'Unknown "post" id "100".', - source: '', + source: {}, title: 'Error' }) done() @@ -129,7 +130,7 @@ describe('loopback json api errors', function () { status: 422, code: 'presence', detail: 'JSON API resource object must contain `data.type` property', - source: '', + source: {}, title: 'ValidationError' }) done() @@ -178,3 +179,49 @@ describe('loopback json api errors', function () { ) }) }) + +describe('loopback json api errors with `errorStackInResponse` enabled', function () { + beforeEach(function () { + app = loopback() + app.set('legacyExplorer', false) + var ds = loopback.createDataSource('memory') + Post = ds.createModel('post', { + id: { type: Number, id: true }, + title: String, + content: String + }) + app.model(Post) + app.use(loopback.rest()) + JSONAPIComponent(app, { restApiRoot: '', errorStackInResponse: true }) + }) + + it( + 'POST /models should return a more specific 422 status code on the error object if type key is not present', + function (done) { + request(app) + .post('/posts') + .send({ + data: { + attributes: { title: 'my post', content: 'my post content' } + } + }) + .set('Content-Type', 'application/json') + .end(function (err, res) { + expect(err).to.equal(null) + expect(res.body).to.have.keys('errors') + expect(res.body.errors.length).to.equal(1) + + expect(res.body.errors[0].source).to.haveOwnProperty('stack') + expect(res.body.errors[0].source.stack.length).to.be.above(100) + + expect(_.omit(res.body.errors[0], 'source')).to.deep.equal({ + status: 422, + code: 'presence', + detail: 'JSON API resource object must contain `data.type` property', + title: 'ValidationError' + }) + done() + }) + } + ) +}) From c250d4cb6f20410f91449484c52b92c2f1e19e15 Mon Sep 17 00:00:00 2001 From: Jeremy Trufier Date: Mon, 13 Nov 2017 10:09:43 +0100 Subject: [PATCH 2/3] feat(errors): allow for custom `source` and `meta` properties in a error response Signed-off-by: Jeremy Trufier --- README.md | 39 +++++++++++++++++++++++++++++++++++++++ lib/errors.js | 35 ++++++++++++++++++++++++++--------- test/errors.test.js | 45 ++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 107 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ac8819c..766abc5 100644 --- a/README.md +++ b/README.md @@ -439,6 +439,45 @@ module.exports = function (MyModel) { } ``` +## Custom Errors +Generic errors respond with a 500, but sometime you want to have a better control over the error that is returned to the client, taking advantages of fields provided by JSONApi. + +**It is recommanded to extend the base Error constructor, before throwing errors, ex: BadRequestError** + +`meta` and `source` fields needs to be objects. + +#### example +```js +module.exports = function (MyModel) { + MyModel.find = function () { + var err = new Error('April 1st, 1998'); + + err.status = 418; + err.name = 'I\'m a teapot'; + err.source = { model: 'Post', method: 'find' }; + err.detail = 'April 1st, 1998'; + err.code = 'i\'m a teapot'; + err.meta = { rfc: 'RFC2324' }; + + throw err + } +} + +// This will be returned as : +// { +// errors: [ +// { +// status: 418, +// meta: { rfc: 'RFC2324' }, +// code: 'i\'m a teapot', +// detail: 'April 1st, 1998', +// title: 'I\'m a teapot', +// source: { model: 'Post', method: 'find' } +// } +// ] +// } +``` + ##### function parameters - `options` All config options set for the deserialization process. diff --git a/lib/errors.js b/lib/errors.js index f60566e..4699240 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -3,6 +3,7 @@ var debug var errorStackInResponse var statusCodes = require('http-status-codes') +var _ = require('lodash') module.exports = function (app, options) { debug = options.debug @@ -51,7 +52,7 @@ function JSONAPIErrorHandler (err, req, res, next) { err.details.messages[key][0], err.details.codes[key][0], err.name, - key + { pointer: 'data/attributes/' + key } ) }) } else if (err.message) { @@ -81,8 +82,24 @@ function JSONAPIErrorHandler (err, req, res, next) { err.name = 'BadRequest' } + var errorSource = err.source && typeof err.source === 'object' + ? err.source + : {} + if (errorStackInResponse) { + // We do not want to mutate err.source, so we clone it first + errorSource = _.clone(errorSource) + errorSource.stack = err.stack + } + errors.push( - buildErrorResponse(statusCode, err.message, err.code, err.name, null, err.stack) + buildErrorResponse( + statusCode, + err.message, + err.code, + err.name, + errorSource, + err.meta + ) ) } else { debug( @@ -113,8 +130,8 @@ function JSONAPIErrorHandler (err, req, res, next) { * @param {String} errorDetail error message for the user, human readable * @param {String} errorCode internal system error code * @param {String} errorName error title for the user, human readable - * @param {String} propertyName for validation errors, name of property validation refers to - * @param {String} errorStack Some debbuging informations + * @param {String} errorSource Some informations about the source of the issue + * @param {String} errorMeta Some custom metas informations to give to the error response * @return {Object} */ function buildErrorResponse ( @@ -122,19 +139,19 @@ function buildErrorResponse ( errorDetail, errorCode, errorName, - propertyName, - errorStack + errorSource, + errorMeta ) { var out = { status: httpStatusCode || statusCodes.INTERNAL_SERVER_ERROR, - source: propertyName ? { pointer: 'data/attributes/' + propertyName } : {}, + source: errorSource || {}, title: errorName || '', code: errorCode || '', detail: errorDetail || '' } - if (errorStackInResponse && errorStack) { - out.source.stack = errorStack + if (errorMeta && typeof errorMeta === 'object') { + out.meta = errorMeta } return out diff --git a/test/errors.test.js b/test/errors.test.js index 97afcda..6ec90d2 100644 --- a/test/errors.test.js +++ b/test/errors.test.js @@ -25,7 +25,7 @@ describe('disabling loopback-component-jsonapi error handler', function () { request(app).get('/posts/100').end(function (err, res) { expect(err).to.equal(null) expect(res.body).to.have.keys('error') - expect(res.body.error).to.have.keys('name', 'message', 'statusCode', 'code') + expect(res.body.error).to.contain.keys('name', 'message', 'statusCode') done() }) }) @@ -180,7 +180,16 @@ describe('loopback json api errors', function () { }) }) -describe('loopback json api errors with `errorStackInResponse` enabled', function () { +describe('loopback json api errors with advanced reporting', function () { + var errorMetaMock = { + status: 418, + meta: { rfc: 'RFC2324' }, + code: "i'm a teapot", + detail: 'April 1st, 1998', + title: "I'm a teapot", + source: { model: 'Post', method: 'find' } + } + beforeEach(function () { app = loopback() app.set('legacyExplorer', false) @@ -190,13 +199,43 @@ describe('loopback json api errors with `errorStackInResponse` enabled', functio title: String, content: String }) + + Post.find = function () { + var err = new Error(errorMetaMock.detail) + err.name = errorMetaMock.title + err.meta = errorMetaMock.meta + err.source = errorMetaMock.source + err.statusCode = errorMetaMock.status + err.code = errorMetaMock.code + throw err + } + app.model(Post) app.use(loopback.rest()) JSONAPIComponent(app, { restApiRoot: '', errorStackInResponse: true }) }) it( - 'POST /models should return a more specific 422 status code on the error object if type key is not present', + 'should return the given meta and source in the error response when an Error with a meta and source object is thrown', + function (done) { + request(app) + .get('/posts') + .set('Content-Type', 'application/json') + .end(function (err, res) { + expect(err).to.equal(null) + expect(res.body).to.have.keys('errors') + expect(res.body.errors.length).to.equal(1) + + expect(_.omit(res.body.errors[0], 'source.stack')).to.deep.equal( + errorMetaMock + ) + done() + }) + } + ) + + it( + 'should return the corresponding stack in error when `errorStackInResponse` enabled', function (done) { request(app) .post('/posts') From 94b7435ad84939e12cde69096e79b698039c7312 Mon Sep 17 00:00:00 2001 From: Jeremy Trufier Date: Thu, 16 Nov 2017 22:53:43 +0100 Subject: [PATCH 3/3] chore(wording) Fix grammatical issues in comments --- README.md | 9 ++++----- lib/errors.js | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 766abc5..039d3fe 100644 --- a/README.md +++ b/README.md @@ -204,10 +204,9 @@ out of the box with EmberJS. ### errorStackInResponse Along handleErrors, When true, this option will send the error stack if available within the error -reponse. It will be stored under the `source.stack` key. +response. It will be stored under the `source.stack` key. -**Be careful, this option should never be enabled in production environment. It can propagate some -sensitive datas.** +**Please be careful, this option should never be enabled in a production environment. Doing so can expose sensitive data.** #### example ```js @@ -440,9 +439,9 @@ module.exports = function (MyModel) { ``` ## Custom Errors -Generic errors respond with a 500, but sometime you want to have a better control over the error that is returned to the client, taking advantages of fields provided by JSONApi. +Generic errors respond with a 500, but sometimes you want to have a better control over the error that is returned to the client, taking advantages of fields provided by JSONApi. -**It is recommanded to extend the base Error constructor, before throwing errors, ex: BadRequestError** +**It is recommended that you extend the base Error constructor before throwing errors. Eg. BadRequestError** `meta` and `source` fields needs to be objects. diff --git a/lib/errors.js b/lib/errors.js index 4699240..d256972 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -130,8 +130,8 @@ function JSONAPIErrorHandler (err, req, res, next) { * @param {String} errorDetail error message for the user, human readable * @param {String} errorCode internal system error code * @param {String} errorName error title for the user, human readable - * @param {String} errorSource Some informations about the source of the issue - * @param {String} errorMeta Some custom metas informations to give to the error response + * @param {String} errorSource Some information about the source of the issue + * @param {String} errorMeta Some custom meta information to give to the error response * @return {Object} */ function buildErrorResponse (