Conversation
…eak (#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) <noreply@anthropic.com>
dingsdax
approved these changes
May 19, 2026
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
…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) <noreply@anthropic.com>
solnic
force-pushed
the
2951-testhelper-setup_sentry_test-ineffective-in-request-specs-due-to-clone_hub_to_current_thread-overwriting-thread-local-hub
branch
from
041229b to
12db627
Compare
solnic
deleted the
2951-testhelper-setup_sentry_test-ineffective-in-request-specs-due-to-clone_hub_to_current_thread-overwriting-thread-local-hub
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #295