Provide getProperty method for VError and WError

[thinkkevin]

I find it useful that if VError and WError instance could get its parent(s)'s property value. A use case could be like

var request = require('request');
var VError = require('verror');

function getTestData(callback) {
  request.get('http://test.com/api', function(err, resp, body) {
    if(err){
      err.code = 'ERR:REQ:1';
    } else {
      if(resp && resp.statusCode != 200) {
        err = new Error('Request Failed');
        err.code = 'ERR:REQ:2';
      }
    }
    callback(err, body);
  });
}

function performTest(callback){
  getTestData(function(err, data) {
    if(err){
      callback(new VError(err, "Perform Test Failed"));
    }else{
      var testResult = {};
      callback(null, testResult);
    }
  });
}

performTest(function(err, results){
  if(err){
    console.log('Error occured with code %s', err.getProperty('code'));
  }
});

I have created a utility method like below, but I think it would be better to be part of the library

function getPropertyFromError(err, propertyName) {
  return err && (err[propertyName] ||
    (typeof err.cause === 'function'? getPropertyFromError(err.cause(), propertyName) : undefined));
};

[davepacheco]

I think it's useful to first-class the notion of properties, and we've been talking about it over on #10. I've got a branch there with a prototype that I'm getting pretty confident about. It's a little different than the way you've done it here, but addresses some other issues.

[thinkkevin]

I looked at the verror.js, the info works for the errors chain starting with a VError instance, as it only read from error.jse_info. Other properties from the beginning Error instance will be ignored.

IMHO, there are two things not good with this way:
a). The constructor with options as first parameter is terrifying. (Can't image creating a large block of codes to just make VError work )
b). Have to create VError instance in every the async callback (especially all kinds of libraries - such as request, redis, fs) to wrap Error instance and store its properties to VError's jse_info. Most of 3rd party libraries just assign properties to error instance.

Suggestion with what you have built:

1. When instantiate VError with cause (Error instance) as first parameter, automatically move all non-function properties (except message, name, stack) to jse_info (address the #a point above)
2. Provide VError.info(error, propertyName) to find most outer error carrying specified perpertyName and return the propertyValue.(address this issue though compromise #b point above)

My opinion about VError is that it's the best fit in my use case to build errors for both developer and end users. No matter it's 3rd-party npm module or internal lib/util, the error processing always starts with Error instance with whatever properties assigned to it. Take express.js req/res for example

Route Handler ---(invoke)---> Controller ---(invoke)---> Library
Library ---(Error)---> Controller ---(VError/WError)---> Route Handler ---(WError)---> Client

Library generate whatever error (e.g. request connection timeout error), should be wrapped in Controller with Developer friendly message (e.g. XXX API Request Timeout), and before we send the error to client, we need to wrap it again with Client friendly message (e.g. Oops, something wrong blabla, retry pls...)

In this common use case, the most likely place to set properties is at the Library. And I don't think it's significant better to group all properties to error.jse_info and force user to create a VError at Library.

[davepacheco]

[edited]

The "jse_info" bit is an implementation detail, but what you're getting at is that the design for #10 does not use arbitrary properties hanging off the Error object. That is a little more inconvenient, but it has a couple of advantages:

• An important design goal for VError is that it always be valid to take an existing Error and wrap it with a VError (i.e., making the original Error the cause of the new VError) and not have that change the behavior of consumers looking at that Error. If properties are hanging off the Error object and you wrap it with a VError, then we presumably need to copy those properties onto the VError. I explored doing this first, but it felt janky.
• The current approach protects us from future property names added to the Error class. As it is, there are a number of properties provided by different implementations (see the MDN Error documentation), which would presumably have to be reserved, which I think would make the interface even more confusing.
• A lot of times, people build subclasses of Error that have other properties that are not informational properties. VError itself and MultiError are examples of this. We don't want to copy these private properties (like jse_cause) up to wrapped Errors. Trying to distinguish private properties from information properties this way is nasty, and I really felt like it was much cleaner for both producers, consumers, and the implementation when I switched to the "info" constructor argument.
• This approach makes it easy to collect the set of "info" properties for a given Error (e.g., for logging or inserting into a fault monitoring system). If we hang info properties off the Error object directly, then callers wanting the list of informational properties need to know to ignore all the pseudo-standard property names linked above -- plus all the private properties I mentioned above, which they're by definition not supposed to know about.

As for (a), you're right that the constructor can look more like a large block, but I don't think it's very different than if you're using separate properties. You've got to compare:

err = new VError('message');
err.prop1 = value1;
err.prop2 = value2;
err.prop3 = value3;

to:

err = new VError({
    'info': {
        'prop1': value1,
        'prop2': value2,
        'prop3': value3
    }
}, 'message');

I think it's cleaner that it's declarative. And in the much more common case where you don't need additional properties, it's still ergonomic:

new VError('my message')

As for (b), I think it's a reasonable idea to provide a VError function that takes an Error provided by a third-party library and creates a VError containing all of the non-standard property names as VError 'info' properties.

[davepacheco]

Having heard no more, I'm closing this out. I'm hoping to land #10 soon.