From 1e9e34267ca16d2c14acfc8bcb556a0471241fef Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 19 May 2026 10:45:11 +0000 Subject: [PATCH 1/5] test(test_helper): add failing regression specs for clone_hub event leak (#2951) Captures the documented bug where setup_sentry_test is ineffective in request specs because clone_hub_to_current_thread clones @main_hub rather than the thread-local hub setup_sentry_test mutated, and DummyTransport has no #clear, so events from a prior unrelated request leak into a later test via the main hub's base-layer DummyTransport. - Reproduction spec (intentional TDD red): after a setup/teardown cycle, an intermediate clone_hub_to_current_thread + capture, then a second setup_sentry_test, sentry_events must be empty both immediately and after a further clone_hub_to_current_thread. Currently the second assertion fails because the stale event is still visible. - Guard spec (passes today, must keep passing): events captured through a hub cloned by the Rack middleware after setup_sentry_test remain observable via sentry_events, so the eventual fix cannot regress by silently dropping request-captured events. Co-Authored-By: Claude Opus 4.7 (1M context) --- sentry-ruby/spec/sentry/test_helper_spec.rb | 42 +++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/sentry-ruby/spec/sentry/test_helper_spec.rb b/sentry-ruby/spec/sentry/test_helper_spec.rb index caf4e1c3f..81332fb8c 100644 --- a/sentry-ruby/spec/sentry/test_helper_spec.rb +++ b/sentry-ruby/spec/sentry/test_helper_spec.rb @@ -92,6 +92,48 @@ end end + describe "event leakage across clone_hub_to_current_thread (regression for #2951)" do + it "keeps sentry_events empty after setup_sentry_test even when an earlier request captured events through a cloned hub" do + # Cycle 1: a normal test that uses the test helper. + setup_sentry_test + Sentry.capture_message("event from a previous test") + teardown_sentry_test + + # Simulate an unrelated request that runs *without* the test helper: + # Sentry::Rack::CaptureExceptions clones the main hub onto the request + # thread, then an event is captured through that cloned hub. + Sentry.clone_hub_to_current_thread + Sentry.capture_message("event from an unrelated request") + + # Cycle 2: the next test sets the helper up again. + setup_sentry_test + + expect(sentry_events).to be_empty + + # The Rack middleware clones the hub again for *this* test's request. + Sentry.clone_hub_to_current_thread + + expect(sentry_events).to be_empty + + teardown_sentry_test + 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::Rack::CaptureExceptions clones the main hub onto the request + # thread before the request body runs. + 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 "#teardown_sentry_test" do before do setup_sentry_test From 839e01715513454f4ba268163531ad49f00f9c59 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 19 May 2026 10:50:13 +0000 Subject: [PATCH 2/5] feat(test_helper): add DummyTransport#clear to empty captured events clear_sentry_events called #clear only if the transport responded to it; DummyTransport never did, so clearing the testing transport was a silent no-op. Add #clear that empties the captured events and envelopes in place (keeping array references valid). Refs #2951 Co-Authored-By: Claude Opus 4.7 (1M context) --- sentry-ruby/lib/sentry/transport/dummy_transport.rb | 7 +++++++ 1 file changed, 7 insertions(+) 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 From 67be7e1ec820d03c4c91e98846c5ee38f0bc35eb Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 19 May 2026 10:51:05 +0000 Subject: [PATCH 3/5] fix(test_helper): anchor setup/teardown to the main hub (#2951) setup_sentry_test mutated the thread-local hub, but Sentry.clone_hub_to_current_thread (used by Sentry::Rack::CaptureExceptions) always clones the *main* hub. After an intermediate clone the thread-local hub is a clone, so a later setup_sentry_test reconfigured the clone while the main hub kept a stale DummyTransport on its base layer. Combined with clear being a no-op for that transport, an unrelated request's event leaked into the next test's sentry_events (intermittent under RSpec random ordering). - setup_sentry_test now binds the base and test clients on the main hub and realigns the current thread's hub to it, so direct sentry_events reads and request-time clones share one DummyTransport. - teardown_sentry_test pops the testing layer off the main hub (where setup pushed it) instead of the current thread's hub. - clear_sentry_events now clears every transport reachable from the current hub and the main hub (including its base layer) via the new sentry_test_transports helper, so no stale DummyTransport survives. Co-Authored-By: Claude Opus 4.7 (1M context) --- sentry-ruby/lib/sentry/test_helper.rb | 49 ++++++++++++++++++++++----- 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/sentry-ruby/lib/sentry/test_helper.rb b/sentry-ruby/lib/sentry/test_helper.rb index 48c4ff244..3f182af40 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.instance_variable_get(:@stack).map { |layer| layer.client&.transport } + end.compact.uniq + end + # Returns the captured event objects. # @return [Array] def sentry_events From 197417242cebdacbed4ef38f42502fb3d3d763c1 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 19 May 2026 10:55:29 +0000 Subject: [PATCH 4/5] test(test_helper): add Rack two-request leak regression spec (#2951) Drives two consecutive requests through the real Sentry::Rack::CaptureExceptions middleware (which calls clone_hub_to_current_thread) wrapped in setup_sentry_test/ teardown_sentry_test, asserting the first request's event does not leak into the second and that each request's event stays observable via sentry_events. Fails against pre-fix code (second request sees 2 events); passes with the #2951 fix. Co-Authored-By: Claude Opus 4.7 (1M context) --- sentry-ruby/spec/sentry/test_helper_spec.rb | 50 +++++++++++++++++---- 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/sentry-ruby/spec/sentry/test_helper_spec.rb b/sentry-ruby/spec/sentry/test_helper_spec.rb index 81332fb8c..c5c6b7303 100644 --- a/sentry-ruby/spec/sentry/test_helper_spec.rb +++ b/sentry-ruby/spec/sentry/test_helper_spec.rb @@ -94,23 +94,17 @@ describe "event leakage across clone_hub_to_current_thread (regression for #2951)" do it "keeps sentry_events empty after setup_sentry_test even when an earlier request captured events through a cloned hub" do - # Cycle 1: a normal test that uses the test helper. setup_sentry_test Sentry.capture_message("event from a previous test") teardown_sentry_test - # Simulate an unrelated request that runs *without* the test helper: - # Sentry::Rack::CaptureExceptions clones the main hub onto the request - # thread, then an event is captured through that cloned hub. Sentry.clone_hub_to_current_thread Sentry.capture_message("event from an unrelated request") - # Cycle 2: the next test sets the helper up again. setup_sentry_test expect(sentry_events).to be_empty - # The Rack middleware clones the hub again for *this* test's request. Sentry.clone_hub_to_current_thread expect(sentry_events).to be_empty @@ -125,8 +119,6 @@ it "still exposes events captured through a hub the Rack middleware cloned after setup_sentry_test" do setup_sentry_test - # Sentry::Rack::CaptureExceptions clones the main hub onto the request - # thread before the request body runs. Sentry.clone_hub_to_current_thread Sentry.capture_message("event from the request") @@ -134,6 +126,48 @@ end end + describe "Sentry::Rack::CaptureExceptions across consecutive requests (regression for #2951)", when: :rack_available? do + # 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")) + + teardown_sentry_test + end + end + describe "#teardown_sentry_test" do before do setup_sentry_test From 12db6274293f5ca16c3eb76cd8ed399d541e08b9 Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Tue, 19 May 2026 11:13:19 +0000 Subject: [PATCH 5/5] refactor(test_helper): use public Hub#clients instead of @stack ivar (#2951) Address code review feedback on the #2951 fix: - Add a documented public Sentry::Hub#clients accessor returning all clients across the scope stack, and use it from TestHelper#sentry_test_transports instead of reaching into Hub's private @stack ivar. This keeps the test helper resilient to future Hub internals refactors. Covered by new hub_spec.rb examples. - Replace the final inline teardown_sentry_test in the #2951 regression and Rack consecutive-requests describe blocks with an unconditional 'after { teardown_sentry_test }' hook, matching the sibling describe style so cleanup runs even if an expectation fails mid-example. The intentional mid-scenario teardowns remain inline. Co-Authored-By: Claude Opus 4.7 (1M context) --- sentry-ruby/lib/sentry/hub.rb | 6 ++++++ sentry-ruby/lib/sentry/test_helper.rb | 2 +- sentry-ruby/spec/sentry/hub_spec.rb | 14 ++++++++++++++ sentry-ruby/spec/sentry/test_helper_spec.rb | 8 ++++---- 4 files changed, 25 insertions(+), 5 deletions(-) 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 3f182af40..0387ba354 100644 --- a/sentry-ruby/lib/sentry/test_helper.rb +++ b/sentry-ruby/lib/sentry/test_helper.rb @@ -112,7 +112,7 @@ def sentry_transport # @return [Array] def sentry_test_transports [Sentry.get_current_hub, Sentry.get_main_hub].compact.uniq.flat_map do |hub| - hub.instance_variable_get(:@stack).map { |layer| layer.client&.transport } + hub.clients.map(&:transport) end.compact.uniq 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 c5c6b7303..c610923bc 100644 --- a/sentry-ruby/spec/sentry/test_helper_spec.rb +++ b/sentry-ruby/spec/sentry/test_helper_spec.rb @@ -93,6 +93,8 @@ 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") @@ -108,8 +110,6 @@ Sentry.clone_hub_to_current_thread expect(sentry_events).to be_empty - - teardown_sentry_test end end @@ -127,6 +127,8 @@ 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. @@ -163,8 +165,6 @@ def 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")) - - teardown_sentry_test end end