Add raw error to errorLogger?

[bithavoc]

Add raw error to errorLogger?

[rosston]

I don't have a PR ready for this yet, but here's what I plan to do:

• Add an errorWhitelist that works just like the requestWhitelist and responseWhitelist
• Add an errorFilter to the options that is used just like the requestFilter and responseFilter
• Put the result of the errorWhitelist properties being run through the errorFilter on an error property on the metadata object

[floatingLomas]

Alternatively, we could approach this by being able to supply an 'error transformer' function that would just transform a received error however anyone might want to transform it, with a default of just returning the given error.

[rosston]

I think that might be a clearer way. Something like error as a top-level property? And then there's a special handler that is provided the error object and can return whatever it wants to go on error?

The only tricky thing there is that the base error object in V8 has no toJSON method, no enumerable properties, and Object.getOwnPropertyNames returns ["stack", "message"], which excludes the (semi-important) name property. So if, say, the user decides to simply return the error as is, possibly nothing will show up. Or maybe we just always put the name and message properties on the error object?

I'm expecting to have time tomorrow to work on this specifically, so ideas are welcome!

[rosston]

After testing this error transform approach in a real application, I think it's a little too dangerous. I think libraries will sometimes throw errors with circular references (e.g., https://github.com/vpulim/node-soap), which causes winston to exceed the max call stack size when trying to serialize the error. In that case, a user of this module would be required to use the errorTransform if a library they use happens to throw errors with circular references.

I think it would be preferable to have safe defaults, so I'll probably open a new PR later with an errorWhitelist and errorFilter as described in #95 (comment).

[cbaclig]

@rosston Any updates on this? I'm happy to open a PR, just wanted to check if any prior work outside of #99 had been done.

I'm using VError, and I've recently run into a scenario where I'd like to use something like errorTransform to add info from VError.info(err) to my logs.

[rosston]

@cbaclig No updates/progress on this. It eventually ended up being less of an issue at my day job, so I never got around to it. You're welcome to work on it if you want. 😄

I think my suggested approach in #95 (comment) is probably the best way to go. I really prefer the idea of an errorTransform, but it seems too prone to developers accidentally introducing server crashes into their app by forgetting to remove circular-dependent properties. errorWhitelist + errorFilter provides a bit more safe-by-default functionality, I think.

[rosston]

Just realized json-stringify-safe exists, so forget what I said about circular references. We just need to use that module + errorTransform, and everything will be fine.

[cbaclig]

Given the addition of dynamicMeta in #124, instead of an errorTransform property, adding a dynamicMeta that effectively does the same thing seems like it'd accomplish the same thing and keep the interface more consistent between the "normal" logger and the errorLogger. The only difference would be that the function passed to dynamicMeta for the errorLogger would get an additional err parameter. e.g.

dynamicMeta: function(err, req, res) { return [Object]; } 

Thoughts?

[rosston]

I think I'm fine with this approach as a customization option, although err should probably be the last parameter to keep backward compatibility.

Is there any reason not to just always put the entire error (run through json-stringify-safe) in the metadata (on, say, a rawError key)?

[rosston]

Fixed in #139 (via dynamic meta in the error logger) and released in 2.2.0.