diff --git a/sentry-ruby/lib/sentry/hub.rb b/sentry-ruby/lib/sentry/hub.rb index f3ca5c3ba..535dafbe1 100644 --- a/sentry-ruby/lib/sentry/hub.rb +++ b/sentry-ruby/lib/sentry/hub.rb @@ -54,6 +54,12 @@ def current_client current_layer&.client end + # All clients bound across the hub's scope stack, base layer first. + # @return [Array] + def clients + @stack.map(&:client).compact + end + def configuration current_client.configuration end diff --git a/sentry-ruby/lib/sentry/test_helper.rb b/sentry-ruby/lib/sentry/test_helper.rb index 48c4ff244..0387ba354 100644 --- a/sentry-ruby/lib/sentry/test_helper.rb +++ b/sentry-ruby/lib/sentry/test_helper.rb @@ -37,13 +37,25 @@ def setup_sentry_test(&block) # - auto_session_tracking block&.call(dummy_config) + # Install the testing clients on the *main* hub rather than the current + # thread's hub. `Sentry.clone_hub_to_current_thread` (used by + # Sentry::Rack::CaptureExceptions) always clones the main hub, so if we + # only mutated the thread-local hub a request-time clone would observe a + # stale transport. + main_hub = Sentry.get_main_hub + # the base layer's client should already use the dummy config so nothing will be sent by accident base_client = Sentry::Client.new(dummy_config) - Sentry.get_current_hub.bind_client(base_client) + main_hub.bind_client(base_client) # create a new layer so mutations made to the testing scope or configuration could be simply popped later - Sentry.get_current_hub.push_scope + main_hub.push_scope test_client = Sentry::Client.new(dummy_config.dup) - Sentry.get_current_hub.bind_client(test_client) + main_hub.bind_client(test_client) + + # Realign the current thread's hub with the main hub so direct + # `sentry_events` reads and any hub the Rack middleware clones from the + # main hub all observe the same DummyTransport. + Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, main_hub) end # Clears all stored events and envelopes. @@ -54,11 +66,15 @@ def teardown_sentry_test clear_sentry_events - # pop testing layer created by `setup_sentry_test` - # but keep the base layer to avoid nil-pointer errors + # pop the testing layer created by `setup_sentry_test` off the *main* + # hub (that is where `setup_sentry_test` pushed it), keeping the base + # layer to avoid nil-pointer errors. Popping the current thread's hub + # would leave the test layer dangling on the main hub, which the next + # request-time clone would inherit. # TODO: find a way to notify users if they somehow popped the test layer before calling this method - if Sentry.get_current_hub.instance_variable_get(:@stack).size > 1 - Sentry.get_current_hub.pop_scope + main_hub = Sentry.get_main_hub + if main_hub.instance_variable_get(:@stack).size > 1 + main_hub.pop_scope end Sentry::Scope.global_event_processors.clear end @@ -66,7 +82,13 @@ def teardown_sentry_test def clear_sentry_events return unless Sentry.initialized? - sentry_transport.clear if sentry_transport.respond_to?(:clear) + # Clear every transport reachable from the current thread's hub and the + # main hub (including its base layer). A request-time clone shares the + # main hub's base-layer transport, so clearing only the current + # transport would let stale events survive into the next test. + sentry_test_transports.each do |transport| + transport.clear if transport.respond_to?(:clear) + end if Sentry.configuration.enable_logs && sentry_logger.respond_to?(:clear) sentry_logger.clear @@ -83,6 +105,17 @@ def sentry_transport Sentry.get_current_client.transport end + # Every transport reachable from the current thread's hub and the main + # hub, across all stack layers. Used by `clear_sentry_events` so a stale + # DummyTransport (e.g. the main hub's base layer that a request-time clone + # shares) cannot carry leftover events into the next test. + # @return [Array] + def sentry_test_transports + [Sentry.get_current_hub, Sentry.get_main_hub].compact.uniq.flat_map do |hub| + hub.clients.map(&:transport) + end.compact.uniq + end + # Returns the captured event objects. # @return [Array] def sentry_events diff --git a/sentry-ruby/lib/sentry/transport/dummy_transport.rb b/sentry-ruby/lib/sentry/transport/dummy_transport.rb index 4179ba1cc..68df2d122 100644 --- a/sentry-ruby/lib/sentry/transport/dummy_transport.rb +++ b/sentry-ruby/lib/sentry/transport/dummy_transport.rb @@ -18,5 +18,12 @@ def send_event(event) def send_envelope(envelope) @envelopes << envelope end + + # Empties the captured events and envelopes so `TestHelper.clear_sentry_events` + # also clears the dummy transport instance + def clear + @events.clear + @envelopes.clear + end end end diff --git a/sentry-ruby/spec/sentry/hub_spec.rb b/sentry-ruby/spec/sentry/hub_spec.rb index 14c874f40..ad28057b9 100644 --- a/sentry-ruby/spec/sentry/hub_spec.rb +++ b/sentry-ruby/spec/sentry/hub_spec.rb @@ -537,6 +537,20 @@ end end + describe "#clients" do + it "returns the only client for a single-layer hub" do + expect(subject.clients).to eq([client]) + end + + it "returns every client across the scope stack, base layer first" do + new_client = Sentry::Client.new(configuration) + subject.push_scope + subject.bind_client(new_client) + + expect(subject.clients).to eq([client, new_client]) + end + end + describe "#pop_scope" do it "pops the current scope" do prev_scope = subject.current_scope diff --git a/sentry-ruby/spec/sentry/test_helper_spec.rb b/sentry-ruby/spec/sentry/test_helper_spec.rb index caf4e1c3f..c610923bc 100644 --- a/sentry-ruby/spec/sentry/test_helper_spec.rb +++ b/sentry-ruby/spec/sentry/test_helper_spec.rb @@ -92,6 +92,82 @@ end end + describe "event leakage across clone_hub_to_current_thread (regression for #2951)" do + after { teardown_sentry_test } + + it "keeps sentry_events empty after setup_sentry_test even when an earlier request captured events through a cloned hub" do + setup_sentry_test + Sentry.capture_message("event from a previous test") + teardown_sentry_test + + Sentry.clone_hub_to_current_thread + Sentry.capture_message("event from an unrelated request") + + setup_sentry_test + + expect(sentry_events).to be_empty + + Sentry.clone_hub_to_current_thread + + expect(sentry_events).to be_empty + end + end + + describe "request-captured events remain observable after clone_hub_to_current_thread" do + after { teardown_sentry_test } + + it "still exposes events captured through a hub the Rack middleware cloned after setup_sentry_test" do + setup_sentry_test + + Sentry.clone_hub_to_current_thread + Sentry.capture_message("event from the request") + + expect(sentry_events.map(&:message)).to include("event from the request") + end + end + + describe "Sentry::Rack::CaptureExceptions across consecutive requests (regression for #2951)", when: :rack_available? do + after { teardown_sentry_test } + + # Drives a single request through the real Rack middleware. The middleware + # calls Sentry.clone_hub_to_current_thread before handing off to the app, + # exactly like a Rails request spec would. + def perform_request(exception_message) + exception = RuntimeError.new(exception_message) + app = lambda do |env| + env["rack.exception"] = exception + [200, {}, ["ok"]] + end + stack = Sentry::Rack::CaptureExceptions.new(app) + stack.call(Rack::MockRequest.env_for("/#{exception_message}")) + end + + def captured_exception_messages + sentry_events.map { |event| event.to_h.dig(:exception, :values, 0, :value) } + end + + it "isolates each request's events and keeps them observable via sentry_events" do + # First request, wrapped in the test helper just like a request spec. + setup_sentry_test + perform_request("first-request") + messages = captured_exception_messages + expect(messages.size).to eq(1) + expect(messages.first).to include("first-request") + teardown_sentry_test + + # Second request: a fresh setup must not see the first request's event, + # even though the Rack middleware clones the main hub on every request. + setup_sentry_test + expect(sentry_events).to be_empty + + perform_request("second-request") + messages = captured_exception_messages + expect(messages.size).to eq(1) + expect(messages.first).to include("second-request") + expect(messages).not_to include(a_string_including("first-request")) + end + end + describe "#teardown_sentry_test" do before do setup_sentry_test