Proposal: suppress stack traces in HTTP responses when running in production

[bajtos]

LoopBack users are asking for an option to suppress stack traces when running in production - see strongloop/loopback#564, strongloop/loopback#1502 and strongloop/strong-remoting#87. So far, we have implemented this feature at our side, but think it would be nice to upstream that changes and make this feature available to all errorhandler users.

I can see two rules for deciding whether the stack should be included or not:

• remove stack when NODE_ENV=production, keep it otherwise
• remove stack when options.includeStack is false, keep it otherwise

Perhaps we can implement both of them?

• if options.includeStack is not defined and NODE_ENV=production -> remove the stack
• else if options.includeStack is false -> remove the stack
• else keep the stack in

Thoughts?

[dougwilson]

Hi! Did you perhaps post this issue on the wrong module?

The first sentence on the readme is

Development-only error handler middleware

And the example usage already shows how to turn off stack traces in production...

[dougwilson]

Unless, perhaps your proposal isn't just "add an option to disable stack traces" (which, that proposal is denied, as this is a development-only middleware you should not have in your production stack at all for more reasons than just the stack trace), but rather "let's rewrite this module to be a production-ready module", which entails a lot more than adding a single switch.

[dougwilson]

I would be willing to rewrite this module to be production-ready, but it's hard for me to promise any kind of timeline currently.

[dougwilson]

I've had a change of heart in the drive over, just for you guys, I guess. Though no users should ever be loading this middleware in their production environments, I will add an option to control the display of the stack traces. The NODE_ENV=production rule will not be added, however, for the following reason:

1. If people are using his module in production for the last couple years, then they wanted the stack traces there. Adding this magic is not backwards-compatible for them.
2. In general, the expressjs org's module are moving away from doing magic based on NODE_ENV, as discussed in previous expressjs meetings :)

[bajtos]

Hi @dougwilson, thank you for the comments.

Did you perhaps post this issue on the wrong module?

The first sentence on the readme is

Development-only error handler middleware

Ha, I did not noticed that! So what's the recommended error handling middleware to use in production? One that can return JSON-formatted error responses?

And the example usage already shows how to turn off stack traces in production...

Please correct me if I am wrong, but the example "Custom output location" shows how to redirect logs, but does not show how to return error responses in production as a JSON object without the stack property.

The NODE_ENV=production rule will not be added

That's fine 👍

---

Let's rephrase my problem. I am looking for an error-handling middleware that:

• supports at least HTML and JSON response formats, plain-text and XML are nice to have
• has a configuration option to control whether stack traces are included in the message
• optionally can log the error via console.error too

Is there any existing project you can recommend for that?

[dougwilson]

There are so many projects on npm, it's hard to say ;) but no, I'm not aware of other projects. The main reason this is not reasonable to use in production is because it uses HTML users cannot change, with a title that's hard to change, and inline CSS, which violates most people's CSP.

Typically, for people to get a good error-handling experience, they have to change so much in this module, they are literally just writing the module themselves in their own code:

app.use(function (err, req, res, next) {
  // do all my error logic, like loading my error view templates,
  // providing json payloads that match the format i designed on my api,
  // and more
})

Please correct me if I am wrong, but the example "Custom output location" shows how to redirect logs, but does not show how to return error responses in production as a JSON object without the stack property.

I'm referring to the "Simple example", which, if followed, will not display stack traces in production.

[dougwilson]

So I was just working on this and there is another problem: because this is a development-only module, if a user passes down a non-error object, this module will actually call util.inspect on it, revealing a lot of internal details, which is similar is nature to a stack trace.

Obviously, adding an option to turn on/off stack traces will not help with this information leak, and with that as a key feature of this module, I'm not sure what should be done.

What is your opinion? This means that, even if there was a switch for stack traces, it still is not suitable for use in production environments.

[dougwilson]

This means that the PR strongloop/loopback#1502 is only a false sense of security; yes, the stack property will no longer be included, but it will still expose anything util.inspect will provide for non-Error objects.

[dougwilson]

So what I'll do is add two new options: one to toggle stack traces and one to toggle the util.inspect behavior. To make this module even close to being usable in a production environment, you'll need to set both to false. But even if you did that, there still remains a big problem with using this in production: the err.message will be exposed still, which will contain a full path if that err came from fs.readFile, or a hostname if that came from a net operation in the newer io.js builds, etc.

In reality, only the finalhandler module provides suitable out-of-the-box production error support (and you get that by... not adding an error handler to your express app, which is what the example in this module's README does). You can also customize finalhandler or similar modules by providing the callback as the optional third argument to your app() call.

[dougwilson]

Anyway, I hope you understand all the different reasons why simply adding the ability to disable stack traces will not, in any way, make this module suitable for running in a production environment, due to the multitude of ways for information leaks to occurs. I plead to you, do not ever use this module in production, as this is very dangerous. The purpose of this module is to run in development while developing to get insight into the errors that are occurring.

Changing this module to be more suitable for a production environment is a large task fo completely re-writing/re-imaging this module and, once done, where will the development people do to get a similar module to this?

I highly suggest you rely on finalhandler (the error handling that is built-in to Express) manager errors if you don't have something specific you want to achieve (which, at that point, you'll want to make your own customer error handler).

If you desire some kind of default JSON error handler, perhaps use https://github.com/expressjs/api-error-handler or any other module that is in the https://github.com/expressjs organization. You're also welcome to create your own "default" error handlers and published them to npm :)

[dougwilson]

(Sorry, didn't realize I accidentally closed this issue on you)

[bajtos]

What is your opinion? This means that, even if there was a switch for stack traces, it still is not suitable for use in production environments.

Hmm, I did not realise the consequences of sending non-Error errors.

I agree with your conclusion in #13 (comment), thank you very much for this thorough analysis. I created a LoopBack issue strongloop/loopback#1650 to discuss this further in the scope of LoopBack project.

I think this issue can be closed now, thank you again for your help!

[bajtos]

Hi @dougwilson, I spent some time today thinking about an ideal error handler for LoopBack (possibly for any Express app). If you can spare some time, could you please review my proposal described here? https://hackpad.com/ExpressLB-Error-Handler-for-production-2wOxkScSQw8