Skip to content

Commit

Permalink
Update default responses
Browse files Browse the repository at this point in the history
* Denormalize response code by removing `build-outlet-function` factory.  This just makes the response code clearer.
* No more use of `jsonx` -- when `res.json()` is used, errors are turned into stack traces first unless they have a `.toJSON()`
* Only `badRequest` and `serverError` can send output
* No more view guessing / second arguments
* Default 500 handler uses `serverError` instead of `negotiate`
  • Loading branch information
sgress454 committed Nov 29, 2016
1 parent cdcc3c0 commit 510504e
Show file tree
Hide file tree
Showing 8 changed files with 203 additions and 148 deletions.
57 changes: 47 additions & 10 deletions lib/hooks/responses/defaults/badRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies
*/

var buildOutletFunction = require('../helpers/build-outlet-function');
var _ = require('@sailshq/lodash');



Expand All @@ -23,16 +23,53 @@ var buildOutletFunction = require('../helpers/build-outlet-function');
* ```
*/

module.exports = function badRequest(data, options) {
module.exports = function badRequest(data) {

var config = {
logMessage: 'Sending 400 ("Bad Request") response',
statusCode: 400,
logData: true,
isError: true,
isGuessView: true
};
// Get access to `req` and `res`
var req = this.req;
var res = this.res;

buildOutletFunction(this.req, this.res, data, options, config);
// Get access to `sails`
var sails = req._sails;

// Log error to console
if (data !== undefined) {
sails.log.verbose('Sending 400 ("Bad Request") response: \n', data);
}

// Set status code
res.status(400);

// If appropriate, serve data as JSON.
if (req.wantsJSON) {
// If the data is an error instance and it doesn't have a custom .toJSON(),
// use its stack instead (otherwise res.json() will turn it into an empty dictionary).
if (_.isError(data)) {
if (!_.isFunction(data.toJSON)) {
data = data.stack;
}
}
return res.json(data);
}

return res.view('400', { data: data }, function (err, html) {

// If a view error occured, fall back to JSON.
if (err) {
//
// Additionally:
// • If the view was missing, ignore the error but provide a verbose log.
if (err.code === 'E_VIEW_FAILED') {
sails.log.verbose('res.badRequest() :: Could not locate view for error page (sending JSON instead). Details: ', err);
}
// Otherwise, if this was a more serious error, log to the console with the details.
else {
sails.log.warn('res.badRequest() :: When attempting to render error page view, an error occured (sending JSON instead). Details: ', err);
}
return res.json(data);
}

return res.send(html);
});

};
50 changes: 37 additions & 13 deletions lib/hooks/responses/defaults/forbidden.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies
*/

var buildOutletFunction = require('../helpers/build-outlet-function');
var _ = require('@sailshq/lodash');



Expand All @@ -20,17 +20,41 @@ var buildOutletFunction = require('../helpers/build-outlet-function');
* ```
*/

module.exports = function forbidden (data, options) {

var config = {
logMessage: 'Sending 403 ("Forbidden") response',
statusCode: 403,
logData: true,
isError: true,
isGuessView: false,
name: 'forbidden'
};

buildOutletFunction(this.req, this.res, data, options, config);
module.exports = function forbidden () {

This comment has been minimized.

Copy link
@mikermcneil

// Get access to `req` and `res`
var req = this.req;
var res = this.res;

// Get access to `sails`
var sails = req._sails;

// Set status code
res.status(403);

// If appropriate, serve data as JSON.

This comment has been minimized.

Copy link
@mikermcneil
if (req.wantsJSON) {
return res.send();
}

return res.view('403', function (err, html) {

// If a view error occured, fall back to JSON.
if (err) {
//
// Additionally:
// • If the view was missing, ignore the error but provide a verbose log.
if (err.code === 'E_VIEW_FAILED') {
sails.log.verbose('res.forbidden() :: Could not locate view for error page (sending text instead). Details: ', err);
}
// Otherwise, if this was a more serious error, log to the console with the details.
else {
sails.log.warn('res.forbidden() :: When attempting to render error page view, an error occured (sending text instead). Details: ', err);
}
return res.send('FORBIDDEN');

This comment has been minimized.

Copy link
@mikermcneil
}

return res.send(html);
});

};
46 changes: 35 additions & 11 deletions lib/hooks/responses/defaults/notFound.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies
*/

var buildOutletFunction = require('../helpers/build-outlet-function');
var _ = require('@sailshq/lodash');



Expand All @@ -25,17 +25,41 @@ var buildOutletFunction = require('../helpers/build-outlet-function');
* automatically.
*/

module.exports = function notFound (data, options) {
module.exports = function notFound (data) {

This comment has been minimized.

Copy link
@mikermcneil

mikermcneil Nov 30, 2016

Member

@sgress454 lets get rid of both arguments here


var config = {
logMessage: 'Sending 404 ("Not Found") response',
statusCode: 404,
logData: true,
isError: true,
isGuessView: false,
name: 'notFound'
};
// Get access to `req` and `res`
var req = this.req;
var res = this.res;

buildOutletFunction(this.req, this.res, data, options, config);
// Get access to `sails`
var sails = req._sails;

// Set status code
res.status(404);

// If appropriate, serve data as JSON.

This comment has been minimized.

Copy link
@mikermcneil

mikermcneil Nov 30, 2016

Member

@sgress454

// If the request wants JSON, send back the appropriate status code.
if (req.wantsJSON) {
return res.send();
}

return res.view('404', function (err, html) {

// If a view error occured, fall back to JSON.
if (err) {
//
// Additionally:
// • If the view was missing, ignore the error but provide a verbose log.
if (err.code === 'E_VIEW_FAILED') {
sails.log.verbose('res.notFound() :: Could not locate view for error page (sending text instead). Details: ', err);
}
// Otherwise, if this was a more serious error, log to the console with the details.
else {
sails.log.warn('res.notFound() :: When attempting to render error page view, an error occured (sending text instead). Details: ', err);
}
return res.send('NOT FOUND');

This comment has been minimized.

Copy link
@mikermcneil

mikermcneil Nov 30, 2016

Member

@sgress454 and this can just be res.send() (it'll use Express's built-in thing, like above)

}

return res.send(html);
});

};
31 changes: 20 additions & 11 deletions lib/hooks/responses/defaults/ok.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies
*/

var buildOutletFunction = require('../helpers/build-outlet-function');
var _ = require('@sailshq/lodash');



Expand All @@ -19,17 +19,26 @@ var buildOutletFunction = require('../helpers/build-outlet-function');
* - pass string to render specified view
*/

module.exports = function sendOK (data, options) {
module.exports = function sendOK (data) {

var config = {
logMethod: 'silly',
logMessage: 'res.ok() :: Sending 200 ("OK") response',
statusCode: 200,
logData: false,
isError: false,
isGuessView: true
};
// Get access to `req` and `res`
var req = this.req;
var res = this.res;

buildOutletFunction(this.req, this.res, data, options, config);
// Get access to `sails`
var sails = req._sails;

// Set status code
res.status(200);

// If the data is an error instance and it doesn't have a custom .toJSON(),
// use its stack instead (otherwise res.json() will turn it into an empty dictionary).
if (_.isError(data)) {
if (!_.isFunction(data.toJSON)) {
data = data.stack;
}
}

return res.json(data);

};
64 changes: 52 additions & 12 deletions lib/hooks/responses/defaults/serverError.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Module dependencies
*/

var buildOutletFunction = require('../helpers/build-outlet-function');
var _ = require('@sailshq/lodash');



Expand All @@ -20,18 +20,58 @@ var buildOutletFunction = require('../helpers/build-outlet-function');
* automatically.
*/

module.exports = function serverError (data, options) {
module.exports = function serverError (data) {

var config = {
logMethod: 'error',
logMessage: 'Sending 500 ("Server Error") response',
statusCode: 500,
logData: true,
isError: true,
isGuessView: false,
name: 'serverError'
};
// Get access to `req` and `res`
var req = this.req;
var res = this.res;

buildOutletFunction(this.req, this.res, data, options, config);
// Get access to `sails`
var sails = req._sails;

// Log error to console
if (data !== undefined) {
sails.log.error('Sending 500 ("Server Error") response: \n', data);
}

// Don't output error data with response in production.
if (sails.config.environment === 'production' && sails.config.keepResponseErrors !== true) {
data = undefined;
}

// Set status code
res.status(500);

// If appropriate, serve data as JSON.
if (req.wantsJSON) {
// If the data is an error instance and it doesn't have a custom .toJSON(),
// use its stack instead (otherwise res.json() will turn it into an empty dictionary).
if (_.isError(data)) {
if (!_.isFunction(data.toJSON)) {
data = data.stack;
}
}
return res.json(data);
}

return res.view('500', { error: data }, function (err, html) {

// If a view error occured, fall back to JSON.
if (err) {
//
// Additionally:
// • If the view was missing, ignore the error but provide a verbose log.
if (err.code === 'E_VIEW_FAILED') {
sails.log.verbose('res.serverError() :: Could not locate view for error page (sending JSON instead). Details: ', err);
}
// Otherwise, if this was a more serious error, log to the console with the details.
else {
sails.log.warn('res.serverError() :: When attempting to render error page view, an error occured (sending JSON instead). Details: ', err);
}
return res.json(data);
}

return res.send(html);
});

};
Loading

0 comments on commit 510504e

Please sign in to comment.