From cc8e3294e21bc01470ec34557a53096ab98bd01b Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 4 Mar 2025 12:16:19 +0100 Subject: [PATCH 01/10] Better handling of exception groups --- sentry_sdk/utils.py | 37 +++++++++---------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 89b2354c52..379e0c08fc 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -897,36 +897,17 @@ def exceptions_from_error_tuple( # type: (...) -> List[Dict[str, Any]] exc_type, exc_value, tb = exc_info - is_exception_group = BaseExceptionGroup is not None and isinstance( - exc_value, BaseExceptionGroup + _, exceptions = exceptions_from_error( + exc_type=exc_type, + exc_value=exc_value, + tb=tb, + client_options=client_options, + mechanism=mechanism, + exception_id=0, + parent_id=0, + full_stack=full_stack, ) - if is_exception_group: - (_, exceptions) = exceptions_from_error( - exc_type=exc_type, - exc_value=exc_value, - tb=tb, - client_options=client_options, - mechanism=mechanism, - exception_id=0, - parent_id=0, - full_stack=full_stack, - ) - - else: - exceptions = [] - for exc_type, exc_value, tb in walk_exception_chain(exc_info): - exceptions.append( - single_exception_from_error_tuple( - exc_type=exc_type, - exc_value=exc_value, - tb=tb, - client_options=client_options, - mechanism=mechanism, - full_stack=full_stack, - ) - ) - exceptions.reverse() return exceptions From 1c2c5c1a2852fe53ecbe7553f07d6e1163db4bc8 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Tue, 4 Mar 2025 15:24:27 +0100 Subject: [PATCH 02/10] Updated some tests --- sentry_sdk/utils.py | 3 ++- tests/new_scopes_compat/test_new_scopes_compat_event.py | 6 +++++- tests/test_exceptiongroup.py | 3 +++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 379e0c08fc..b1ee9986d2 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -697,7 +697,8 @@ def single_exception_from_error_tuple( is_root_exception = exception_id == 0 if not is_root_exception and parent_id is not None: exception_value["mechanism"]["parent_id"] = parent_id - exception_value["mechanism"]["type"] = "chained" + if "type" not in exception_value["mechanism"]: + exception_value["mechanism"]["type"] = "chained" if is_root_exception and "type" not in exception_value["mechanism"]: exception_value["mechanism"]["type"] = "generic" diff --git a/tests/new_scopes_compat/test_new_scopes_compat_event.py b/tests/new_scopes_compat/test_new_scopes_compat_event.py index db1e5fec4b..efd28f911a 100644 --- a/tests/new_scopes_compat/test_new_scopes_compat_event.py +++ b/tests/new_scopes_compat/test_new_scopes_compat_event.py @@ -36,7 +36,11 @@ def create_expected_error_event(trx, span): "exception": { "values": [ { - "mechanism": {"type": "generic", "handled": True}, + "mechanism": { + "exception_id": 0, + "type": "generic", + "handled": True, + }, "module": None, "type": "ValueError", "value": "This is a test exception", diff --git a/tests/test_exceptiongroup.py b/tests/test_exceptiongroup.py index 4c7afc58eb..e163101199 100644 --- a/tests/test_exceptiongroup.py +++ b/tests/test_exceptiongroup.py @@ -258,6 +258,7 @@ def test_exception_chain_context(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 1, }, "module": None, "type": "TypeError", @@ -267,6 +268,7 @@ def test_exception_chain_context(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -297,6 +299,7 @@ def test_simple_exception(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", From e5d4f56c4ed0dc352897153d33ec3ddd65f2e2cf Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 5 Mar 2025 11:12:02 +0100 Subject: [PATCH 03/10] trigger ci From 8c957c6ecbc78b64d8624720ee407f06dc331907 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 5 Mar 2025 12:33:24 +0100 Subject: [PATCH 04/10] Updated some tests --- sentry_sdk/utils.py | 2 +- tests/test_exceptiongroup.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index b1ee9986d2..d38e55128f 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -697,7 +697,7 @@ def single_exception_from_error_tuple( is_root_exception = exception_id == 0 if not is_root_exception and parent_id is not None: exception_value["mechanism"]["parent_id"] = parent_id - if "type" not in exception_value["mechanism"]: + if exception_id is not None and exception_id > 0: exception_value["mechanism"]["type"] = "chained" if is_root_exception and "type" not in exception_value["mechanism"]: diff --git a/tests/test_exceptiongroup.py b/tests/test_exceptiongroup.py index e163101199..01ec0a78d4 100644 --- a/tests/test_exceptiongroup.py +++ b/tests/test_exceptiongroup.py @@ -217,7 +217,10 @@ def test_exception_chain_cause(): { "mechanism": { "handled": False, - "type": "test_suite", + "type": "chained", + "exception_id": 1, + "parent_id": 0, + "source": "__cause__", }, "module": None, "type": "TypeError", @@ -227,6 +230,7 @@ def test_exception_chain_cause(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -257,8 +261,10 @@ def test_exception_chain_context(): { "mechanism": { "handled": False, - "type": "test_suite", + "type": "chained", "exception_id": 1, + "parent_id": 0, + "source": "__context__", }, "module": None, "type": "TypeError", From 8062aa36525374dc17e49b97b7068720b5619f16 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 5 Mar 2025 12:41:29 +0100 Subject: [PATCH 05/10] Updated ariadne tests --- tests/integrations/ariadne/test_ariadne.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/integrations/ariadne/test_ariadne.py b/tests/integrations/ariadne/test_ariadne.py index 2c3b086aa5..a2de5a9eea 100644 --- a/tests/integrations/ariadne/test_ariadne.py +++ b/tests/integrations/ariadne/test_ariadne.py @@ -68,7 +68,8 @@ def test_capture_request_and_response_if_send_pii_is_on_async( assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert event["contexts"]["response"] == { "data": { "data": {"error": None}, @@ -111,7 +112,9 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" + assert event["contexts"]["response"] == { "data": { "data": {"error": None}, @@ -152,7 +155,9 @@ def test_do_not_capture_request_and_response_if_send_pii_is_off_async( assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" + assert "data" not in event["request"] assert "response" not in event["contexts"] @@ -182,7 +187,8 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert event["exception"]["values"][-1]["mechanism"]["type"] == "ariadne" assert "data" not in event["request"] assert "response" not in event["contexts"] From 5c4501995fcbaa172d341fc0370f01bd25435e04 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 5 Mar 2025 12:47:03 +0100 Subject: [PATCH 06/10] Updated strawberry tests --- tests/integrations/strawberry/test_strawberry.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index 7b40b238d2..c768778730 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -204,7 +204,8 @@ def test_capture_request_if_available_and_send_pii_is_on( (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry" assert error_event["request"]["api_target"] == "graphql" assert error_event["request"]["data"] == { "query": query, @@ -258,7 +259,8 @@ def test_do_not_capture_request_if_send_pii_is_off( assert len(events) == 1 (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + assert error_event["exception"]["values"][0]["mechanism"]["type"] == "chained" + assert error_event["exception"]["values"][-1]["mechanism"]["type"] == "strawberry" assert "data" not in error_event["request"] assert "response" not in error_event["contexts"] From f3791ecf5ebc215a93b35fc2e9426def9ae49bdb Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Mar 2025 09:19:32 +0100 Subject: [PATCH 07/10] Removed useless if --- sentry_sdk/utils.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index d38e55128f..379e0c08fc 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -697,8 +697,7 @@ def single_exception_from_error_tuple( is_root_exception = exception_id == 0 if not is_root_exception and parent_id is not None: exception_value["mechanism"]["parent_id"] = parent_id - if exception_id is not None and exception_id > 0: - exception_value["mechanism"]["type"] = "chained" + exception_value["mechanism"]["type"] = "chained" if is_root_exception and "type" not in exception_value["mechanism"]: exception_value["mechanism"]["type"] = "generic" From 781925ad60e1b9c162d4a5da71a2e816f686af3a Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Mar 2025 10:16:13 +0100 Subject: [PATCH 08/10] Make it easier to undertand --- sentry_sdk/utils.py | 80 +++++++++++++++++++++++++++------------------ 1 file changed, 49 insertions(+), 31 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 379e0c08fc..9d99e96190 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -800,14 +800,16 @@ def exceptions_from_error( ): # type: (...) -> Tuple[int, List[Dict[str, Any]]] """ - Creates the list of exceptions. - This can include chained exceptions and exceptions from an ExceptionGroup. - - See the Exception Interface documentation for more details: - https://develop.sentry.dev/sdk/event-payloads/exception/ + Converts the given exception information into the Sentry structured "exception" format. + This will return a list of exceptions in the format of the Exception Interface documentation: + https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ + + This function can handle: + - simple exceptions + - chained exceptions (raise .. from ..) + - exception groups """ - - parent = single_exception_from_error_tuple( + base_exception = single_exception_from_error_tuple( exc_type=exc_type, exc_value=exc_value, tb=tb, @@ -818,64 +820,70 @@ def exceptions_from_error( source=source, full_stack=full_stack, ) - exceptions = [parent] + exceptions = [base_exception] parent_id = exception_id exception_id += 1 - should_supress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore - if should_supress_context: - # Add direct cause. - # The field `__cause__` is set when raised with the exception (using the `from` keyword). - exception_has_cause = ( + # Note: __suppress_context__ is True if the exception is raised with the `from` keyword. + should_suppress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore + if should_suppress_context: + # Explicitly chained exceptions (Like: raise NewException() from OriginalException()) + # The field `__cause__` is set to OriginalException + exception_has_explicit_causing_exception = ( exc_value and hasattr(exc_value, "__cause__") and exc_value.__cause__ is not None ) - if exception_has_cause: - cause = exc_value.__cause__ # type: ignore + if exception_has_explicit_causing_exception: + causing_exception = exc_value.__cause__ # type: ignore + (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(cause), - exc_value=cause, - tb=getattr(cause, "__traceback__", None), + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), client_options=client_options, mechanism=mechanism, exception_id=exception_id, + # parent_id=parent_id, TODO: why is this not set? source="__cause__", full_stack=full_stack, ) exceptions.extend(child_exceptions) else: - # Add indirect cause. - # The field `__context__` is assigned if another exception occurs while handling the exception. - exception_has_content = ( + # Implicitly chained exceptions (when an exception occurs while handling another exception) + # The field `__context__` is set in the exception that occurs while handling another exception, + # to the other exception. + exception_has_implicit_causing_exception = ( exc_value and hasattr(exc_value, "__context__") and exc_value.__context__ is not None ) - if exception_has_content: - context = exc_value.__context__ # type: ignore + if exception_has_implicit_causing_exception: + causing_exception = exc_value.__context__ # type: ignore + (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(context), - exc_value=context, - tb=getattr(context, "__traceback__", None), + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), client_options=client_options, mechanism=mechanism, exception_id=exception_id, + # parent_id=parent_id, TODO: why is this not set? source="__context__", full_stack=full_stack, ) exceptions.extend(child_exceptions) - # Add exceptions from an ExceptionGroup. + # Add child exceptions from an ExceptionGroup. is_exception_group = exc_value and hasattr(exc_value, "exceptions") if is_exception_group: - for idx, e in enumerate(exc_value.exceptions): # type: ignore + for idx, causing_exception in enumerate(exc_value.exceptions): # type: ignore (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(e), - exc_value=e, - tb=getattr(e, "__traceback__", None), + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), client_options=client_options, mechanism=mechanism, exception_id=exception_id, @@ -895,8 +903,15 @@ def exceptions_from_error_tuple( full_stack=None, # type: Optional[list[dict[str, Any]]] ): # type: (...) -> List[Dict[str, Any]] + """ + Convert Python's exception information into Sentry's structured "exception" format in the event. + See https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ + This is the entry point for the exception handling. + """ + # unpack the exception info tuple exc_type, exc_value, tb = exc_info + # let exceptions_from_error do the actual work _, exceptions = exceptions_from_error( exc_type=exc_type, exc_value=exc_value, @@ -908,6 +923,9 @@ def exceptions_from_error_tuple( full_stack=full_stack, ) + # make sure the exceptions are sorted + # from the innermost (oldest) + # to the outermost (newest) exception exceptions.reverse() return exceptions From 5a3a3acd91c2eb9c671d66738228ed52d27225cd Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Mar 2025 10:18:32 +0100 Subject: [PATCH 09/10] Give missing parent to recursive calls --- sentry_sdk/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 9d99e96190..6410f31590 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -845,7 +845,7 @@ def exceptions_from_error( client_options=client_options, mechanism=mechanism, exception_id=exception_id, - # parent_id=parent_id, TODO: why is this not set? + parent_id=parent_id, source="__cause__", full_stack=full_stack, ) @@ -870,7 +870,7 @@ def exceptions_from_error( client_options=client_options, mechanism=mechanism, exception_id=exception_id, - # parent_id=parent_id, TODO: why is this not set? + parent_id=parent_id, source="__context__", full_stack=full_stack, ) From e36d43ef340aaf0beab1a11d18c41c721b4848b5 Mon Sep 17 00:00:00 2001 From: Anton Pirker Date: Wed, 19 Mar 2025 10:29:21 +0100 Subject: [PATCH 10/10] Cleanup --- sentry_sdk/utils.py | 58 ++++++++++++++++++++------------------------- 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 6410f31590..a3bc1b0967 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -801,7 +801,8 @@ def exceptions_from_error( # type: (...) -> Tuple[int, List[Dict[str, Any]]] """ Converts the given exception information into the Sentry structured "exception" format. - This will return a list of exceptions in the format of the Exception Interface documentation: + This will return a list of exceptions (a flattened tree of exceptions) in the + format of the Exception Interface documentation: https://develop.sentry.dev/sdk/data-model/event-payloads/exception/ This function can handle: @@ -825,56 +826,49 @@ def exceptions_from_error( parent_id = exception_id exception_id += 1 - # Note: __suppress_context__ is True if the exception is raised with the `from` keyword. + causing_exception = None + exception_source = None + + # Add any causing exceptions, if present. should_suppress_context = hasattr(exc_value, "__suppress_context__") and exc_value.__suppress_context__ # type: ignore + # Note: __suppress_context__ is True if the exception is raised with the `from` keyword. if should_suppress_context: # Explicitly chained exceptions (Like: raise NewException() from OriginalException()) # The field `__cause__` is set to OriginalException - exception_has_explicit_causing_exception = ( + has_explicit_causing_exception = ( exc_value and hasattr(exc_value, "__cause__") and exc_value.__cause__ is not None ) - if exception_has_explicit_causing_exception: + if has_explicit_causing_exception: + exception_source = "__cause__" causing_exception = exc_value.__cause__ # type: ignore - - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(causing_exception), - exc_value=causing_exception, - tb=getattr(causing_exception, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - parent_id=parent_id, - source="__cause__", - full_stack=full_stack, - ) - exceptions.extend(child_exceptions) - else: # Implicitly chained exceptions (when an exception occurs while handling another exception) # The field `__context__` is set in the exception that occurs while handling another exception, # to the other exception. - exception_has_implicit_causing_exception = ( + has_implicit_causing_exception = ( exc_value and hasattr(exc_value, "__context__") and exc_value.__context__ is not None ) - if exception_has_implicit_causing_exception: + if has_implicit_causing_exception: + exception_source = "__context__" causing_exception = exc_value.__context__ # type: ignore - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(causing_exception), - exc_value=causing_exception, - tb=getattr(causing_exception, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - parent_id=parent_id, - source="__context__", - full_stack=full_stack, - ) - exceptions.extend(child_exceptions) + if causing_exception: + (exception_id, child_exceptions) = exceptions_from_error( + exc_type=type(causing_exception), + exc_value=causing_exception, + tb=getattr(causing_exception, "__traceback__", None), + client_options=client_options, + mechanism=mechanism, + exception_id=exception_id, + parent_id=parent_id, + source=exception_source, + full_stack=full_stack, + ) + exceptions.extend(child_exceptions) # Add child exceptions from an ExceptionGroup. is_exception_group = exc_value and hasattr(exc_value, "exceptions")