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

* Fix: Handle exception with large stacktrace without dropping entire item

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 most of the stacktrace frames (except first 10) when payload is too large, so that the envelope item won't be dropped = exception still reported

* Update frame truncating to keep first & last N/2 frames instead of just last N frames

* Update CHANGELOG

Author: PikachuEXE

CHANGELOG.md

@@ -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

sentry-ruby/lib/sentry/transport.rb

@@ -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 = [
@@ -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
+
         if result.bytesize > Event::MAX_SERIALIZED_PAYLOAD_SIZE
           size_breakdown = item.payload.map do |key, value|
             "#{key}: #{JSON.generate(value).bytesize}"

sentry-ruby/spec/sentry/transport_spec.rb

@@ -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