-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat(symbolication): Return event_id on error #103812
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,13 +17,15 @@ | |
|
|
||
| from sentry import options | ||
| from sentry.attachments.base import CachedAttachment | ||
| from sentry.lang.native.error import SymbolicationFailed, write_error | ||
| from sentry.lang.native.sources import ( | ||
| get_internal_artifact_lookup_source, | ||
| get_internal_source, | ||
| get_scraping_config, | ||
| sources_for_symbolication, | ||
| ) | ||
| from sentry.lang.native.utils import Backoff | ||
| from sentry.models.eventerror import EventError | ||
| from sentry.models.project import Project | ||
| from sentry.net.http import Session | ||
| from sentry.objectstore import get_attachments_session | ||
|
|
@@ -396,6 +398,30 @@ def process_jvm( | |
| return self._process("symbolicate_jvm_stacktraces", "symbolicate-jvm", json=json) | ||
|
|
||
|
|
||
| def handle_response_status(event_data: Any, response_json: dict[str, Any]) -> bool | None: | ||
| """Checks the response from Symbolicator and reports errors. | ||
| Returns `True` on success.""" | ||
|
|
||
| if not response_json: | ||
| error = SymbolicationFailed(type=EventError.NATIVE_INTERNAL_FAILURE) | ||
| elif response_json["status"] == "completed": | ||
| return True | ||
| elif response_json["status"] == "failed": | ||
| error = SymbolicationFailed( | ||
| message=response_json.get("message") or None, | ||
| type=EventError.NATIVE_SYMBOLICATOR_FAILED, | ||
| event_id=response_json.get("event_id"), | ||
| ) | ||
| else: | ||
| logger.error("Unexpected symbolicator status: %s", response_json["status"]) | ||
| error = SymbolicationFailed( | ||
| type=EventError.NATIVE_INTERNAL_FAILURE, event_id=response_json.get("event_id") | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missing Sentry event capture for unexpected statusWhen Symbolicator returns an unexpected status value, the code logs an error but doesn't capture it to Sentry, so |
||
|
|
||
| write_error(error, event_data) | ||
| return None | ||
|
|
||
|
|
||
| class TaskIdNotFound(Exception): | ||
| pass | ||
|
|
||
|
|
@@ -496,9 +522,13 @@ def _request(self, method, path, **kwargs): | |
| else: | ||
| with sentry_sdk.isolation_scope(): | ||
| sentry_sdk.set_extra("symbolicator_response", response.text) | ||
| sentry_sdk.capture_message("Symbolicator request failed") | ||
| event_id = sentry_sdk.capture_message("Symbolicator request failed") | ||
|
|
||
| json = {"status": "failed", "message": "internal server error"} | ||
| json = { | ||
| "status": "failed", | ||
| "message": "internal server error", | ||
| "event_id": event_id, | ||
| } | ||
|
|
||
| return json | ||
| except (OSError, RequestException) as e: | ||
|
|
||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand what you mean. Are you suggesting that we store the event ID of the Sentry error on the scope?
Uh oh!
There was an error while loading. Please reload this page.
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 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.