feat(symbolication): Return event_id on error#103812
feat(symbolication): Return event_id on error#103812loewenheim wants to merge 1 commit intomasterfrom
Conversation
This adds a sentry event ID to the error message when symbolication fails because of an internal error. That should make it easier to track down the specific problem in Sentry.
3fcf89f to
f215577
Compare
| logger.error("Unexpected symbolicator status: %s", response_json["status"]) | ||
| error = SymbolicationFailed( | ||
| type=EventError.NATIVE_INTERNAL_FAILURE, event_id=response_json.get("event_id") | ||
| ) |
There was a problem hiding this comment.
Bug: Missing Sentry event capture for unexpected status
When Symbolicator returns an unexpected status value, the code logs an error but doesn't capture it to Sentry, so response_json.get("event_id") returns None. This prevents including a Sentry event ID for tracking internal errors, which contradicts the PR's goal. Unlike HTTP-level errors (lines 525-530), unexpected statuses don't get captured to Sentry, creating an inconsistency in error tracking.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103812 +/- ##
===========================================
- Coverage 80.58% 80.58% -0.01%
===========================================
Files 9293 9293
Lines 396687 396681 -6
Branches 25281 25281
===========================================
- Hits 319670 319658 -12
- Misses 76557 76563 +6
Partials 460 460 |
| else: | ||
| logger.error("Unexpected symbolicator status: %s", response_json["status"]) | ||
| error = SymbolicationFailed( | ||
| type=EventError.NATIVE_INTERNAL_FAILURE, event_id=response_json.get("event_id") |
There was a problem hiding this comment.
Does this need to come from the response? I assume sentry can just send this id as a scope when making the request?
There was a problem hiding this comment.
I don't quite understand what you mean. Are you suggesting that we store the event ID of the Sentry error on the scope?
There was a problem hiding this comment.
I don't understand why the event id needs to come from the Symbolicator response, Sentry should already have it.
One possibility is to instead setup the SDK scope in a way that the event id is always present, then there is also no need to parse it from the response. Assuming your use is for a Sentry error.
|
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you add the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
This adds a sentry event ID to the error message when symbolication fails because of an internal error. That should make it easier to track down the specific problem in Sentry.
It also centralizes the (identical)
_handle_response_statusfunctions for the different platforms in thesymbolicatormodule.