TritonDataCenter / node-verror

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

could use VError.hasCauseWithName() #47

Closed davepacheco closed 7 years ago

davepacheco commented 7 years ago

When you only care whether a particular Error has a specific cause, you can use findCauseByName(err, name) and check whether the result is non-null. It's surprisingly easy to get the check backwards, and it's a little verbose if you're particular about falsey types (e.g., if findCauseByName(err, myErrorName) !== null) {). We could use a convenience function hasCauseWithName(err, myErrorName).

davepacheco commented 7 years ago

Suggested change: https://cr.joyent.us/#/c/907/

I tested this with "make prepush" on Node v0.10.45, v0.12.16, v4.6.0, and v6.8.0, all on OS X.

On the one hand, it's a little silly that the name is actually longer than findCauseByName, but the real motivation is clarity, not brevity. Frankly, I got the findCauseByName() !== null check wrong in several places where I was updating error handling, so I do think this change is worthwhile just for that. And I prefer the new name of the new function to be explicit (hasCauseWithName), partly because error handling code is easy to get subtly wrong and miss the problems during testing, and also because we may well end up adding other ways to search for causes in the future.