Skip to content

Fix FastAPI recursive exceptions #4334

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

Open
wants to merge 3 commits into
base: potel-base
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions sentry_sdk/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@
be affected by this limit if they have a custom recursion limit.
"""

MAX_EXCEPTIONS = 25
"""Maximum number of exceptions in a chain or group to send to Sentry.

This is a sanity limit to avoid ending in an infinite loop of exceptions when the same exception is in the root and a leave
of the exception tree.
"""


def env_to_bool(value, *, strict=False):
# type: (Any, Optional[bool]) -> bool | None
Expand Down Expand Up @@ -823,6 +830,9 @@ def exceptions_from_error(
parent_id = exception_id
exception_id += 1

if exception_id > MAX_EXCEPTIONS - 1:
return (exception_id, exceptions)

causing_exception = None
exception_source = None

Expand Down Expand Up @@ -853,6 +863,19 @@ def exceptions_from_error(
exception_source = "__context__"
causing_exception = exc_value.__context__ # type: ignore

if causing_exception:
# Some frameworks (e.g. FastAPI) wrap the causing exception in an
# ExceptionGroup that only contain one exception: the causing exception.
# This would lead to an infinite loop, so we skip the causing exception
# in this case. (because it is the same as the base_exception above)
if (
BaseExceptionGroup is not None
and isinstance(causing_exception, BaseExceptionGroup)
and len(causing_exception.exceptions) == 1
and causing_exception.exceptions[0] == exc_value
):
causing_exception = None

if causing_exception:
(exception_id, child_exceptions) = exceptions_from_error(
exc_type=type(causing_exception),
Expand Down
238 changes: 238 additions & 0 deletions tests/test_exceptiongroup.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import sys
from unittest import mock
import pytest

from sentry_sdk.utils import event_from_exception
Expand Down Expand Up @@ -315,3 +316,240 @@ def test_simple_exception():

exception_values = event["exception"]["values"]
assert exception_values == expected_exception_values


@minimum_python_311
def test_exceptiongroup_recursion():
exception_group = None

my_error = RuntimeError("my error")
try:
try:
raise my_error
except RuntimeError:
raise ExceptionGroup(
"my_group",
[my_error],
)
except ExceptionGroup as e:
exception_group = e

(event, _) = event_from_exception(
exception_group,
client_options={
"include_local_variables": True,
"include_source_context": True,
"max_value_length": 1024,
},
mechanism={"type": "test_suite", "handled": False},
)

values = event["exception"]["values"]

# For this test the stacktrace and the module is not important
for x in values:
if "stacktrace" in x:
del x["stacktrace"]
if "module" in x:
del x["module"]

# One ExceptionGroup,
# then the RuntimeError in the ExceptionGroup,
# and the original RuntimeError that was raised.
assert len(values) == 3

expected_values = [
{
"mechanism": {
"exception_id": 2,
"handled": False,
"parent_id": 0,
"source": "exceptions[0]",
"type": "chained",
},
"type": "RuntimeError",
"value": "my error",
},
{
"mechanism": {
"exception_id": 1,
"handled": False,
"parent_id": 0,
"source": "__context__",
"type": "chained",
},
"type": "RuntimeError",
"value": "my error",
},
{
"mechanism": {
"exception_id": 0,
"handled": False,
"is_exception_group": True,
"type": "test_suite",
},
"type": "ExceptionGroup",
"value": "my_group",
},
]

assert values == expected_values


@minimum_python_311
def test_exceptiongroup_recursion_multiple_levels():
error = None

my_error = RuntimeError("my error")
my_error_2 = RuntimeError("my error 2")
try:
try:
raise my_error
except RuntimeError:
try:
raise ExceptionGroup(
"my_group",
[my_error_2],
)
except ExceptionGroup:
raise my_error

except RuntimeError as e:
error = e

(event, _) = event_from_exception(
error,
client_options={
"include_local_variables": True,
"include_source_context": True,
"max_value_length": 1024,
},
mechanism={"type": "test_suite", "handled": False},
)

values = event["exception"]["values"]

# For this test the stacktrace and the module is not important
for x in values:
if "stacktrace" in x:
del x["stacktrace"]
if "module" in x:
del x["module"]

# One ExceptionGroup,
# then the RuntimeError in the ExceptionGroup,
# and the original RuntimeError that was raised.
assert len(values) == 3

expected_values = [
{
"mechanism": {
"type": "chained",
"handled": False,
"exception_id": 2,
"source": "exceptions[0]",
"parent_id": 1,
},
"type": "RuntimeError",
"value": "my error 2",
},
{
"mechanism": {
"type": "chained",
"handled": False,
"exception_id": 1,
"source": "__context__",
"parent_id": 0,
"is_exception_group": True,
},
"type": "ExceptionGroup",
"value": "my_group",
},
{
"mechanism": {
"type": "test_suite",
"handled": False,
"exception_id": 0,
},
"type": "RuntimeError",
"value": "my error",
},
]

assert values == expected_values


@minimum_python_311
def test_too_many_exceptions():
with mock.patch("sentry_sdk.utils.MAX_EXCEPTIONS", 3):
error = None
try:
try:
raise RuntimeError("my error 1")
except RuntimeError:
try:
raise RuntimeError("my error 2")
except RuntimeError:
try:
raise RuntimeError("my error 3")
except RuntimeError:
raise RuntimeError("my error 4")
except RuntimeError as e:
error = e

(event, _) = event_from_exception(
error,
client_options={
"include_local_variables": True,
"include_source_context": True,
"max_value_length": 1024,
},
mechanism={"type": "test_suite", "handled": False},
)

values = event["exception"]["values"]

# For this test the stacktrace and the module is not important
for x in values:
if "stacktrace" in x:
del x["stacktrace"]
if "module" in x:
del x["module"]

assert len(values) == 3

expected_values = [
{
"mechanism": {
"type": "chained",
"handled": False,
"exception_id": 2,
"source": "__context__",
"parent_id": 1,
},
"type": "RuntimeError",
"value": "my error 2",
},
{
"mechanism": {
"type": "chained",
"handled": False,
"exception_id": 1,
"source": "__context__",
"parent_id": 0,
},
"type": "RuntimeError",
"value": "my error 3",
},
{
"mechanism": {
"type": "test_suite",
"handled": False,
"exception_id": 0,
},
"type": "RuntimeError",
"value": "my error 4",
},
]

assert values == expected_values
Loading