-
Notifications
You must be signed in to change notification settings - Fork 409
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
feat: Allowed undici error reporting to be disabled with feature flag undici_error_tracking
#2956
feat: Allowed undici error reporting to be disabled with feature flag undici_error_tracking
#2956
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.
Looks good. I'm going to commit a documentation update and test tweak but thanks for adding @Voziv !
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.
i cannot push to your branch, so i suggested a change and I'll follow up with doc update
undici_error_tracking
undici_error_tracking
undici_error_tracking
Co-authored-by: Bob Evans <robert.evans25@gmail.com>
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.
thanks for handling the back and forth @Voziv, LGTM. Just a heads up, we will probably release this next week.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2956 +/- ##
==========================================
- Coverage 97.35% 97.30% -0.05%
==========================================
Files 317 317
Lines 48690 48692 +2
==========================================
- Hits 47402 47382 -20
- Misses 1288 1310 +22
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
When
fetch
in node 20+ is wrapped in a try/catch the error that is thrown even if the user doesn't rethrow the error. At best this means errors being reported that are not errors, at worst it means errors are being reported twice.This adds the feature flag
undici_error_tracking
as suggested by @bizob2828 to allow disabling error capture via undici message channels.How to Test
See test in the files changed. I've caught an undici error and ensured that when the flag is turned off the transaction error array is empty.
Related Issues
Fixes #2954