Skip to content

Fix: Handle exception with large stacktrace without dropping entire item - by keeping N frames #1807

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## Unreleased

### Features

- Handle exception with large stacktrace without dropping entire item [#1807](https://github.com/getsentry/sentry-ruby/pull/1807)

## 5.3.1

### Bug Fixes
Expand Down
27 changes: 27 additions & 0 deletions sentry-ruby/lib/sentry/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class Transport
PROTOCOL_VERSION = '7'
USER_AGENT = "sentry-ruby/#{Sentry::VERSION}"
CLIENT_REPORT_INTERVAL = 30
STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD = 500

# https://develop.sentry.dev/sdk/client-reports/#envelope-item-payload
CLIENT_REPORT_REASONS = [
Expand Down Expand Up @@ -82,6 +83,32 @@ def serialize_envelope(envelope)
result = item.to_s
end

if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
if single_exceptions = item.payload.dig(:exception, :values)
single_exceptions.each do |single_exception|
traces = single_exception.dig(:stacktrace, :frames)
if traces && traces.size > STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD
size_on_both_ends = STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD / 2
traces.replace(
traces[0..(size_on_both_ends - 1)] + traces[-size_on_both_ends..-1],
)
end
end
elsif single_exceptions = item.payload.dig("exception", "values")
single_exceptions.each do |single_exception|
traces = single_exception.dig("stacktrace", "frames")
if traces && traces.size > STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD
size_on_both_ends = STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD / 2
traces.replace(
traces[0..(size_on_both_ends - 1)] + traces[-size_on_both_ends..-1],
)
end
end
end

result = item.to_s
end

Copy link
Collaborator

Choose a reason for hiding this comment

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

As mentioned in #1806 (comment), we should update the log message as well (see line 106).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should it be updated to since this PR remove some frames only (or keeping some frames)

if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
size_breakdown = item.payload.map do |key, value|
"#{key}: #{JSON.generate(value).bytesize}"
Expand Down
121 changes: 98 additions & 23 deletions sentry-ruby/spec/sentry/transport_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,40 +100,115 @@
end

context "oversized event" do
let(:event) { client.event_from_message("foo") }
let(:envelope) { subject.envelope_from_event(event) }
context "due to breadcrumb" do
let(:event) { client.event_from_message("foo") }
let(:envelope) { subject.envelope_from_event(event) }

before do
event.breadcrumbs = Sentry::BreadcrumbBuffer.new(100)
100.times do |i|
event.breadcrumbs.record Sentry::Breadcrumb.new(category: i.to_s, message: "x" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES)
before do
event.breadcrumbs = Sentry::BreadcrumbBuffer.new(100)
100.times do |i|
event.breadcrumbs.record Sentry::Breadcrumb.new(category: i.to_s, message: "x" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES)
end
serialized_result = JSON.generate(event.to_hash)
expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
end
serialized_result = JSON.generate(event.to_hash)
expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
end

it "removes breadcrumbs and carry on" do
data, _ = subject.serialize_envelope(envelope)
expect(data.bytesize).to be < Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
it "removes breadcrumbs and carry on" do
data, _ = subject.serialize_envelope(envelope)
expect(data.bytesize).to be < Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE

expect(envelope.items.count).to eq(1)

event_item = envelope.items.first
expect(event_item.payload[:breadcrumbs]).to be_nil
end

expect(envelope.items.count).to eq(1)
context "if it's still oversized" do
before do
100.times do |i|
event.contexts["context_#{i}"] = "s" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES
end
end

event_item = envelope.items.first
expect(event_item.payload[:breadcrumbs]).to be_nil
it "rejects the item and logs attributes size breakdown" do
data, _ = subject.serialize_envelope(envelope)
expect(data).to be_nil
expect(io.string).not_to match(/Sending envelope with items \[event\]/)
expect(io.string).to match(/tags: 2, contexts: 820791, extra: 2/)
end
end
end

context "if it's still oversized" do
context "due to stacktrace frames" do
let(:event) { client.event_from_exception(SystemStackError.new("stack level too deep")) }
let(:envelope) { subject.envelope_from_event(event) }

let(:in_app_pattern) do
project_root = "/fake/project_root"
Regexp.new("^(#{project_root}/)?#{Sentry::Backtrace::APP_DIRS_PATTERN}")
end
let(:frame_list_limit) { Sentry::Transport::STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD }
let(:frame_list_size) { frame_list_limit * 4 }

before do
100.times do |i|
event.contexts["context_#{i}"] = "s" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES
end
single_exception = event.exception.instance_variable_get(:@values)[0]
new_stacktrace = Sentry::StacktraceInterface.new(
frames: frame_list_size.times.map do |zero_based_index|
Sentry::StacktraceInterface::Frame.new(
"/fake/path",
Sentry::Backtrace::Line.parse("app.rb:#{zero_based_index + 1}:in `/'", in_app_pattern)
)
end,
)
single_exception.instance_variable_set(:@stacktrace, new_stacktrace)

serialized_result = JSON.generate(event.to_hash)
expect(serialized_result.bytesize).to be > Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE
end

it "rejects the item and logs attributes size breakdown" do
it "keeps some stacktrace frames and carry on" do
data, _ = subject.serialize_envelope(envelope)
expect(data).to be_nil
expect(io.string).not_to match(/Sending envelope with items \[event\]/)
expect(io.string).to match(/tags: 2, contexts: 820791, extra: 2/)
expect(data.bytesize).to be < Sentry::Event::MAX_SERIALIZED_PAYLOAD_SIZE

expect(envelope.items.count).to eq(1)

event_item = envelope.items.first
frames = event_item.payload[:exception][:values][0][:stacktrace][:frames]
expect(frames.length).to eq(frame_list_limit)

# Last N lines kept
# N = Frame limit / 2
expect(frames[-1][:lineno]).to eq(frame_list_size)
expect(frames[-1][:filename]).to eq('app.rb')
expect(frames[-1][:function]).to eq('/')
#
expect(frames[-(frame_list_limit / 2)][:lineno]).to eq(frame_list_size - ((frame_list_limit / 2) - 1))
expect(frames[-(frame_list_limit / 2)][:filename]).to eq('app.rb')
expect(frames[-(frame_list_limit / 2)][:function]).to eq('/')

# First N lines kept
# N = Frame limit / 2
expect(frames[0][:lineno]).to eq(1)
expect(frames[0][:filename]).to eq('app.rb')
expect(frames[0][:function]).to eq('/')
expect(frames[(frame_list_limit / 2) - 1][:lineno]).to eq(frame_list_limit / 2)
expect(frames[(frame_list_limit / 2) - 1][:filename]).to eq('app.rb')
expect(frames[(frame_list_limit / 2) - 1][:function]).to eq('/')
end

context "if it's still oversized" do
before do
100.times do |i|
event.contexts["context_#{i}"] = "s" * Sentry::Event::MAX_MESSAGE_SIZE_IN_BYTES
end
end

it "rejects the item and logs attributes size breakdown" do
data, _ = subject.serialize_envelope(envelope)
expect(data).to be_nil
expect(io.string).not_to match(/Sending envelope with items \[event\]/)
expect(io.string).to match(/tags: 2, contexts: 820791, extra: 2/)
end
end
end
end
Expand Down