diff --git a/3.0-RELEASE-NOTES.md b/3.0-RELEASE-NOTES.md index 55865894..c3c0c1e3 100644 --- a/3.0-RELEASE-NOTES.md +++ b/3.0-RELEASE-NOTES.md @@ -27,3 +27,89 @@ To disable a static remote method To disable a prototype remote method: `disableMethodByName('prototype.updateAttributes')` + +## New error-handler for rest-adapter + +The REST adapter (typically created via `loopback.rest()`) uses a new error +handler implementation that provides more secure configuration out of the box. + +The `error.status` has been removed and will be replaced by `error.statusCode`, +`statusCode` is more descriptive and avoids ambiguity, users relying on `status` +will need to change implementation accordingly. + +To replicate old behavior user can specify `config.local.js` as follow: +```js +module.exports = { + remoting : { + errorHandler: { + handler : function(err, req, res, defaultHandler) { + err.status = err.statusCode; + defaultHandler(); + } + } + } +} +``` + +The environment setting `NODE_ENV='production'` is no longer supported, +production vs. debug mode is controlled exclusively by configuration. +Production environment is assumed by default, the insecure debug mode +must be explicitely turned on. + +You can learn more about the rationale behind the new handler in +[this comment](https://github.com/strongloop/loopback/issues/1650#issuecomment-161920555) + +User can specific options in their applications in `config.json` as follow: +```json +{ + "restApiRoot": "/api", + "host": "0.0.0.0", + "port": 3000, + "remoting": { + "errorHandler": { + "debug": false, + "log": false + } + } +} +``` + +#### Production mode + +Stack trace is never returned in HTTP responses. + +Bad Request errors (4xx) provide the following properties copied from the +error object: `name`, `message`, `statusCode` and `details`. + +All other errors (including non-Error values like strings or arrays) provide +only basic information: `statusCode` and `message` set to status name from HTTP +specification. + +#### Debug mode + +When in debug mode, HTTP responses include all properties provided by the error. + +For errors that are not an object, their string value is returned in +`message` field. + +When a method fails with an array of errors (e.g. bulk create), HTTP the response +contains still a single wrapper error with `details` set to the original array +of errors. + +An example of an error response when an array of errors was raised: + +```js +{ + error: { + statusCode: 500, + name: 'ArrayOfErrors', + message: 'Failed with multiple errors, see `details` for more information.', + details: [ + { name: 'Error1', message: 'expected error', statusCode: 500, stack: '' }, + { name: 'Error2', message: 'expected error2', statusCode: 500, stack: ''} + ] + } +} +``` + +Please see [Related code change](https://github.com/strongloop/strong-remoting/pull/302) here, and new [`strong-error-handler` here](https://github.com/strongloop/strong-error-handler/). diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index adc8ef95..1af8fa51 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -29,6 +29,7 @@ var async = require('async'); var HttpInvocation = require('./http-invocation'); var ContextBase = require('./context-base'); var HttpContext = require('./http-context'); +var strongErrorHandler = require('strong-error-handler'); var json = bodyParser.json; var urlencoded = bodyParser.urlencoded; @@ -333,7 +334,7 @@ RestAdapter.remoteMethodNotFoundHandler = function(className) { var message = 'Shared class "' + className + '"' + ' has no method handling ' + req.method + ' ' + req.url; var error = new Error(message); - error.status = error.statusCode = 404; + error.statusCode = 404; next(error); }; }; @@ -342,13 +343,21 @@ RestAdapter.urlNotFoundHandler = function() { return function restUrlNotFound(req, res, next) { var message = 'There is no method to handle ' + req.method + ' ' + req.url; var error = new Error(message); - error.status = error.statusCode = 404; + error.statusCode = 404; next(error); }; }; RestAdapter.errorHandler = function(options) { options = options || {}; + if (options.hasOwnProperty('disableStackTrace')) { + console.warn( + 'strong-remoting no longer supports "errorHandler.disableStackTrace" option. ' + + 'Use the new option "errorHandler.debug" instead.'); + } + + var strongErrorHandlerInstance = strongErrorHandler(options); + return function restErrorHandler(err, req, res, next) { if (typeof options.handler === 'function') { try { @@ -366,49 +375,7 @@ RestAdapter.errorHandler = function(options) { // the handler are reported err = handlerError; } - if (typeof err === 'string') { - err = new Error(err); - err.status = err.statusCode = 500; - } - - if (res.statusCode === undefined || res.statusCode < 400) { - res.statusCode = err.statusCode || err.status || 500; - } - - if (Array.isArray(err)) { - var details = err.map(function(it) { - var data = generateResponseError(it); - delete data.statusCode; - return data; - }); - - var msg = 'Failed with multiple errors, see `details` for more information.'; - err = new Error(msg); - err.details = details; - } - - res.send({ error: generateResponseError(err) }); - - function generateResponseError(error) { - debug('Error in %s %s: %s', req.method, req.url, error.stack); - - var data = { - name: error.name, - status: res.statusCode, - message: error.message || 'An unknown error occurred', - }; - - for (var prop in error) { - data[prop] = error[prop]; - } - - data.stack = error.stack; - if (process.env.NODE_ENV === 'production' || options.disableStackTrace) { - delete data.stack; - } - - return data; - } + return strongErrorHandlerInstance(err, req, res, next); } }; }; diff --git a/package.json b/package.json index d08ebbce..8802f33e 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "qs": "^2.4.2", "request": "^2.55.0", "sse": "0.0.6", + "strong-error-handler": "^1.0.0", "traverse": "^0.6.6", "xml2js": "^0.4.8" }, @@ -73,7 +74,8 @@ "express": false, "body-parser": false, "cors": false, - "js2xmlparser": false + "js2xmlparser": false, + "strong-error-handler": false }, "license": "Artistic-2.0" } diff --git a/test/rest.browser.test.js b/test/rest.browser.test.js index f30472f2..65ec5f27 100644 --- a/test/rest.browser.test.js +++ b/test/rest.browser.test.js @@ -286,6 +286,10 @@ describe('strong-remoting-rest', function() { }); describe('uncaught errors', function() { + beforeEach(function() { + var optsErrorHandler = { errorHandler: { debug: true, log: false }}; + extend(objects.options, optsErrorHandler); + }); it('should return 500 if an error object is thrown', function(done) { var errMsg = 'an error'; var method = givenSharedStaticMethod( diff --git a/test/rest.test.js b/test/rest.test.js index d0036b62..12e2590f 100644 --- a/test/rest.test.js +++ b/test/rest.test.js @@ -46,11 +46,8 @@ describe('strong-remoting-rest', function() { // setup beforeEach(function() { - if (process.env.NODE_ENV === 'production') { - process.env.NODE_ENV = 'test'; - } objects = RemoteObjects.create({ json: { limit: '1kb' }, - errorHandler: { disableStackTrace: false }}); + errorHandler: { debug: true, log: false }}); remotes = objects.exports; // connect to the app @@ -131,36 +128,18 @@ describe('strong-remoting-rest', function() { })); }); - it('should disable stack trace', function(done) { - objects.options.errorHandler.disableStackTrace = true; + it('should exclude stack traces by default', function(done) { var method = givenSharedStaticMethod( - function(cb) { - cb(new Error('test-error')); - } - ); + function(cb) { cb(new Error('test-error')); }); - // Send a plain, non-json request to make sure the error handler - // always returns a json response. - request(app).get(method.url) - .expect('Content-Type', /json/) - .expect(500) - .end(expectErrorResponseContaining({ message: 'test-error' }, ['stack'], done)); - }); + // reset the errorHandler options + objects.options.errorHandler = {}; - it('should disable stack trace', function(done) { - process.env.NODE_ENV = 'production'; - var method = givenSharedStaticMethod( - function(cb) { - cb(new Error('test-error')); - } - ); - - // Send a plain, non-json request to make sure the error handler - // always returns a json response. request(app).get(method.url) .expect('Content-Type', /json/) .expect(500) - .end(expectErrorResponseContaining({ message: 'test-error' }, ['stack'], done)); + .end(expectErrorResponseContaining( + { message: 'Internal Server Error' }, ['stack'], done)); }); it('should turn off url-not-found handler', function(done) { @@ -2072,7 +2051,7 @@ describe('strong-remoting-rest', function() { it('returns 404 with standard JSON body for unknown URL', function(done) { json('/unknown-url') .expect(404) - .end(expectErrorResponseContaining({ status: 404 }, done)); + .end(expectErrorResponseContaining({ statusCode: 404 }, done)); }); });