Skip to content

Commit

Permalink
Use new Skipper onBodyParserError option instead of separate `handl…
Browse files Browse the repository at this point in the history
…eBodyParserError` middleware
  • Loading branch information
sgress454 committed Nov 15, 2016
1 parent 69110d2 commit 701ed99
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 21 deletions.
28 changes: 9 additions & 19 deletions lib/hooks/http/get-configured-http-middleware-fns.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,15 @@ module.exports = function getBuiltInHttpMiddleware (expressRouterMiddleware, sai
var opts = {};
var fn;

opts.onBodyParserError = function (err, req, res, next) {
var bodyParserFailureErrorMsg = 'Unable to parse HTTP body- error occurred :: ' + util.inspect((err&&err.stack)?err.stack:err, false, null);
sails.log.error(bodyParserFailureErrorMsg);
if (IS_NODE_ENV_PRODUCTION && sails.config.keepResponseErrors !== true) {
return res.status(400).send();
}
return res.status(400).send(bodyParserFailureErrorMsg);
};

// Handle original bodyParser config:
////////////////////////////////////////////////////////
// If a body parser was configured, use it
Expand All @@ -178,25 +187,6 @@ module.exports = function getBuiltInHttpMiddleware (expressRouterMiddleware, sai

})(),

// Should be installed immediately after the bodyParser- prevents bubbling to
// the default error handler which will attempt to use body parameters, which may
// cause unexpected issues.
//
// (included here so it still protects against this edge case if bodyParser
// is overridden in userland. Should probably be phased out at some point,
// since it could be accomplished more elegantly- but for now it's practical.)
//
// * TODO: fold this into Skipper itself, perhaps as an "errorHandlerFn" option.
handleBodyParserError: function handleBodyParserError(err, req, res, next) {
var bodyParserFailureErrorMsg = 'Unable to parse HTTP body- error occurred :: ' + util.inspect((err&&err.stack)?err.stack:err, false, null);
sails.log.error(bodyParserFailureErrorMsg);
if (IS_NODE_ENV_PRODUCTION && sails.config.keepResponseErrors !== true) {
return res.status(400).send();
}
return res.status(400).send(bodyParserFailureErrorMsg);
},


// Allow simulation of PUT and DELETE HTTP methods for user agents
// which don't support it natively (looks for a `_method` param)
methodOverride: (function() {
Expand Down
1 change: 0 additions & 1 deletion lib/hooks/http/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ module.exports = function(sails) {
'cookieParser',
'session',
'bodyParser',
'handleBodyParserError',
'compress',
'methodOverride',
'poweredBy',
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
"semver": "4.3.6",
"serve-favicon": "2.3.0",
"serve-static": "1.10.2",
"skipper": "~0.6.0",
"skipper": "~0.7.0",
"sort-route-addresses": "^0.0.1",
"uid-safe": "1.1.0",
"vary": "1.1.0",
Expand Down

2 comments on commit 701ed99

@mikermcneil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sgress454 just checking: does skipper supply a reasonable default for this if left unspecified?

@sgress454
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mikermcneil if left unspecified, Skipper just does what it did before (calls next(err)) -- see sailshq/skipper@ac386f0.

Please sign in to comment.