Skip to content

Commit a88dadf

Browse files
antonpirkerantonpirker
authored
Fix FastAPI recursive exceptions (#4334)
In some cases FastAPI emits an exception that has as `__cause__` an ExceptionGroup that contains a single excpetion. That single exepction is the original exception. This PR prevents an infinite loop by trying to add this construct in the `exception.values` field. It also introduces an hard upper limit of chained/nested transaction, to never run into an infinite loop.
1 parent 8ee7dd4 commit a88dadf

File tree

2 files changed

+261
-0
lines changed

2 files changed

+261
-0
lines changed

‎sentry_sdk/utils.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,13 @@
8686
be affected by this limit if they have a custom recursion limit.
8787
"""
8888

89+
MAX_EXCEPTIONS = 25
90+
"""Maximum number of exceptions in a chain or group to send to Sentry.
91+
92+
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
93+
of the exception tree.
94+
"""
95+
8996

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

833+
if exception_id > MAX_EXCEPTIONS - 1:
834+
return (exception_id, exceptions)
835+
826836
causing_exception = None
827837
exception_source = None
828838

@@ -853,6 +863,19 @@ def exceptions_from_error(
853863
exception_source = "__context__"
854864
causing_exception = exc_value.__context__ # type: ignore
855865

866+
if causing_exception:
867+
# Some frameworks (e.g. FastAPI) wrap the causing exception in an
868+
# ExceptionGroup that only contain one exception: the causing exception.
869+
# This would lead to an infinite loop, so we skip the causing exception
870+
# in this case. (because it is the same as the base_exception above)
871+
if (
872+
BaseExceptionGroup is not None
873+
and isinstance(causing_exception, BaseExceptionGroup)
874+
and len(causing_exception.exceptions) == 1
875+
and causing_exception.exceptions[0] == exc_value
876+
):
877+
causing_exception = None
878+
856879
if causing_exception:
857880
(exception_id, child_exceptions) = exceptions_from_error(
858881
exc_type=type(causing_exception),

‎tests/test_exceptiongroup.py

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import sys
2+
from unittest import mock
23
import pytest
34

45
from sentry_sdk.utils import event_from_exception
@@ -315,3 +316,240 @@ def test_simple_exception():
315316

316317
exception_values = event["exception"]["values"]
317318
assert exception_values == expected_exception_values
319+
320+
321+
@minimum_python_311
322+
def test_exceptiongroup_recursion():
323+
exception_group = None
324+
325+
my_error = RuntimeError("my error")
326+
try:
327+
try:
328+
raise my_error
329+
except RuntimeError:
330+
raise ExceptionGroup(
331+
"my_group",
332+
[my_error],
333+
)
334+
except ExceptionGroup as e:
335+
exception_group = e
336+
337+
(event, _) = event_from_exception(
338+
exception_group,
339+
client_options={
340+
"include_local_variables": True,
341+
"include_source_context": True,
342+
"max_value_length": 1024,
343+
},
344+
mechanism={"type": "test_suite", "handled": False},
345+
)
346+
347+
values = event["exception"]["values"]
348+
349+
# For this test the stacktrace and the module is not important
350+
for x in values:
351+
if "stacktrace" in x:
352+
del x["stacktrace"]
353+
if "module" in x:
354+
del x["module"]
355+
356+
# One ExceptionGroup,
357+
# then the RuntimeError in the ExceptionGroup,
358+
# and the original RuntimeError that was raised.
359+
assert len(values) == 3
360+
361+
expected_values = [
362+
{
363+
"mechanism": {
364+
"exception_id": 2,
365+
"handled": False,
366+
"parent_id": 0,
367+
"source": "exceptions[0]",
368+
"type": "chained",
369+
},
370+
"type": "RuntimeError",
371+
"value": "my error",
372+
},
373+
{
374+
"mechanism": {
375+
"exception_id": 1,
376+
"handled": False,
377+
"parent_id": 0,
378+
"source": "__context__",
379+
"type": "chained",
380+
},
381+
"type": "RuntimeError",
382+
"value": "my error",
383+
},
384+
{
385+
"mechanism": {
386+
"exception_id": 0,
387+
"handled": False,
388+
"is_exception_group": True,
389+
"type": "test_suite",
390+
},
391+
"type": "ExceptionGroup",
392+
"value": "my_group",
393+
},
394+
]
395+
396+
assert values == expected_values
397+
398+
399+
@minimum_python_311
400+
def test_exceptiongroup_recursion_multiple_levels():
401+
error = None
402+
403+
my_error = RuntimeError("my error")
404+
my_error_2 = RuntimeError("my error 2")
405+
try:
406+
try:
407+
raise my_error
408+
except RuntimeError:
409+
try:
410+
raise ExceptionGroup(
411+
"my_group",
412+
[my_error_2],
413+
)
414+
except ExceptionGroup:
415+
raise my_error
416+
417+
except RuntimeError as e:
418+
error = e
419+
420+
(event, _) = event_from_exception(
421+
error,
422+
client_options={
423+
"include_local_variables": True,
424+
"include_source_context": True,
425+
"max_value_length": 1024,
426+
},
427+
mechanism={"type": "test_suite", "handled": False},
428+
)
429+
430+
values = event["exception"]["values"]
431+
432+
# For this test the stacktrace and the module is not important
433+
for x in values:
434+
if "stacktrace" in x:
435+
del x["stacktrace"]
436+
if "module" in x:
437+
del x["module"]
438+
439+
# One ExceptionGroup,
440+
# then the RuntimeError in the ExceptionGroup,
441+
# and the original RuntimeError that was raised.
442+
assert len(values) == 3
443+
444+
expected_values = [
445+
{
446+
"mechanism": {
447+
"type": "chained",
448+
"handled": False,
449+
"exception_id": 2,
450+
"source": "exceptions[0]",
451+
"parent_id": 1,
452+
},
453+
"type": "RuntimeError",
454+
"value": "my error 2",
455+
},
456+
{
457+
"mechanism": {
458+
"type": "chained",
459+
"handled": False,
460+
"exception_id": 1,
461+
"source": "__context__",
462+
"parent_id": 0,
463+
"is_exception_group": True,
464+
},
465+
"type": "ExceptionGroup",
466+
"value": "my_group",
467+
},
468+
{
469+
"mechanism": {
470+
"type": "test_suite",
471+
"handled": False,
472+
"exception_id": 0,
473+
},
474+
"type": "RuntimeError",
475+
"value": "my error",
476+
},
477+
]
478+
479+
assert values == expected_values
480+
481+
482+
@minimum_python_311
483+
def test_too_many_exceptions():
484+
with mock.patch("sentry_sdk.utils.MAX_EXCEPTIONS", 3):
485+
error = None
486+
try:
487+
try:
488+
raise RuntimeError("my error 1")
489+
except RuntimeError:
490+
try:
491+
raise RuntimeError("my error 2")
492+
except RuntimeError:
493+
try:
494+
raise RuntimeError("my error 3")
495+
except RuntimeError:
496+
raise RuntimeError("my error 4")
497+
except RuntimeError as e:
498+
error = e
499+
500+
(event, _) = event_from_exception(
501+
error,
502+
client_options={
503+
"include_local_variables": True,
504+
"include_source_context": True,
505+
"max_value_length": 1024,
506+
},
507+
mechanism={"type": "test_suite", "handled": False},
508+
)
509+
510+
values = event["exception"]["values"]
511+
512+
# For this test the stacktrace and the module is not important
513+
for x in values:
514+
if "stacktrace" in x:
515+
del x["stacktrace"]
516+
if "module" in x:
517+
del x["module"]
518+
519+
assert len(values) == 3
520+
521+
expected_values = [
522+
{
523+
"mechanism": {
524+
"type": "chained",
525+
"handled": False,
526+
"exception_id": 2,
527+
"source": "__context__",
528+
"parent_id": 1,
529+
},
530+
"type": "RuntimeError",
531+
"value": "my error 2",
532+
},
533+
{
534+
"mechanism": {
535+
"type": "chained",
536+
"handled": False,
537+
"exception_id": 1,
538+
"source": "__context__",
539+
"parent_id": 0,
540+
},
541+
"type": "RuntimeError",
542+
"value": "my error 3",
543+
},
544+
{
545+
"mechanism": {
546+
"type": "test_suite",
547+
"handled": False,
548+
"exception_id": 0,
549+
},
550+
"type": "RuntimeError",
551+
"value": "my error 4",
552+
},
553+
]
554+
555+
assert values == expected_values

0 commit comments

Comments
 (0)