better support for extra properties on Errors

[davepacheco]

It would be nice to support putting more properties on Errors. That means:

• making it easy to add properties (e.g., with an extra "properties" argument or something)
• consider shallow-copying properties from the cause into the new Error, so that consumers don't have to look through causes to find details

CC @jclulow @tjfontaine

[ronkorving]

+1

[cappslock]

Would you merge a pull request which adds this behavior? I find it very useful to merge errors rather than push them onto an error stack.

[davepacheco]

@cappslock Check out #15. I have a branch that supports this but I haven't played with it much in anything real. Let me know what you think.

[wachunga]

👍

[DonutEspresso]

@davepacheco, @yunong and I recently added support for contextual properties in restify-errors. For now, we opted to maintain all that context in its own bucket and additionally added support for a custom bunyan serializer that iterates through those context properties. The scope for this feature was mostly to add additional context, since overriding name and other "native" error properties are done at constructor creation time.

We'd love to see something like this supported natively in verror and bunyan. We're not particularly opinionated about the how, but do think this is a pretty valuable use case. I saw that you had a branch that already does some of this - any thoughts on how we can help get this feature integrated?

[davepacheco]

@DonutEspresso yes, it's long past time to merge this. Given how widely we use this library, it's really important that the change be backwards-compatible, and supportable indefinitely as well. That's the main reason I haven't merged it -- I haven't prioritized trying it in some projects to make sure it does what we'd hope from such an interface, and I haven't heard that anybody else has either. It's probably time to update that branch and try it out.

[davepacheco]

FWIW: I've rebased the changes I had before, and decided to make a few interfaces changes. I'm mostly through putting together a complete description of the new interface, and it shouldn't take all that long to update the code and tests to match and build some useful examples. I hope to have something for this soon, and I'll be interested in feedback. Thanks for the patience.

[davepacheco]

This still needs some cleanup and improved examples, but here's where I am right now:
https://github.com/davepacheco/node-verror/tree/dev-issue-10

I'd suggest checking out the new README.md. Here are the non-trivial design choices I went with:

• These extra properties are called "information" properties. To specify them, you pass them as an "info" option to the constructor. (See the README.md.)
• To get these properties out, you use the info() method to return an object containing all of the properties for this error and its whole cause chain. This inheritance behavior is important to us because we want you to be able to wrap any VError with another one without losing the info properties.
• Notably, this does not expose the properties as top-level properties on the Error. We thought it was nearly as convenient to have them returned by info(), but this allows more flexibility, and also allows you to dump all of these properties without dumping the VError-internal properties.

I'd be interested in feedback on this (CC @jclulow).

The full diff is here, but includes a ton of noise because as part of this change I'm bringing a bunch of other stuff up to date:
https://github.com/davepacheco/node-verror/compare/dev-issue-10

Unrelated changes I made as part of this:

• major README.md rewrite
• pull in catest for a test runner
• update Makefiles for more efficient "check" targets
• normalize VError, WError, and SError so that they all process constructor arguments exactly the same way, and update tests to reflect that

[davepacheco]

@DonutEspresso, @yunong, or anyone else: have you had a chance to review those changes?

[DonutEspresso]

Unfortunately I haven't had a chance yet to check it out - been super heads down the last few weeks. I'll try to take a look sometime next week. Thanks for the hard work!

[davepacheco]

I understand -- no problem at all!

[davepacheco]

I've been trying this on a new project of my own, and I've found a small problem with the interface: it uses a new method called info(), but in general, when you've got an Error object, you don't know if you can call info() because you don't know if it's a VError or not. As a result, I'm leaning towards replacing the info() method with a class function (rather than an instance method) called VError.info(err) which works on any kind of Error.

I'm also leaning towards implementing #11 along with this because as I write new code to work with cause chains reasonably, I find I need that functionality as well. I would similarly implement that using a VError.findCause() method rather than adding a method to VError instances.

Thoughts?

[davepacheco]

I've updated my branch to include the changes in my previous comment.

[DonutEspresso]

I started work on a new module that is pretty stand alone, which gives me the perfect opportunity to try out the new changes. Is dev-issue-10 the right branch? I'll probably be working on this new module over the next few weeks, so as long as you're not wanting to land this right now now, I might be able to provide some feedback (if I have any) before you land it.

[davepacheco]

That would be great, and yes, that's the right branch. Hopefully the documentation is much better now!

There's also a detailed design at https://github.com/joyent/rfd/tree/master/rfd/0041.

[davepacheco]

I'm just about ready to land this. Any last feedback would be great.

[DonutEspresso]

Sorry for delay - been using this for a bit now and have found it great for modules. Typed errors work well at the application layer, but quickly become unruly since instanceof checks can easily fail as different context(s) are built up. No complaints here at the moment!

I do think native Bunyan integration would be excellent. I'm happy to write my own serializer as we did for restify-errors, but it would be pretty rad if bunyan had this baked in. Do you know if there are any plans to make that happen?