search

arb / celebrate

A joi validation middleware for Express.
MIT License
1.33k stars 66 forks source link

Celebrate errors no longer extend `Error` #138

Closed schmod closed 4 years ago

schmod commented 4 years ago

Celebrate 10.0.0 changed the structure and context of the error objects that Celebrate produces (ie. the err that Celebrate passes into next(err)).

This change appears to have broken some 3rd-party error-logging middlewares (specifically sentry) that expect errors passed to next() to extend the built-in JS Error object.

Would it be possible for format to start returning an Error object with a sensible message property, instead of a plain object?

arb commented 4 years ago

Yeah I changed that because I didn't like mutation the result object, even if I was confident there wouldn't be a collision...

Are there any other benefits of sub classing Error other than to work cleaner with some 3rd (4th in this case 😅) party libraries. POJOs are way easier to work with than Error objects plus Error objects are much heavier than celebrate really needs. Real Errors include things like stack traces which I don't need.

schmod commented 4 years ago

What if we put a message property on the root-level Celebrate error? It seems like this could be a middle-ground that does the right thing for many logging/error-handling middlewares that expect "error-like" objects to be passed to next(err).

Sentry still wouldn't currently handle this correctly, but I could open an issue with them to see if they'd be open to fixing this on their end.

arb commented 4 years ago

@schmod how's this look to you?