TritonDataCenter / node-verror

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

[Question] best practice handing verrors to users of a library #62

Closed andykais closed 5 years ago

andykais commented 5 years ago

I have a general question about the usage of this library when used in a developer facing library. I have a npm package that runs some tasks and gives the user an event emitter. The event emitter can listen for errors, and as an extra helper, if there is no error event listener, just throw the error:

// library's code
class MyEmitter extends EventEmitter {
  emitError(error) {
    if (this.listenerCount('error') {
      this.emit('error', error)
    } else {
      throw error
    }
  }
}
// user's code
import mylibrary from 'mylibrary'
const myEmitter = mylibrary.run()
myEmitter.on('error', error => {
  // can be either VError or nodejs Error instance
  console.error(error)
}

The situations I use verror for are

My question is, what is considered the best practice in this situation? The user likely wants to log this error and possibly report it back to me, the library author if something goes wrong. However, I cannot assume they know what to do with a VError, nor should they have to care about the internal workings of the library. If they happen to print error.stack, they will just receive the most top level stacktrace, which is unhelpful. If not, they still receive a VError object, which is not a standard error object and is unwieldy to try to understand, especially when it is several levels deep. VError.fullStack(error) is the most user friendly output of this library, but I should not expect users of the library to know that is what they should run. Is it best practice to emit the string from VError.fullStack instead of an verror object to anything client facing?

davepacheco commented 5 years ago

I'm not sure I understand your question, but here's some background that may be helpful.

Generally, my goals for VError have been that:

If you decide to use VError-specific features in errors exposed by your module, you probably need to document that and tell callers to use VError.findCauseByName() if they ever care about what kind of error they're looking at and VError.info() if they care about informational properties. (Otherwise, they might try to use .name and get unexpected behavior.)

The rest of this comment is background I wrote up for myself to better understand the issue. I saved it here in case it's helpful for you or others (but I imagine much of it is stuff you know).


To make VErrors look as similar as possible to basic Errors, they have useful name, message, and stack properties. Traditionally, that was essentially all that JavaScript guaranteed about Errors. (I'm not sure what you mean by it being "unwieldy to understand". A lot of other libraries use their own error classes with their own properties, so I'm not sure it's reasonable to expect Errors not to contain other properties.)

These are the use-cases I think about for errors:

Basically, the second and third use-cases get tricky if you want to (1) use cause chains, and (2) don't want callers to have to know about VError. I don't think there's a great way to do that. However, I would definitely not recommend emitting a string of the stack instead of a VError object because that breaks two of the use-cases above.

I'm not sure if I've answered your question. Does that help?

andykais commented 5 years ago

Thank you @davepacheco for the extensive response! The third use case is in fact what I was describing (apologies if it was unclear). I wanted to make sure developers using the library could report back full errors to me if necessary.

As a solution, I did in fact start using bunyan. If a caller encounters a problem now, I ask that they send up the log file produced by bunyan, which will expand my VErrors. I still emit an error event, but after reading your use cases, callers dealing with the error will likely treat all errors universally. It therefore seems like it would fall under the first use case. I will emit the VError instance and leave a note in the docs.