WIP: Up limits on payload lengths, more focussed trimming

.rubocop_todo.yml

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

lib/bugsnag/helpers.rb

@@ -6,22 +6,32 @@
 module Bugsnag
   module Helpers
     MAX_STRING_LENGTH = 3072
-    MAX_PAYLOAD_LENGTH = 256000
-    MAX_ARRAY_LENGTH = 40
+    MAX_PAYLOAD_LENGTH = 512000
+    MAX_ARRAY_LENGTH = 80
     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)
+
+      # Recursively trim code from stacktrace, oldest function first
+      threshold = get_payload_length(reduced_value) - MAX_PAYLOAD_LENGTH
+      reduced_value = trim_stacktrace_code(reduced_value, threshold)
       return reduced_value unless payload_too_long?(reduced_value)
-      remove_metadata_from_events(reduced_value)
+
+      # Recursively remove oldest functions in stacktrace
+      threshold = get_payload_length(reduced_value) - MAX_PAYLOAD_LENGTH
+      trim_stacktrace_functions(reduced_value, threshold)
     end
 
     ##
@@ -60,21 +70,74 @@ def self.deep_merge!(l_hash, r_hash)
 
     TRUNCATION_INFO = '[TRUNCATED]'
 
+    ##
+    # Trim stacktrace code out if they're too large, oldest functions first
+    def self.trim_stacktrace_code(payload, threshold)
+      extract_exception(payload) do |exception|
+        total = exception[:stacktrace].length
+        count = 0
+        saved = 0
+        while (count < total) && (saved < threshold)
+          trace = exception[:stacktrace][total - count - 1]
+          saved += get_payload_length(trace[:code])
+          trace.delete(:code)
+          count += 1
+        end
+      end
+      payload
+    end
+
+    ##
+    # Trim stacktrace entries out oldest functions first
+    def self.trim_stacktrace_functions(payload, threshold)
+      extract_exception(payload) do |exception|
+        saved = 0
+        while (exception[:stacktrace].size > 1) && (saved < threshold)
+          saved += get_payload_length(exception[:stacktrace].last)
+          exception[:stacktrace].pop
+        end
+      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)
+    def self.truncate_array(array, limit = MAX_ARRAY_LENGTH)
       return [] unless array.respond_to?(:slice)
-      array.slice(0, MAX_ARRAY_LENGTH).map do |item|
-        truncate_arrays_in_value(item)
+      array.slice(0, limit).map do |item|
+        truncate_arrays_in_value(item, limit)
       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 +151,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
 
@@ -124,12 +192,12 @@ def self.trim_strings_in_array(collection)
       collection.map {|value| trim_strings_in_value(value)}
     end
 
-    def self.truncate_arrays_in_value(value)
+    def self.truncate_arrays_in_value(value, limit = MAX_ARRAY_LENGTH)
       case value
       when Hash
-        truncate_arrays_in_hash(value)
+        truncate_arrays_in_hash(value, limit)
       when Array, Set
-        truncate_array(value)
+        truncate_array(value, limit)
       else
         value
       end
@@ -144,10 +212,10 @@ def self.remove_metadata_from_events(object)
       object
     end
 
-    def self.truncate_arrays_in_hash(hash)
+    def self.truncate_arrays_in_hash(hash, limit = MAX_ARRAY_LENGTH)
       return {} unless hash.is_a?(Hash)
       hash.each_with_object({}) do |(key, value), reduced_hash|
-        if reduced_value = truncate_arrays_in_value(value)
+        if reduced_value = truncate_arrays_in_value(value, limit)
           reduced_hash[key] = reduced_value
         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,104 +72,93 @@ 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
-      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 "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 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
+      it "trims stacktrace code, oldest first" 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_not 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 "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
-        end
+      it "trims stacktrace entries, oldest first" do
+        payload = {
+          :events => [{
+            :exceptions => [{
+              :stacktrace => [
+                {
+                  :lineNumber => 1,
+                  :file => '/trace1',
+                  :something => 150.times.map {|i| 2.times.map { |k| SecureRandom.hex(700) } }
+                },
+                {
+                  :lineNumber => 2,
+                  :file => '/trace2',
+                  :something => 200.times.map {|i| 2.times.map { |k| SecureRandom.hex(1500) } }
+                }
+              ]
+            }]
+          }]
+        }
+        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(1)
+        expect(trace[0][:lineNumber]).to eq(1)
+        expect(trace[0][:file]).to eq('/trace1')
+        expect(trace[0][:something]).to_not be_nil
       end
     end
   end

spec/integrations/rack_spec.rb

@@ -62,8 +62,8 @@
     before do
       unless defined?(::Rack)
         @mocked_rack = true
-        class ::Rack
-          class ::Request
+        class Rack
+          class Request
           end
         end
       end
@@ -94,7 +94,7 @@ class ::Request
         :referer => "referer",
         :fullpath => "/TEST_PATH"
       )
-      expect(::Rack::Request).to receive(:new).with(rack_env).and_return(rack_request)
+      expect(Rack::Request).to receive(:new).with(rack_env).and_return(rack_request)
 
       report = double("Bugsnag::Report")
       allow(report).to receive(:request_data).and_return({