Skip to content

Comments

fix: Don't hang when capturing long stacktrace#4191

Merged
szokeasaurusrex merged 4 commits intomasterfrom
szokeasaurusrex/recursion-depth
Mar 28, 2025
Merged

fix: Don't hang when capturing long stacktrace#4191
szokeasaurusrex merged 4 commits intomasterfrom
szokeasaurusrex/recursion-depth

Conversation

@szokeasaurusrex
Copy link
Member

@szokeasaurusrex szokeasaurusrex commented Mar 24, 2025

Fixes #2764

@szokeasaurusrex szokeasaurusrex requested a review from a team as a code owner March 24, 2025 14:49
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/recursion-depth branch 2 times, most recently from 47f7812 to 09b2f95 Compare March 24, 2025 15:02
@codecov
Copy link

codecov bot commented Mar 24, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 83.33333% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.55%. Comparing base (7406113) to head (335e9a3).
Report is 4 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/client.py 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #4191      +/-   ##
==========================================
+ Coverage   79.50%   79.55%   +0.05%     
==========================================
  Files         141      141              
  Lines       15738    15748      +10     
  Branches     2675     2679       +4     
==========================================
+ Hits        12512    12528      +16     
+ Misses       2378     2372       -6     
  Partials      848      848
Files with missing lines Coverage Δ
sentry_sdk/utils.py 84.14% <100.00%> (+0.15%) ⬆️
sentry_sdk/client.py 78.86% <0.00%> (-0.31%) ⬇️

... and 3 files with indirect coverage changes

@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/recursion-depth branch 2 times, most recently from 7e31f86 to 7642fc2 Compare March 25, 2025 08:54
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/recursion-depth branch from 7642fc2 to 2fa2d46 Compare March 25, 2025 09:07
sentrivana
sentrivana approved these changes Mar 26, 2025
View reviewed changes
Copy link
Contributor

@sentrivana sentrivana left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks ok, left a minor comment. I assume you've verified that relay/Sentry has no issues with a None stacktrace -- if not please do before merging.

@szokeasaurusrex
Copy link
Member Author

szokeasaurusrex commented Mar 26, 2025

I think this looks ok, left a minor comment. I assume you've verified that relay/Sentry has no issues with a None stacktrace -- if not please do before merging.

Yep, here is an example event with None stack trace. Interestingly, although the _meta is set correctly from the AnnotatedValue in the event JSON, this information is not shown in the Sentry UI. I am assuming this would need to be fixed/implemented in the Sentry UI though, don't think it's an SDK problem

@sentrivana
Copy link
Contributor

sentrivana commented Mar 26, 2025

Interestingly, although the _meta is set correctly from the AnnotatedValue in the event JSON, this information is not shown in the Sentry UI. I am assuming this would need to be fixed/implemented in the Sentry UI though, don't think it's an SDK problem

Also don't think so, afaik Sentry has been mostly ignoring the _meta field except for some cases (dropped spans come to mind as a recent example).

szokeasaurusrex
szokeasaurusrex commented Mar 26, 2025
View reviewed changes
sentry_sdk/utils.py
Comment on lines +744 to +758
# Process at most MAX_STACK_FRAMES + 1 frames, to avoid hanging on
# processing a super-long stacktrace.
for tb, _ in zip(iter_stacks(tb), range(MAX_STACK_FRAMES + 1))
] # type: List[Dict[str, Any]]

if frames:
if len(frames) > MAX_STACK_FRAMES:
# If we have more frames than the limit, we remove the stacktrace completely.
# We don't trim the stacktrace here because we have not processed the whole
# thing (see above, we stop at MAX_STACK_FRAMES + 1). Normally, Relay would
# intelligently trim by removing frames in the middle of the stacktrace, but
# since we don't have the whole stacktrace, we can't do that. Instead, we
# drop the entire stacktrace.
exception_value["stacktrace"] = AnnotatedValue.removed_because_over_size_limit(
value=None
)
Copy link
Member Author

@szokeasaurusrex szokeasaurusrex Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sentrivana what do you think about these comments?

Copy link
Contributor

@sentrivana sentrivana Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@szokeasaurusrex szokeasaurusrex merged commit e432fb4 into master Mar 28, 2025
139 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/recursion-depth branch March 28, 2025 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sentrivana sentrivana sentrivana approved these changes

@antonpirker antonpirker Awaiting requested review from antonpirker

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SDK stuck in processing RecursionError:maximum recursion depth exceeded events

3 participants

@szokeasaurusrex @sentrivana @antonpirker