TritonDataCenter / node-verror

Rich JavaScript errors
MIT License
1.18k stars 61 forks source link

Inheriting information properties is suboptimal #51

Closed JayChandler closed 6 years ago

JayChandler commented 6 years ago

This simple example shows the problem. I know that this is expected behaviour but when i want to nest VError instances i don't want the VError.Info to collapse the properties and overwrite them. When i call VError.info( err2 ); i would expect to get exactly the properties frm err2 and not the whole error chain.

The current behaviour does not allow to have propeties with the same name because they all would be collaped!

Why is there not a flag like

let info = VError.info( err2, false )

to prevent recursing into the error chain or something lile

let info = VError.flatInfo( err2)

which only returns the instance info properties

var err1 = new VError({
    'name': 'Err1',
    'info': {
        'errno': 'ERR_FIRST'
    }
}, 'First error');

var err2 = new VError({
    'name': 'Err2',
    'cause': err2,
    'info': {
        'errno': 'ERR_SECOND'
    }
}, 'Second error');

let info = VError.info( err2 );

console.log(info);
davepacheco commented 6 years ago

As you mentioned, this is all by design. This is a guiding principle (quote from the README):

In order for all this to work, programmers need to know that it's generally safe to wrap lower-level Errors with higher-level ones. If you have existing code that handles Errors produced by a library, you should be able to wrap those Errors with a VError to add information without breaking the error handling code. There are two obvious ways that this could break such consumers: ... The error's properties might change.

A prototypical use-case for nested informational properties is making an HTTP request and getting ECONNREFUSED. The socket library would attach the "remoteIp" to the error, and the HTTP agent would attach "remoteHostname". That way, the caller gets both. Critically, the HTTP library shouldn't have to know all of the useful properties that the socket library might add and explicitly propagate them.

As you point out, this does create a namespace problem on the informational properties; however, I think that results directly from the design goal of wanting to be able to wrap errors transparently (i.e., without consumers having to know about the whole cause chain and the properties on each one), and I think it's a worthwhile tradeoff. I deal with this by using prefixes for the properties provided by a library, which is helpful for other reasons, too.

The highest-level property shadows any lower-level ones. I think your proposed "flat" version would not change the values of any properties present -- it would only hide some.