8000 fix: Don't hang when capturing long stacktrace · getsentry/sentry-python@2fa2d46 · GitHub
[go: up one dir, main page]
Skip to content

Commit 2fa2d46

Browse files
szokeasaurusrexszokeasaurusrex
committed
fix: Don't hang when capturing long stacktrace
Fixes #2764
1 parent 08bbe00 commit 2fa2d46

File tree

4 files changed

+81
-12
lines changed

4 files changed

+81
-12
lines changed

‎sentry_sdk/_types.py

Lines changed: 8 additions & 5 deletions
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
[
@@ -160,7 +163,7 @@ class SDKInfo(TypedDict):
160163
"errors": list[dict[str, Any]], # TODO: We can expand on this type
161164
"event_id": str,
162165
"exception": dict[
163-
Literal["values"], list[dict[str, Any]]
166+
Literal["values"], list[Annotated[dict[str, Any]]]
164167
], # TODO: We can expand on this type
165168
"extra": MutableMapping[str, object],
166169
"fingerprint": list[str],

‎sentry_sdk/client.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -756,6 +756,8 @@ def _update_session_from_event(
756756
if exceptions:
757757
errored = True
758758
for error in exceptions:
759+
if isinstance(error, AnnotatedValue):
760+
error = error.value or {}
759761
mechanism = error.get("mechanism")
760762
if isinstance(mechanism, Mapping) and mechanism.get("handled") is False:
761763
crashed = True

‎sentry_sdk/utils.py

Lines changed: 27 additions & 7 deletions
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
@@ -667,7 +676,7 @@ def single_exception_from_error_tuple(
667676
source=None, # type: Optional[str]
668677
full_stack=None, # type: Optional[list[dict[str, Any]]]
669678
):
670-
# type: (...) -> Dict[str, Any]
679+
# type: (...) -> Annotated[Dict[str, Any]]
671680
"""
672681
Creates a dict that goes into the events `exception.values` list and is ingestible by Sentry.
673682
@@ -732,10 +741,15 @@ 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+
for tb, _ in zip(iter_stacks(tb), range(MAX_STACK_FRAMES + 1))
736745
] # type: List[Dict[str, Any]]
737746

738-
if frames:
747+
if len(frames) > MAX_STACK_FRAMES:
748+
exception_value["stacktrace"] = AnnotatedValue.removed_because_over_size_limit(
749+
value=None
750+
)
751+
752+
elif frames:
739753
if not full_stack:
740754
new_frames = frames
741755
else:
@@ -798,7 +812,7 @@ def exceptions_from_error(
798812
source=None, # type: Optional[str]
799813
full_stack=None, # type: Optional[list[dict[str, Any]]]
800814
):
801-
# type: (...) -> Tuple[int, List[Dict[str, Any]]]
815+
# type: (...) -> Tuple[int, List[Annotated[Dict[str, Any]]]]
802816
"""
803817
Creates the list of exceptions.
804818
This can include chained exceptions and exceptions from an ExceptionGroup.
@@ -894,7 +908,7 @@ def exceptions_from_error_tuple(
894908
mechanism=None, # type: Optional[Dict[str, Any]]
895909
full_stack=None, # type: Optional[list[dict[str, Any]]]
896910
):
897-
# type: (...) -> List[Dict[str, Any]]
911+
# type: (...) -> List[Annotated[Dict[str, Any]]]
898912
exc_type, exc_value, tb = exc_info
899913

900914
is_exception_group = BaseExceptionGroup is not None and isinstance(
@@ -941,7 +955,7 @@ def to_string(value):
941955

942956

943957
def iter_event_stacktraces(event):
944-
# type: (Event) -> Iterator[Dict[str, Any]]
958+
# type: (Event) -> Iterator[Annotated[Dict[str, Any]]]
945959
if "stacktrace" in event:
946960
yield event["stacktrace"]
947961
if "threads" in event:
@@ -950,20 +964,26 @@ def iter_event_stacktraces(event):
950964
yield thread["stacktrace"]
951965
if "exception" in event:
952966
for exception in event["exception"].get("values") or ():
953-
if "stacktrace" in exception:
967+
if isinstance(exception, dict) and "stacktrace" in exception:
954968
yield exception["stacktrace"]
955969

956970

957971
def iter_event_frames(event):
958972
# type: (Event) -> Iterator[Dict[str, Any]]
959973
for stacktrace in iter_event_stacktraces(event):
974+
if isinstance(stacktrace, AnnotatedValue):
975+
stacktrace = stacktrace.value or {}
976+
960977
for frame in stacktrace.get("frames") or ():
961978
yield frame
962979

963980

964981
def handle_in_app(event, in_app_exclude=None, in_app_include=None, project_root=None):
965982
# type: (Event, Optional[List[str]], Optional[List[str]], Optional[str]) -> Event
966983
for stacktrace in iter_event_stacktraces(event):
984+
if isinstance(stacktrace, AnnotatedValue):
985+
stacktrace = stacktrace.value or {}
986+
967987
set_in_app_in_frames(
968988
stacktrace.get("frames"),
969989
in_app_exclude=in_app_exclude,

‎tests/test_basics.py

Lines changed: 44 additions & 0 deletions
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)
0