feat: Raise payload size limit, refine size trimming (#431)

Improve report delivery by trimming payload more intelligently
in the case that the initial size is too large to be delivered

.rubocop_todo.yml

@@ -302,6 +302,8 @@ Metrics/MethodLength:
 # Configuration parameters: CountComments.
 Metrics/ModuleLength:
   Max: 125
+  Exclude:
+    - 'lib/bugsnag/helpers.rb'
 
 # Offense count: 11
 Metrics/PerceivedComplexity:

lib/bugsnag/helpers.rb

@@ -6,22 +6,35 @@
 module Bugsnag
   module Helpers
     MAX_STRING_LENGTH = 3072
-    MAX_PAYLOAD_LENGTH = 256000
-    MAX_ARRAY_LENGTH = 40
+    MAX_PAYLOAD_LENGTH = 512000
+    MAX_ARRAY_LENGTH = 80
+    MAX_TRIM_STACK_FRAMES = 30
     RAW_DATA_TYPES = [Numeric, TrueClass, FalseClass]
 
     ##
     # Trim the size of value if the serialized JSON value is longer than is
     # accepted by Bugsnag
     def self.trim_if_needed(value)
       value = "" if value.nil?
+
+      # Sanitize object
       sanitized_value = Bugsnag::Cleaner.clean_object_encoding(value)
       return sanitized_value unless payload_too_long?(sanitized_value)
-      reduced_value = trim_strings_in_value(sanitized_value)
+
+      # Trim metadata
+      reduced_value = trim_metadata(sanitized_value)
       return reduced_value unless payload_too_long?(reduced_value)
-      reduced_value = truncate_arrays_in_value(reduced_value)
+
+      # Trim code from stacktrace
+      reduced_value = trim_stacktrace_code(reduced_value)
       return reduced_value unless payload_too_long?(reduced_value)
-      remove_metadata_from_events(reduced_value)
+
+      # Remove metadata
+      reduced_value = remove_metadata_from_events(reduced_value)
+      return reduced_value unless payload_too_long?(reduced_value)
+
+      # Remove oldest functions in stacktrace
+      trim_stacktrace_functions(reduced_value)
     end
 
     ##
@@ -60,13 +73,56 @@ def self.deep_merge!(l_hash, r_hash)
 
     TRUNCATION_INFO = '[TRUNCATED]'
 
+    ##
+    # Remove all code from stacktraces
+    def self.trim_stacktrace_code(payload)
+      extract_exception(payload) do |exception|
+        exception[:stacktrace].each do |frame|
+          frame.delete(:code)
+        end
+      end
+      payload
+    end
+
+    ##
+    # Truncate stacktraces
+    def self.trim_stacktrace_functions(payload)
+      extract_exception(payload) do |exception|
+        stack = exception[:stacktrace]
+        exception[:stacktrace] = stack.take(MAX_TRIM_STACK_FRAMES)
+      end
+      payload
+    end
+
+    ##
+    # Wrapper for trimming stacktraces
+    def self.extract_exception(payload)
+      valid_payload = payload.is_a?(Hash) && payload[:events].respond_to?(:map)
+      return unless valid_payload && block_given?
+      payload[:events].each do |event|
+        event[:exceptions].each { |exception| yield exception }
+      end
+    end
+
+    ##
+    # Take the metadata from the events and trim it down
+    def self.trim_metadata(payload)
+      return payload unless payload.is_a?(Hash) and payload[:events].respond_to?(:map)
+      payload[:events].map do |event|
+        event[:metaData] = truncate_arrays_in_value(event[:metaData])
+        event[:metaData] = trim_strings_in_value(event[:metaData])
+      end
+      payload
+    end
+
     ##
     # Check if a value is a raw type which should not be trimmed, truncated
     # or converted to a string
     def self.is_json_raw_type?(value)
       RAW_DATA_TYPES.detect {|klass| value.is_a?(klass)} != nil
     end
 
+    ##
     # Shorten array until it fits within the payload size limit when serialized
     def self.truncate_array(array)
       return [] unless array.respond_to?(:slice)
@@ -75,6 +131,7 @@ def self.truncate_array(array)
       end
     end
 
+    ##
     # Trim all strings to be less than the maximum allowed string length
     def self.trim_strings_in_value(value)
       return value if is_json_raw_type?(value)
@@ -88,13 +145,18 @@ def self.trim_strings_in_value(value)
       end
     end
 
+    ##
     # Validate that the serialized JSON string value is below maximum payload
     # length
     def self.payload_too_long?(value)
+      get_payload_length(value) >= MAX_PAYLOAD_LENGTH
+    end
+
+    def self.get_payload_length(value)
       if value.is_a?(String)
-        value.length >= MAX_PAYLOAD_LENGTH
+        value.length
       else
-        ::JSON.dump(value).length >= MAX_PAYLOAD_LENGTH
+        ::JSON.dump(value).length
       end
     end
 

lib/bugsnag/stacktrace.rb

@@ -16,6 +16,7 @@ def initialize(backtrace, configuration)
       @configuration = configuration
 
       backtrace = caller if !backtrace || backtrace.empty?
+
       @processed_backtrace = backtrace.map do |trace|
         if trace.match(BACKTRACE_LINE_REGEX)
           file, line_str, method = [$1, $2, $3]

spec/helper_spec.rb

@@ -72,103 +72,89 @@ def to_s
 
     context "payload length is greater than allowed" do
 
-      context "value is a String" do
-        it "trims length" do
-          value = Bugsnag::Helpers.trim_if_needed(SecureRandom.hex(500_000/2))
-          expect(value.length).to be <= Bugsnag::Helpers::MAX_STRING_LENGTH
-        end
-      end
-
-      context "value is an Array" do
-        it "trims nested string contents" do
-          value = [[30.times.map {|i| SecureRandom.hex(8192) }]]
-          json = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(value))
-          expect(json.length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-        end
-
-        it "trims string contents" do
-          value = 30.times.map {|i| SecureRandom.hex(8192) }
-          json = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(value))
-          expect(json.length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-        end
+      it "trims metadata strings" do
+        payload = {
+          :events => [{
+            :metaData => 50000.times.map {|i| "should truncate" }.join(""),
+            :preserved => "Foo"
+          }]
+        }
+        expect(::JSON.dump(payload).length).to be > Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trimmed = Bugsnag::Helpers.trim_if_needed(payload)
+        expect(::JSON.dump(trimmed).length).to be <= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        expect(trimmed[:events][0][:metaData].length).to be <= Bugsnag::Helpers::MAX_STRING_LENGTH
+        expect(trimmed[:events][0][:preserved]).to eq("Foo")
       end
 
-      context "value is a Set" do
-        it "trims string contents" do
-          value = Set.new(30.times.map {|i| SecureRandom.hex(8192) })
-          json = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(value))
-          expect(json.length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-        end
+      it "truncates metadata arrays" do
+        payload = {
+          :events => [{
+            :metaData => 50000.times.map {|i| "should truncate" },
+            :preserved => "Foo"
+          }]
+        }
+        expect(::JSON.dump(payload).length).to be > Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trimmed = Bugsnag::Helpers.trim_if_needed(payload)
+        expect(::JSON.dump(trimmed).length).to be <= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        expect(trimmed[:events][0][:metaData].length).to be <= Bugsnag::Helpers::MAX_ARRAY_LENGTH
+        expect(trimmed[:events][0][:preserved]).to eq("Foo")
       end
 
-      context "value can be converted to a String" do
-        it "converts to a string and trims" do
-          value = Set.new(30_000.times.map {|i| Bugsnag::Helpers })
-          json = ::JSON.dump(Bugsnag::Helpers.trim_if_needed(value))
-          expect(json.length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-        end
+      it "trims stacktrace code" do
+        payload = {
+          :events => [{
+            :exceptions => [{
+              :stacktrace => [
+                {
+                  :lineNumber => 1,
+                  :file => '/trace1',
+                  :code => 50.times.map {|i| SecureRandom.hex(3072) }
+                },
+                {
+                  :lineNumber => 2,
+                  :file => '/trace2',
+                  :code => 50.times.map {|i| SecureRandom.hex(3072) }
+                }
+              ]
+            }]
+          }]
+        }
+        expect(::JSON.dump(payload).length).to be > Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trimmed = Bugsnag::Helpers.trim_if_needed(payload)
+        expect(::JSON.dump(trimmed).length).to be <= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trace = trimmed[:events][0][:exceptions][0][:stacktrace]
+        expect(trace.length).to eq(2)
+        expect(trace[0][:lineNumber]).to eq(1)
+        expect(trace[0][:file]).to eq('/trace1')
+        expect(trace[0][:code]).to be_nil
+        expect(trace[1][:lineNumber]).to eq(2)
+        expect(trace[1][:file]).to eq('/trace2')
+        expect(trace[1][:code]).to be_nil
       end
 
-      context "value is a Hash" do
-
-        before(:each) do
-          @metadata = {
-            :short_string => "this should not be truncated",
-            :long_string => 10000.times.map {|i| "should truncate" }.join(""),
-            :long_string_ary => 30.times.map {|i| SecureRandom.hex(8192) }
-          }
-
-          @trimmed_metadata = Bugsnag::Helpers.trim_if_needed @metadata
-        end
-
-        it "does not trim short values" do
-          expect(@trimmed_metadata[:short_string]).to eq @metadata[:short_string]
-        end
-
-        it "trims long string values" do
-          expect(@trimmed_metadata[:long_string].length).to eq(Bugsnag::Helpers::MAX_STRING_LENGTH)
-          expect(@trimmed_metadata[:long_string].match(/\[TRUNCATED\]$/)).to_not be_nil
-        end
-
-        it "trims nested long string values" do
-          @trimmed_metadata[:long_string_ary].each do |str|
-            expect(str.match(/\[TRUNCATED\]$/)).to_not be_nil
-            expect(str.length).to eq(Bugsnag::Helpers::MAX_STRING_LENGTH)
-          end
-        end
-
-        it "does not change the argument value" do
-          expect(@metadata[:long_string].length).to be > Bugsnag::Helpers::MAX_STRING_LENGTH
-          expect(@metadata[:long_string].match(/\[TRUNCATED\]$/)).to be_nil
-          expect(@metadata[:short_string].length).to eq(28)
-          expect(@metadata[:short_string]).to eq("this should not be truncated")
-          expect(@trimmed_metadata[:long_string_ary].length).to eq(30)
-        end
-      end
-
-      context "and trimmed strings are not enough" do
-        it "truncates long arrays" do
-          value = [100.times.map {|i| SecureRandom.hex(8192) }, "a"]
-          trimmed_value = Bugsnag::Helpers.trim_if_needed(value)
-          expect(trimmed_value.length).to eq 2
-          expect(trimmed_value.first.length).to eq Bugsnag::Helpers::MAX_ARRAY_LENGTH
-          trimmed_value.first.each do |str|
-            expect(str.match(/\[TRUNCATED\]$/)).to_not be_nil
-            expect(str.length).to eq(Bugsnag::Helpers::MAX_STRING_LENGTH)
-          end
-
-          expect(::JSON.dump(trimmed_value).length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-        end
-
-        it "removes metadata from events" do
-          metadata = Hash[*20000.times.map {|i| [i,i+1]}.flatten]
-          frames = 100.times.map {|i| SecureRandom.hex(4096) }
-          value = {key:"abc", events:[{metaData: metadata, frames: frames, cake: "carrot"}]}
-          trimmed_value = Bugsnag::Helpers.trim_if_needed(value)
-          expect(::JSON.dump(trimmed_value).length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
-          expect(trimmed_value[:key]).to eq value[:key]
-          expect(trimmed_value[:events].first.keys.to_set).to eq [:frames, :cake].to_set
-          expect(trimmed_value[:events].first[:metaData]).to be_nil
+      it "trims stacktrace entries" do
+        payload = {
+          :events => [{
+            :exceptions => [{
+              :stacktrace => 18000.times.map do |index|
+                {
+                  :lineNumber => index,
+                  :file => "/path/to/item_#{index}.rb",
+                  :code => { "#{index}" => "puts 'code'" }
+                }
+              end
+            }]
+          }]
+        }
+        expect(::JSON.dump(payload).length).to be > Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trimmed = Bugsnag::Helpers.trim_if_needed(payload)
+        expect(::JSON.dump(trimmed).length).to be <= Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
+        trace = trimmed[:events][0][:exceptions][0][:stacktrace]
+        expect(trace.length).to eq(30)
+        30.times.map do |index|
+          expect(trace[index][:lineNumber]).to eq(index)
+          expect(trace[index][:file]).to eq("/path/to/item_#{index}.rb")
+          expect(trace[index][:code]).to be_nil
         end
       end
     end

spec/report_spec.rb

@@ -367,8 +367,8 @@ def gloops
     Bugsnag.notify(BugsnagTestException.new("It crashed")) do |report|
       report.meta_data.merge!({
         some_tab: {
-          giant: SecureRandom.hex(500_000/2),
-          mega: SecureRandom.hex(500_000/2)
+          giant: SecureRandom.hex(1_000_000/2),
+          mega: SecureRandom.hex(1_000_000/2)
         }
       })
     end
@@ -381,7 +381,7 @@ def gloops
   end
 
   it "truncates large messages before sending" do
-    Bugsnag.notify(BugsnagTestException.new(SecureRandom.hex(500_000))) do |report|
+    Bugsnag.notify(BugsnagTestException.new(SecureRandom.hex(250_000))) do |report|
       report.meta_data.merge!({
         some_tab: {
           giant: SecureRandom.hex(500_000/2),
@@ -391,23 +391,21 @@ def gloops
     end
 
     expect(Bugsnag).to have_sent_notification{ |payload, headers|
-      # Truncated body should be no bigger than
-      # 2 truncated hashes (4096*2) + rest of payload (20000)
-      expect(::JSON.dump(payload).length).to be < 4096*2 + 20000
+      expect(::JSON.dump(payload).length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
     }
   end
 
   it "truncate large stacktraces before sending" do
     ex = BugsnagTestException.new("It crashed")
     stacktrace = []
-    5000.times {|i| stacktrace.push("/Some/path/rspec/example.rb:113:in `instance_eval'")}
+    20000.times {|i| stacktrace.push("/Some/path/rspec/example.rb:113:in `instance_eval'")}
     ex.set_backtrace(stacktrace)
     Bugsnag.notify(ex)
 
     expect(Bugsnag).to have_sent_notification{ |payload, headers|
       # Truncated body should be no bigger than
       # 400 stacktrace lines * approx 60 chars per line + rest of payload (20000)
-      expect(::JSON.dump(payload).length).to be < 400*60 + 20000
+      expect(::JSON.dump(payload).length).to be < Bugsnag::Helpers::MAX_PAYLOAD_LENGTH
     }
   end
 
@@ -1054,10 +1052,10 @@ def gloops
       expect(exception["message"]).to eq("'nil' was notified as an exception")
 
       stacktrace = exception["stacktrace"][0]
-      expect(stacktrace["lineNumber"]).to eq(1049)
+      expect(stacktrace["lineNumber"]).to eq(1047)
       expect(stacktrace["file"]).to end_with("spec/report_spec.rb")
-      expect(stacktrace["code"]["1048"]).to eq("  it 'uses an appropriate message if nil is notified' do")
-      expect(stacktrace["code"]["1049"]).to eq("    Bugsnag.notify(nil)")
+      expect(stacktrace["code"]["1046"]).to eq("  it 'uses an appropriate message if nil is notified' do")
+      expect(stacktrace["code"]["1047"]).to eq("    Bugsnag.notify(nil)")
     }
   end