Skip to content

Commit e432fb4

Browse files
fix: Don't hang when capturing long stacktrace (#4191)
Fixes #2764 --------- Co-authored-by: Anton Pirker <[email protected]>
1 parent 2f4b028 commit e432fb4

File tree

4 files changed

+85
-8
lines changed

4 files changed

+85
-8
lines changed

sentry_sdk/_types.py

+7-4
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,14 @@ def removed_because_raw_data(cls):
4747
)
4848

4949
@classmethod
50-
def removed_because_over_size_limit(cls):
51-
# type: () -> AnnotatedValue
52-
"""The actual value was removed because the size of the field exceeded the configured maximum size (specified with the max_request_body_size sdk option)"""
50+
def removed_because_over_size_limit(cls, value=""):
51+
# type: (Any) -> AnnotatedValue
52+
"""
53+
The actual value was removed because the size of the field exceeded the configured maximum size,
54+
for example specified with the max_request_body_size sdk option.
55+
"""
5356
return AnnotatedValue(
54-
value="",
57+
value=value,
5558
metadata={
5659
"rem": [ # Remark
5760
[

sentry_sdk/client.py

+2
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,8 @@ def _update_session_from_event(
755755
if exceptions:
756756
errored = True
757757
for error in exceptions:
758+
if isinstance(error, AnnotatedValue):
759+
error = error.value or {}
758760
mechanism = error.get("mechanism")
759761
if isinstance(mechanism, Mapping) and mechanism.get("handled") is False:
760762
crashed = True

sentry_sdk/utils.py

+32-4
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@
7777
FALSY_ENV_VALUES = frozenset(("false", "f", "n", "no", "off", "0"))
7878
TRUTHY_ENV_VALUES = frozenset(("true", "t", "y", "yes", "on", "1"))
7979

80+
MAX_STACK_FRAMES = 2000
81+
"""Maximum number of stack frames to send to Sentry.
82+
83+
If we have more than this number of stack frames, we will stop processing
84+
the stacktrace to avoid getting stuck in a long-lasting loop. This value
85+
exceeds the default sys.getrecursionlimit() of 1000, so users will only
86+
be affected by this limit if they have a custom recursion limit.
87+
"""
88+
8089

8190
def env_to_bool(value, *, strict=False):
8291
# type: (Any, Optional[bool]) -> bool | None
@@ -732,10 +741,23 @@ def single_exception_from_error_tuple(
732741
max_value_length=max_value_length,
733742
custom_repr=custom_repr,
734743
)
735-
for tb in iter_stacks(tb)
744+
# Process at most MAX_STACK_FRAMES + 1 frames, to avoid hanging on
745+
# processing a super-long stacktrace.
746+
for tb, _ in zip(iter_stacks(tb), range(MAX_STACK_FRAMES + 1))
736747
] # type: List[Dict[str, Any]]
737748

738-
if frames:
749+
if len(frames) > MAX_STACK_FRAMES:
750+
# If we have more frames than the limit, we remove the stacktrace completely.
751+
# We don't trim the stacktrace here because we have not processed the whole
752+
# thing (see above, we stop at MAX_STACK_FRAMES + 1). Normally, Relay would
753+
# intelligently trim by removing frames in the middle of the stacktrace, but
754+
# since we don't have the whole stacktrace, we can't do that. Instead, we
755+
# drop the entire stacktrace.
756+
exception_value["stacktrace"] = AnnotatedValue.removed_because_over_size_limit(
757+
value=None
758+
)
759+
760+
elif frames:
739761
if not full_stack:
740762
new_frames = frames
741763
else:
@@ -941,7 +963,7 @@ def to_string(value):
941963

942964

943965
def iter_event_stacktraces(event):
944-
# type: (Event) -> Iterator[Dict[str, Any]]
966+
# type: (Event) -> Iterator[Annotated[Dict[str, Any]]]
945967
if "stacktrace" in event:
946968
yield event["stacktrace"]
947969
if "threads" in event:
@@ -950,20 +972,26 @@ def iter_event_stacktraces(event):
950972
yield thread["stacktrace"]
951973
if "exception" in event:
952974
for exception in event["exception"].get("values") or ():
953-
if "stacktrace" in exception:
975+
if isinstance(exception, dict) and "stacktrace" in exception:
954976
yield exception["stacktrace"]
955977

956978

957979
def iter_event_frames(event):
958980
# type: (Event) -> Iterator[Dict[str, Any]]
959981
for stacktrace in iter_event_stacktraces(event):
982+
if isinstance(stacktrace, AnnotatedValue):
983+
stacktrace = stacktrace.value or {}
984+
960985
for frame in stacktrace.get("frames") or ():
961986
yield frame
962987

963988

964989
def handle_in_app(event, in_app_exclude=None, in_app_include=None, project_root=None):
965990
# type: (Event, Optional[List[str]], Optional[List[str]], Optional[str]) -> Event
966991
for stacktrace in iter_event_stacktraces(event):
992+
if isinstance(stacktrace, AnnotatedValue):
993+
stacktrace = stacktrace.value or {}
994+
967995
set_in_app_in_frames(
968996
stacktrace.get("frames"),
969997
in_app_exclude=in_app_exclude,

tests/test_basics.py

+44
Original file line numberDiff line numberDiff line change
@@ -1065,3 +1065,47 @@ def __str__(self):
10651065
(event,) = events
10661066

10671067
assert event["exception"]["values"][0]["value"] == "aha!\nnote 1\nnote 3"
1068+
1069+
1070+
@pytest.mark.skipif(
1071+
sys.version_info < (3, 11),
1072+
reason="this test appears to cause a segfault on Python < 3.11",
1073+
)
1074+
def test_stacktrace_big_recursion(sentry_init, capture_events):
1075+
"""
1076+
Ensure that if the recursion limit is increased, the full stacktrace is not captured,
1077+
as it would take too long to process the entire stack trace.
1078+
Also, ensure that the capturing does not take too long.
1079+
"""
1080+
sentry_init()
1081+
events = capture_events()
1082+
1083+
def recurse():
1084+
recurse()
1085+
1086+
old_recursion_limit = sys.getrecursionlimit()
1087+
1088+
try:
1089+
sys.setrecursionlimit(100_000)
1090+
recurse()
1091+
except RecursionError as e:
1092+
capture_start_time = time.perf_counter_ns()
1093+
sentry_sdk.capture_exception(e)
1094+
capture_end_time = time.perf_counter_ns()
1095+
finally:
1096+
sys.setrecursionlimit(old_recursion_limit)
1097+
1098+
(event,) = events
1099+
1100+
assert event["exception"]["values"][0]["stacktrace"] is None
1101+
assert event["_meta"] == {
1102+
"exception": {
1103+
"values": {"0": {"stacktrace": {"": {"rem": [["!config", "x"]]}}}}
1104+
}
1105+
}
1106+
1107+
# On my machine, it takes about 100-200ms to capture the exception,
1108+
# so this limit should be generous enough.
1109+
assert (
1110+
capture_end_time - capture_start_time < 10**9
1111+
), "stacktrace capture took too long, check that frame limit is set correctly"

0 commit comments

Comments
 (0)