8000 Better handling of exception groups (#4164) · getsentry/sentry-python@2753fac · GitHub
[go: up one dir, main page]
Skip to content

Commit 2753fac

Browse files
antonpirkerantonpirker
authored
Better handling of exception groups (#4164)
Properly handle grouped and chained exceptions. The test case in the linked issue illustrates that some ExceptionGroups have been handled in a wrong way. Updated some tests, because now that those are handled correctly all the mechanism types except for the root exception are set to "chained" like described in the RFC: https://github.com/getsentry/rfcs/blob/main/text/0079-exception-groups.md#interpretation Because this will change the grouping of exiting Sentry Issues containing ExceptionGroups, it is safer to release this fix in the next major and make sure that we describe the change in behavior in the changelog. (Note: The grouping in the Ariadne issues will not change because those are not ExceptionGroups and only updating the `mechanism.type` does not change the grouping) Fixes #3913
1 parent e57798f commit 2753fac

File tree

5 files changed

+102
-84
lines changed

5 files changed

+102
-84
lines changed

‎MIGRATION_GUIDE.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh
2222
- clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`).
2323
- `sentry_sdk.init` now returns `None` instead of a context manager.
2424
- The `sampling_context` argument of `traces_sampler` and `profiles_sampler` now additionally contains all span attributes known at span start.
25+
- We updated how we handle `ExceptionGroup`s. You will now get more data if ExceptionGroups are appearing in chained exceptions. It could happen that after updating the SDK the grouping of issues change because of this. So eventually you will see the same exception in two Sentry issues (one from before the update, one from after the update)
2526
- The integration-specific content of the `sampling_context` argument of `traces_sampler` and `profiles_sampler` now looks different.
2627
- The Celery integration doesn't add the `celery_job` dictionary anymore. Instead, the individual keys are now available as:
2728

‎sentry_sdk/utils.py

Lines changed: 69 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -775,14 +775,17 @@ def exceptions_from_error(
775775
):
776776
# type: (...) -> Tuple[int, List[Dict[str, Any]]]
777777
"""
778-
Creates the list of exceptions.
779-
This can include chained exceptions and exceptions from an ExceptionGroup.
780-
781-
See the Exception Interface documentation for more details:
782-
https://develop.sentry.dev/sdk/event-payloads/exception/
778+
Converts the given exception information into the Sentry structured "exception" format.
779+
This will return a list of exceptions (a flattened tree of exceptions) in the
780+
format of the Exception Interface documentation:
781+
https://develop.sentry.dev/sdk/data-model/event-payloads/exception/
782+
783+
This function can handle:
784+
- simple exceptions
785+
- chained exceptions (raise .. from ..)
786+
- exception groups
783787
"""
784-
785-
parent = single_exception_from_error_tuple(
788+
base_exception = single_exception_from_error_tuple(
786789
exc_type=exc_type,
787790
exc_value=exc_value,
788791
tb=tb,
@@ -793,64 +796,63 @@ def exceptions_from_error(
793796
source=source,
794797
full_stack=full_stack,
795798
)
796-
exceptions = [parent]
799+
exceptions = [base_exception]
797800

798801
parent_id = exception_id
799802
exception_id += 1
800803

801-
should_supress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore
802-
if should_supress_context:
803-
# Add direct cause.
804-
# The field `__cause__` is set when raised with the exception (using the `from` keyword).
805-
exception_has_cause = (
804+
causing_exception = None
805+
exception_source = None
806+
807+
# Add any causing exceptions, if present.
808+
should_suppress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore
809+
# Note: __suppress_context__ is True if the exception is raised with the `from` keyword.
810+
if should_suppress_context:
811+
# Explicitly chained exceptions (Like: raise NewException() from OriginalException())
812+
# The field `__cause__` is set to OriginalException
813+
has_explicit_causing_exception = (
806814
exc_value
807815
and hasattr(exc_value, "__cause__")
808816
and exc_value.__cause__ is not None
809817
)
810-
if exception_has_cause:
811-
cause = exc_value.__cause__ # type: ignore
812-
(exception_id, child_exceptions) = exceptions_from_error(
813-
exc_type=type(cause),
814-
exc_value=cause,
815-
tb=getattr(cause, "__traceback__", None),
816-
client_options=client_options,
817-
mechanism=mechanism,
818-
exception_id=exception_id,
819-
source="__cause__",
820-
full_stack=full_stack,
821-
)
822-
exceptions.extend(child_exceptions)
823-
818+
if has_explicit_causing_exception:
819+
exception_source = "__cause__"
820+
causing_exception = exc_value.__cause__ # type: ignore
824821
else:
825-
# Add indirect cause.
826-
# The field `__context__` is assigned if another exception occurs while handling the exception.
827-
exception_has_content = (
822+
# Implicitly chained exceptions (when an exception occurs while handling another exception)
823+
# The field `__context__` is set in the exception that occurs while handling another exception,
824+
# to the other exception.
825+
has_implicit_causing_exception = (
828826
exc_value
829827
and hasattr(exc_value, "__context__")
830828
and exc_value.__context__ is not None
831829
)
832-
if exception_has_content:
833-
context = exc_value.__context__ # type: ignore
834-
(exception_id, child_exceptions) = exceptions_from_error(
835-
exc_type=type(context),
836-
exc_value=context,
837-
tb=getattr(context, "__traceback__", None),
838-
client_options=client_options,
839-
mechanism=mechanism,
840-
exception_id=exception_id,
841-
source="__context__",
842-
full_stack=full_stack,
843-
)
844-
exceptions.extend(child_exceptions)
830+
if has_implicit_causing_exception:
831+
exception_source = "__context__"
832+
causing_exception = exc_value.__context__ # type: ignore
833+
834+
if causing_exception:
835+
(exception_id, child_exceptions) = exceptions_from_error(
836+
exc_type=type(causing_exception),
837+
exc_value=causing_exception,
838+
tb=getattr(causing_exception, "__traceback__", None),
839+
client_options=client_options,
840+
mechanism=mechanism,
841+
exception_id=exception_id,
842+
parent_id=parent_id,
843+
source=exception_source,
844+
full_stack=full_stack,
845+
)
846+
exceptions.extend(child_exceptions)
845847

846-
# Add exceptions from an ExceptionGroup.
848+
# Add child exceptions from an ExceptionGroup.
847849
is_exception_group = exc_value and hasattr(exc_value, "exceptions")
848850
if is_exception_group:
849-
for idx, e in enumerate(exc_value.exceptions): # type: ignore
851+
for idx, causing_exception in enumerate(exc_value.exceptions): # type: ignore
850852
(exception_id, child_exceptions) = exceptions_from_error(
851-
exc_type=type(e),
852-
exc_value=e,
853-
tb=getattr(e, "__traceback__", None),
853+
exc_type=type(causing_exception),
854+
exc_value=causing_exception,
855+
tb=getattr(causing_exception, "__traceback__", None),
854856
client_options=client_options,
855857
mechanism=mechanism,
856858
exception_id=exception_id,
@@ -870,38 +872,29 @@ def exceptions_from_error_tuple(
870872
full_stack=None, # type: Optional[list[dict[str, Any]]]
871873
):
872874
# type: (...) -> List[Dict[str, Any]]
875+
"""
876+
Convert Python's exception information into Sentry's structured "exception" format in the event.
877+
See https://develop.sentry.dev/sdk/data-model/event-payloads/exception/
878+
This is the entry point for the exception handling.
879+
"""
880+
# unpack the exception info tuple
873881
exc_type, exc_value, tb = exc_info
874882

875-
is_exception_group = BaseExceptionGroup is not None and isinstance(
876-
exc_value, BaseExceptionGroup
883+
# let exceptions_from_error do the actual work
884+
_, exceptions = exceptions_from_error(
885+
exc_type=exc_type,
886+
exc_value=exc_value,
887+
tb=tb,
888+
client_options=client_options,
889+
mechanism=mechanism,
890+
exception_id=0,
891+
parent_id=0,
892+
full_stack=full_stack,
877893
)
878894

879-
if is_exception_group:
880-
(_, exceptions) = exceptions_from_error(
881-
exc_type=exc_type,
882-
exc_value=exc_value,
883-
tb=tb,
884-
client_options=client_options,
885-
mechanism=mechanism,
886-
exception_id=0,
887-
parent_id=0,
888-
full_stack=full_stack,
889-
)
890-
891-
else:
892-
exceptions = []
893-
for exc_type, exc_value, tb in walk_exception_chain(exc_info):
894-
exceptions.append(
895-
single_exception_from_error_tuple(
896-
exc_type=exc_type,
897-
exc_value=exc_value,
898-
tb=tb,
899-
client_options=client_options,
900-
mechanism=mechanism,
901-
full_stack=full_stack,
902-
)
903-
)
904-
895+
# make sure the exceptions are sorted
896+
# from the innermost (oldest)
897+
# to the outermost (newest) exception
905898
exceptions.reverse()
906899

907900
return exceptions

‎tests/integrations/ariadne/test_ariadne.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ def test_capture_request_and_response_if_send_pii_is_on_async(
6868
assert len(events) == 1
6969

7070
(event,) = events
71-
assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
71+
assert len(event["exception"]["values"]) == 2
72+
assert event["exception"]["values"][0]["mechanism"]["type"] == "chained"
73+
assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
7274
assert event["contexts"]["response"] == {
7375
"data": {
7476
"data": {"error": None},
@@ -111,7 +113,10 @@ def graphql_server():
111113
assert len(events) == 1
112114

113115
(event,) = events
114-
assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
116+
assert len(event["exception"]["values"]) == 2
117+
assert event["exception"]["values"][0]["mechanism"]["type"] == "chained"
118+
assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
119+
115120
assert event["contexts"]["response"] == {
116121
"data": {
117122
"data": {"error": None},
@@ -152,7 +157,10 @@ def test_do_not_capture_request_and_response_if_send_pii_is_off_async(
152157
assert len(events) == 1
153158

154159
(event,) = events
155-
assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
160+
assert len(event["exception"]["values"]) == 2
161+
assert event["exception"]["values"][0]["mechanism"]["type"] == "chained"
162+
assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
163+
156164
assert "data" not in event["request"]
157165
assert "response" not in event["contexts"]
158166

@@ -182,7 +190,9 @@ def graphql_server():
182190
assert len(events) == 1
183191

184192
(event,) = events
185-
assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne"
193+
assert len(event["exception"]["values"]) == 2
194+
assert event["exception"]["values"][0]["mechanism"]["type"] == "chained"
195+
assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne"
186196
assert "data" not in event["request"]
187197
assert "response" not in event["contexts"]
188198

‎tests/integrations/strawberry/test_strawberry.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,9 @@ def test_capture_request_if_available_and_send_pii_is_on(
204204

205205
(error_event,) = events
206206

207-
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry"
207+
assert len(error_event["exception"]["values"]) == 2
208+
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained"
209+
assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry"
208210
assert error_event["request"]["api_target"] == "graphql"
209211
assert error_event["request"]["data"] == {
210212
"query": query,
@@ -258,7 +260,10 @@ def test_do_not_capture_request_if_send_pii_is_off(
258260
assert len(events) == 1
259261

260262
(error_event,) = events
261-
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry"
263+
264+
assert len(error_event["exception"]["values"]) == 2
265+
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained"
266+
assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry"
262267
assert "data" not in error_event["request"]
263268
assert "response" not in error_event["contexts"]
264269

‎tests/test_exceptiongroup.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,10 @@ def test_exception_chain_cause():
217217
{
218218
"mechanism": {
219219
"handled": False,
220-
"type": "test_suite",
220+
"type": "chained",
221+
"exception_id": 1,
222+
"parent_id": 0,
223+
"source": "__cause__",
221224
},
222225
"module": None,
223226
"type": "TypeError",
@@ -227,6 +230,7 @@ def test_exception_chain_cause():
227230
"mechanism": {
228231
"handled": False,
229232
"type": "test_suite",
233+
"exception_id": 0,
230234
},
231235
"module": None,
232236
"type": "ValueError",
@@ -257,7 +261,10 @@ def test_exception_chain_context():
257261
{
258262
"mechanism": {
259263
"handled": False,
260-
"type": "test_suite",
264+
"type": "chained",
265+
"exception_id": 1,
266+
"parent_id": 0,
267+
"source": "__context__",
261268
},
262269
"module": None,
263270
"type": "TypeError",
@@ -267,6 +274,7 @@ def test_exception_chain_context():
267274
"mechanism": {
268275
"handled": False,
269276
"type": "test_suite",
277+
"exception_id": 0,
270278
},
271279
"module": None,
272280
"type": "ValueError",
@@ -297,6 +305,7 @@ def test_simple_exception():
297305
"mechanism": {
298306
"h 5213 andled": False,
299307
"type": "test_suite",
308+
"exception_id": 0,
300309
},
301310
"module": None,
302311
"type": "ValueError",

0 commit comments

Comments
 (0)
0