diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 7a1275b852..263e27e111 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -22,6 +22,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - clickhouse-driver integration: The query is now available under the `db.query.text` span attribute (only if `send_default_pii` is `True`). - `sentry_sdk.init` now returns `None` instead of a context manager. - The `sampling_context` argument of `traces_sampler` and `profiles_sampler` now additionally contains all span attributes known at span start. +- 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) - The integration-specific content of the `sampling_context` argument of `traces_sampler` and `profiles_sampler` now looks different. - The Celery integration doesn't add the `celery_job` dictionary anymore. Instead, the individual keys are now available as: diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 22bb09c242..2e6d82d0ae 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -775,14 +775,17 @@ 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 (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: + - 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, @@ -793,64 +796,63 @@ 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 = ( + 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 + 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 - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(cause), - exc_value=cause, - tb=getattr(cause, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - source="__cause__", - full_stack=full_stack, - ) - exceptions.extend(child_exceptions) - + if has_explicit_causing_exception: + exception_source = "__cause__" + causing_exception = exc_value.__cause__ # type: ignore 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. + 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 - (exception_id, child_exceptions) = exceptions_from_error( - exc_type=type(context), - exc_value=context, - tb=getattr(context, "__traceback__", None), - client_options=client_options, - mechanism=mechanism, - exception_id=exception_id, - source="__context__", - full_stack=full_stack, - ) - exceptions.extend(child_exceptions) + if has_implicit_causing_exception: + exception_source = "__context__" + causing_exception = exc_value.__context__ # type: ignore + + 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 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, @@ -870,38 +872,29 @@ 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 - is_exception_group = BaseExceptionGroup is not None and isinstance( - exc_value, BaseExceptionGroup + # let exceptions_from_error do the actual work + _, 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, - ) - ) - + # make sure the exceptions are sorted + # from the innermost (oldest) + # to the outermost (newest) exception exceptions.reverse() return exceptions diff --git a/tests/integrations/ariadne/test_ariadne.py b/tests/integrations/ariadne/test_ariadne.py index 2c3b086aa5..6637a88451 100644 --- a/tests/integrations/ariadne/test_ariadne.py +++ b/tests/integrations/ariadne/test_ariadne.py @@ -68,7 +68,9 @@ 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 len(event["exception"]["values"]) == 2 + 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 +113,10 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + 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 +157,10 @@ 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 len(event["exception"]["values"]) == 2 + 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 +190,9 @@ def graphql_server(): assert len(events) == 1 (event,) = events - assert event["exception"]["values"][0]["mechanism"]["type"] == "ariadne" + assert len(event["exception"]["values"]) == 2 + 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"] diff --git a/tests/integrations/strawberry/test_strawberry.py b/tests/integrations/strawberry/test_strawberry.py index fdf7ff71bb..d1774aeca5 100644 --- a/tests/integrations/strawberry/test_strawberry.py +++ b/tests/integrations/strawberry/test_strawberry.py @@ -204,7 +204,9 @@ def test_capture_request_if_available_and_send_pii_is_on( (error_event,) = events - assert error_event["exception"]["values"][0]["mechanism"]["type"] == "strawberry" + assert len(error_event["exception"]["values"]) == 2 + 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 +260,10 @@ 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 len(error_event["exception"]["values"]) == 2 + 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"] diff --git a/tests/test_exceptiongroup.py b/tests/test_exceptiongroup.py index 4c7afc58eb..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,7 +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", @@ -267,6 +274,7 @@ def test_exception_chain_context(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError", @@ -297,6 +305,7 @@ def test_simple_exception(): "mechanism": { "handled": False, "type": "test_suite", + "exception_id": 0, }, "module": None, "type": "ValueError",