-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(hapi): Specify error channel to filter boom errors #12725
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! Great tests, too 🚀
FWIW, I think there's another (arguably worse) version of this bug where onPreResponse recovers from the error with a 200 response. I'm not sure it's valuable for the tests to enumerate all these possibilities though. We're just relying on Hapi to report an error on the error channel when applicable. The details of that are not Sentry's responsibility. But also I'm not a maintainer so don't mind me! |
@camsteffen Thanks for you engagement in this topic! 👍🏻 I am not sure if I understood you correctly. I added a test case for a "2xx override" inside |
I guess what I'm suggesting is - I wonder if you could have a test like |
If errors are handled with Boom inside
onPreResponse
, the error should not be reported to Sentry.fixes #12702