Skip to content

Conversation

@PikachuEXE
Copy link
Contributor

@PikachuEXE PikachuEXE commented May 3, 2022

Description

This should fix #1799

Certain exception type such as SystemStackError has long backtrace (thus the stack error)
The whole envelope item was dropped due to payload size limit logic

This ensures it tries to remove the stacktrace when payload is too large, so that the envelope item won't be dropped = exception still reported

Maybe it's better to keep some frames like #1807 instead of removing whole stacktrace

@PikachuEXE PikachuEXE marked this pull request as ready for review May 3, 2022 07:32
@PikachuEXE PikachuEXE changed the title Fix: Handle exception with large stacktrace without dropping entire item Fix: Handle exception with large stacktrace without dropping entire item - by removing stacktrace May 3, 2022
PikachuEXE added 2 commits May 3, 2022 16:07
Certain exception type such as `SystemStackError` has long backtrace (thus the stack error)
The whole envelope item was dropped due to payload size limit logic

This ensures it tries to remove the stacktrace when payload is too large, so that the envelope item won't be dropped = exception still reported
@PikachuEXE PikachuEXE force-pushed the fix/large-stacktrace-handling branch from e146277 to 3a37b01 Compare May 3, 2022 08:07
Copy link
Contributor

@st0012 st0012 left a comment

Choose a reason for hiding this comment

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

I'm not sure if remove the entire stacktrace because that'll leave the error event almost useless. Perhaps leaving 20 frames is more ideal?
If the event is still oversized without breadcrumbs and has only 20 frames, the problem is probably somewhere else.

@sl0thentr0py Wdyt?


Individual gem's changelog has been deprecated. Please check the [project changelog](https://github.com/getsentry/sentry-ruby/blob/master/CHANGELOG.md).

## Unreleased
Copy link
Contributor

Choose a reason for hiding this comment

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

This changelog has been deprecated. Can you update the top-level one instead?

single_exceptions.each do |single_exception|
traces = single_exception.dig("stacktrace", "frames")
if traces && traces.size > 0
single_exception.delete("stacktrace")
Copy link
Contributor

Choose a reason for hiding this comment

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

I also don't have an answer for this yet, but I'd like to avoid duplicating all the payload truncation logic because of key types.

Copy link
Member

@sl0thentr0py sl0thentr0py May 3, 2022

Choose a reason for hiding this comment

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

python has some overly general serialization logic that drops fields based on nested hash breadth/depth/byte length logic, but I don't like that so much either because it's really hard to read.


result = item.to_s
end

Copy link
Contributor

Choose a reason for hiding this comment

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

I can't comment lower so I just leave it here: we also need to update the message form:

is still oversized without breadcrumbs

to

is still oversized without breadcrumbs and stracktrace

@sl0thentr0py
Copy link
Member

@st0012 @PikachuEXE yes I also prefer #1807 (keeping N frames) over removing the entire field.

@st0012
Copy link
Contributor

st0012 commented May 3, 2022

Closing this in favor of #1807

@st0012 st0012 closed this May 3, 2022
@PikachuEXE PikachuEXE deleted the fix/large-stacktrace-handling branch May 4, 2022 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SystemStackError exceptions dropped by payload truncation logic

3 participants