diff --git a/lib/new_relic/agent/instrumentation/net_http/instrumentation.rb b/lib/new_relic/agent/instrumentation/net_http/instrumentation.rb index dfcf212492..7bf0494db7 100644 --- a/lib/new_relic/agent/instrumentation/net_http/instrumentation.rb +++ b/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 diff --git a/lib/new_relic/agent/transaction/abstract_segment.rb b/lib/new_relic/agent/transaction/abstract_segment.rb index 52fbee80e6..048a2c9f8d 100644 --- a/lib/new_relic/agent/transaction/abstract_segment.rb +++ b/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 diff --git a/test/multiverse/suites/net_http/net_http_core_instrumentation_test.rb b/test/multiverse/suites/net_http/net_http_core_instrumentation_test.rb new file mode 100644 index 0000000000..9ab43bc35a --- /dev/null +++ b/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 diff --git a/test/new_relic/agent/transaction/abstract_segment_test.rb b/test/new_relic/agent/transaction/abstract_segment_test.rb index 096f15f593..0580892f45 100644 --- a/test/new_relic/agent/transaction/abstract_segment_test.rb +++ b/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