Merge pull request #2218 from newrelic/callback_fallback

lib/new_relic/agent/instrumentation/net_http/instrumentation.rb

@@ -35,7 +35,7 @@ def request_with_tracing(request)
             segment.process_response_headers(wrapped_response)
             response
           ensure
-            segment.finish
+            segment&.finish
           end
         end
       end

lib/new_relic/agent/transaction/abstract_segment.rb

@@ -338,6 +338,9 @@ def invoke_callback
 
           NewRelic::Agent.logger.debug("Invoking callback for #{self.class.name}...")
           self.class.instance_variable_get(CALLBACK).call
+        rescue Exception => e
+          NewRelic::Agent.logger.error("Error encountered while invoking callback for #{self.class.name}: " +
+                                       "#{e.class} - #{e.message}")
         end
 
         # Setting and invoking a segment callback

test/multiverse/suites/net_http/net_http_core_instrumentation_test.rb

@@ -0,0 +1,38 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+require 'net_http_test_cases'
+
+class Testbed
+  include NewRelic::Agent::Instrumentation::NetHTTP
+
+  def address; 'localhost'; end
+  def use_ssl?; false; end
+  def port; 1138; end
+end
+
+class NetHttpTest < Minitest::Test
+  # This test will see that `segment` is `nil` within the `ensure` block of
+  # `request_with_tracing` to confirm that we check for `nil` prior to
+  # attempting to call `#finish` on `segment`.
+  # https://github.com/newrelic/newrelic-ruby-agent/issues/2213
+  def test_segment_might_fail_to_start
+    t = Testbed.new
+    response = 'I am a response, which an exception will prevent you from receiving unless you handle a nil segment'
+
+    segment = nil
+    def segment.add_request_headers(_request); end
+    def segment.process_response_headers(_response); end
+
+    request = Minitest::Mock.new
+    2.times { request.expect :path, '/' }
+    request.expect :method, 'GET'
+
+    NewRelic::Agent::Tracer.stub :start_external_request_segment, segment do
+      result = t.request_with_tracing(request) { response }
+
+      assert_equal response, result
+    end
+  end
+end

test/new_relic/agent/transaction/abstract_segment_test.rb

@@ -365,6 +365,15 @@ def test_callback_usage_generated_supportability_metrics
 
           engine_mock.verify
         end
+
+        # No matter what the callback does, carry on with segment creation
+        # https://github.com/newrelic/newrelic-ruby-agent/issues/2213
+        def test_callback_invocation_cannot_prevent_segment_creation
+          callback = proc { raise 'kaboom' }
+          BasicSegment.set_segment_callback(callback)
+
+          assert basic_segment # this calls BasicSegment.new
+        end
         # END callbacks
       end
     end