From 78285348043b0aabaf40d62894c1891db26a2431 Mon Sep 17 00:00:00 2001 From: shimks Date: Thu, 31 May 2018 13:13:14 -0400 Subject: [PATCH] Allow safeFields to work with arrays Co-authored-by: Miroslav Bajtos --- lib/data-builder.js | 38 ++++++++----------- test/handler.test.js | 89 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 86 insertions(+), 41 deletions(-) diff --git a/lib/data-builder.js b/lib/data-builder.js index 47e3a29..be165a2 100644 --- a/lib/data-builder.js +++ b/lib/data-builder.js @@ -8,16 +8,18 @@ var cloneAllProperties = require('../lib/clone.js'); var httpStatus = require('http-status'); -module.exports = function buildResponseData(err, options) { +module.exports = buildResponseData; + +function buildResponseData(err, options) { // Debugging mode is disabled by default. When turned on (in dev), // all error properties (including) stack traces are sent in the response - var isDebugMode = options.debug; + const isDebugMode = options.debug; - if (Array.isArray(err) && isDebugMode) { - err = serializeArrayOfErrors(err); + if (Array.isArray(err)) { + return serializeArrayOfErrors(err, options); } - var data = Object.create(null); + const data = Object.create(null); fillStatusCode(data, err); if (typeof err !== 'object') { @@ -29,35 +31,25 @@ module.exports = function buildResponseData(err, options) { if (isDebugMode) { fillDebugData(data, err); - } else if (data.statusCode >= 400 && data.statusCode <= 499) { + return data; + } + + if (data.statusCode >= 400 && data.statusCode <= 499) { fillBadRequestError(data, err); } else { fillInternalError(data, err); } - var safeFields = options.safeFields || []; + const safeFields = options.safeFields || []; fillSafeFields(data, err, safeFields); return data; }; -function serializeArrayOfErrors(errors) { - var details = []; - for (var ix in errors) { - var err = errors[ix]; - if (typeof err !== 'object') { - details.push('' + err); - continue; - } - - var data = {}; - cloneAllProperties(data, err); - delete data.statusCode; - details.push(data); - } - +function serializeArrayOfErrors(errors, options) { + const details = errors.map(e => buildResponseData(e, options)); return { - name: 'ArrayOfErrors', + statusCode: 500, message: 'Failed with multiple errors, ' + 'see `details` for more information.', details: details, diff --git a/test/handler.test.js b/test/handler.test.js index e89e775..653f7c3 100644 --- a/test/handler.test.js +++ b/test/handler.test.js @@ -397,48 +397,101 @@ describe('strong-error-handler', function() { requestJson().expect(500).end(function(err, res) { if (err) return done(err); - expect(res.body).to.eql({ - error: { - statusCode: 500, - message: 'Internal Server Error', - }, - }); + const data = res.body.error; + expect(data).to.have.property('message').that.match(/multiple errors/); + expect(data).to.have.property('details').eql([ + {statusCode: 500, message: 'Internal Server Error'}, + {statusCode: 500, message: 'Internal Server Error'}, + {statusCode: 500, message: 'Internal Server Error'}, + ]); done(); }); }); it('returns all array items when debug=true', function(done) { - var testError = new ErrorWithProps({ + const testError = new ErrorWithProps({ message: 'expected test error', statusCode: 400, }); - var anotherError = new ErrorWithProps({ + const anotherError = new ErrorWithProps({ message: 'another expected error', statusCode: 500, }); - var errors = [testError, anotherError, 'ERR STRING']; + const errors = [testError, anotherError, 'ERR STRING']; givenErrorHandlerForError(errors, {debug: true}); requestJson().expect(500).end(function(err, res) { if (err) return done(err); - var data = res.body.error; + const data = res.body.error; expect(data).to.have.property('message').that.match(/multiple errors/); - var expectTestError = getExpectedErrorData(testError); - delete expectTestError.statusCode; - var expectAnotherError = getExpectedErrorData(anotherError); - delete expectAnotherError.statusCode; - var expectedDetails = [ - expectTestError, - expectAnotherError, - 'ERR STRING', + const expectedDetails = [ + getExpectedErrorData(testError), + getExpectedErrorData(anotherError), + {message: 'ERR STRING', statusCode: 500}, ]; expect(data).to.have.property('details').to.eql(expectedDetails); done(); }); }); + it('includes safeFields of array items when debug=false', (done) => { + const internalError = new ErrorWithProps({ + message: 'a test error message', + code: 'MACHINE_READABLE_CODE', + details: 'some details', + extra: 'sensitive data', + }); + const validationError = new ErrorWithProps({ + name: 'ValidationError', + message: 'The model instance is not valid.', + statusCode: 422, + code: 'VALIDATION_ERROR', + details: 'some details', + extra: 'sensitive data', + }); + + const errors = [internalError, validationError, 'ERR STRING']; + givenErrorHandlerForError(errors, { + debug: false, + safeFields: ['code'], + }); + + requestJson().end(function(err, res) { + if (err) return done(err); + const data = res.body.error; + + const expectedInternalError = { + statusCode: 500, + message: 'Internal Server Error', + code: 'MACHINE_READABLE_CODE', + // notice the property "extra" is not included + }; + const expectedValidationError = { + statusCode: 422, + message: 'The model instance is not valid.', + name: 'ValidationError', + code: 'VALIDATION_ERROR', + details: 'some details', + // notice the property "extra" is not included + }; + const expectedErrorFromString = { + message: 'Internal Server Error', + statusCode: 500, + }; + const expectedDetails = [ + expectedInternalError, + expectedValidationError, + expectedErrorFromString, + ]; + + expect(data).to.have.property('message').that.match(/multiple errors/); + expect(data).to.have.property('details').to.eql(expectedDetails); + done(); + }); + }); + it('handles non-Error argument as 500 when debug=false', function(done) { givenErrorHandlerForError('Error Message', {debug: false}); requestJson().expect(500).end(function(err, res) {