Keep error data on production using an argument

[eric-burel]

Hello,

I have been struggling with the fact that sails preconfigured responses are deleting the data passed as argument in production. Indeed, I use this argument to pass custom error messages to the frontend (angular).
This default behaviour seems reasonnable to me as it prevents beginner mistakes and security flaws, yet it is a bit tricky for a beginner. Documentation might be clearer about this point (it is said explicitely but too quickly for someone that begins using sails).

Today the only solution consists in changing the keepResponseErrors config options. This is not a good solution as it is too global. To my mind, we should be able to set this argument on every call, using the "options" argument.
It would preserve the sane default behaviour of sails, yet allow to pass end-user error messages along with the error response easily.

Implementation is easy, I slightly changed the code of serverError.js, notFound.js, badRequest.js and notFound.js, adding a line and using a variable :
var keepResponseErrors = sails.config.keepResponseErrors || (options && options.keepResponseErrors); if (sails.config.environment === 'production' && !keepResponseErrors) { data = undefined; }

[mikermcneil]

@eric-burel agree completely, was actually just talking about this today.

What do you think about this:

Never send in production for res.serverError() or res.negotiate()

• default res.serverError() continues to prevent sending provided data in json responses or view locals in production (res.serverError() is overridable - just edit api/responses/serverError.js)
• built-in res.negotiate() prevents sending provided data in json responses or view locals in production (res.negotiate() is overridable - just create api/responses/negotiate.js)

Always send for res.ok(), res.forbidden(), res.badRequest(), res.notFound()

• default res.ok() alwys sends data in json responses or view locals, even in production (this function is overridable - just edit api/responses/forbidden.js)
• default res.forbidden() if called directly (i.e. not from res.negotiate()) always sends data in json responses or view locals, even in production. (this function is overridable - just edit api/responses/forbidden.js)
• default res.badRequest() if called directly (i.e. not from res.negotiate()) always sends data in json responses or view locals, even in production. (this function is overridable - just edit api/responses/badRequest.js)
• default res.notFound() if called directly (i.e. not from res.negotiate()) always sends data in json responses or view locals, even in production. (this function is overridable - just edit api/responses/notFound.js)

Default error handler in Sails calls res.serverError()

Rather than res.negotiate(), for v1, the default error handler in Sails calls res.serverError() directly. That would allow us to change res.negotiate() to no longer worry about preventing data getting into json responses or view locals, while still allowing it to be used by blueprints.

[eric-burel]

Thanks for your answer, I think that's a very good solution, better than passing an argument every time we want to keep response data in production.

[mikermcneil]

@eric-burel great to hear. If you wouldn't mind updating this PR to make the change to the "backlog" section of the roadmap (see https://github.com/balderdashy/sails/blob/master/CONTRIBUTING.md for more info) we can get this merged in

[eric-burel]

That's done :)

[sgress454]

Sorry for the delay, merging this in to backlog now -- I think some work might have been done on it already, too.

[mikermcneil]

This is now resolved in Sails v1! (see http://sailsjs.com/upgrading)