TritonDataCenter / node-verror

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

Why does cause need to be an instance of error? #66

Open ncjones opened 5 years ago

ncjones commented 5 years ago

Javascript allows any object to be thrown so I'm curious why the VError constructor is so strict on cause being an instance of Error. If, unbeknownst to me, some third-party code throws an arbitrary object then when I see that error for the first time I don't want my error handing to break because "cause is not an Error".

davepacheco commented 5 years ago

Interesting! While technically possible, and I remember doing that in the pre-Node days (e.g., using Spidermonkey on the server), I haven't run into code that throws non-Errors in quite some time. As I recall, it doesn't work well because there's no way to figure out where it was thrown from.

VError itself relies on the name, message, and stack properties that are provided by Error objects. It also exposes cause objects directly via VError.cause(). It's pretty useful that callers of that function know they're going to get back a valid Error. I think most callers would be surprised to get back a non-null, non-Error object, and it would be a bunch of work for callers to have to check what they got back.

ncjones commented 5 years ago

Yes 100% agree. If cause is not an Error I think VError should do it's best to preserve what ever context it has available. A best-effort attempt to create a new Error might look like:

  1. If it's a string then use that as the message.
    1. try JSON.stringify
    2. fall back to build a message by stringifying the top level properties.
davepacheco commented 5 years ago

I see. I think you're describing a "DWIM" (do-what-I-mean) approach -- i.e., have the software try to guess what was probably wanted for this particular case. I know a lot of people prefer that, but I often don't because I find it makes the software less predictable. In my case, if a VError gets a cause that's something other than an Error, I want to fix that right away (and I want the program to crash so that I get a core file that shows unambiguously where that object came from). I don't want to find out later, long after the problem happened, by finding an error message that was incomplete or misleading, without much ability to figure out where it came from. I realize this is a preference. We've largely avoided DWIM with VError. In some cases, we've created alternate constructors that are more or less strict in ways like this, and we could do that here if this case seems important.