Skip to content

test: pin the multiple-container behaviour behind Func<IHub> - #5646

Merged
jamescrosswell merged 1 commit into
mainfrom
test/hub-func-regression
Oct 1, 2026
Merged

jamescrosswell merged 1 commit into
mainfrom
test/hub-func-regression

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

Description

The hub is registered in DI as Func<IHub> returning () => HubAdapter.Instance, so nothing ever holds a concrete hub. That shape fixed #103 in #157, but nothing in the suite pinned it. This adds regression tests that fail if the registration is "simplified" to a singleton IHub, plus a one-line pointer to #103 at the registration.

  • Throwaway container (Sentry.AspNetCore.Tests): with the test server running, a second provider is built from the same services, a hub is resolved from it, and the provider is disposed. An unhandled exception on /throw must still reach the background worker through SentryMiddleware.
  • No captured instance (Sentry.Extensions.Logging.Tests): IHub and ISentryClient are resolved, then the SDK's hub is swapped with SentrySdk.UseHub. Captures through both references must reach the new hub.
  • Lifetime (Sentry.Extensions.Logging.Tests): after the provider is disposed, SentrySdk still captures. The test also resolves ILoggerFactory, because SentryLoggerProvider.Dispose disposes the hub when it is IDisposable.

Notes for review

  • Container ordering differs from the issue's sketch. The issue suggests building the throwaway container before the real one. I checked that order against a simulated regression (singleton IHub holding the initialized hub, Func<IHub> closing over it) and it passes, because the real container's hub simply replaces the throwaway's. The failure only shows when the second container is built after the real one: SentrySdk.UseHub disposes the hub the real container is still holding. The test uses that order.
  • All three tests fail against that simulated regression and pass on the current code.
  • Lifetime is asserted by behaviour rather than by ServiceDescriptor.Lifetime, since a singleton HubAdapter would be harmless and a descriptor check would reject it.
  • The new Extensions.Logging tests swap the global hub, and LoggingTests in the same assembly initializes the SDK concurrently, so they run in a new SentrySdkCollection with DisableParallelization = true (same approach as ProfilingTestCollection).

#skip-changelog

Issues

Closes #5645

🤖 Generated with Claude Code

The hub is registered as Func<IHub> returning HubAdapter.Instance so that
nothing captures a hub instance. That shape fixed #103 (ObjectDisposedException
when a second container was built and disposed) but shipped without a
regression test.

Adds tests that fail if the registration is simplified to a singleton IHub:
- a second service provider built and disposed after the app is running must
  not stop the middleware capturing unhandled exceptions
- IHub/ISentryClient resolved from DI follow SentrySdk's current hub
- disposing the container does not dispose the SDK's hub

Closes #5645

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jamescrosswell jamescrosswell added the skip-changelog Suppress automatic changelog generation via Craft label Sep 30, 2026
@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.91%. Comparing base (4af9645) to head (1b89c97).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5646      +/-   ##
==========================================
+ Coverage   74.37%   74.91%   +0.54%     
==========================================
  Files         501      515      +14     
  Lines       18566    18975     +409     
  Branches     3603     3695      +92     
==========================================
+ Hits        13808    14216     +408     
+ Misses       3883     3878       -5     
- Partials      875      881       +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jamescrosswell
jamescrosswell marked this pull request as ready for review September 30, 2026 06:25
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Sep 30, 2026
@jamescrosswell
jamescrosswell merged commit 38a3d14 into main Oct 1, 2026
53 checks passed
@jamescrosswell
jamescrosswell deleted the test/hub-func-regression branch October 1, 2026 21:06
ric-oliv added a commit that referenced this pull request Oct 5, 2026
Brings 6.12.0 and main's changes since 4af9645 into version7, including
#5640, #5643, #5646, #5656, #5659, #5661, #5662 and #5667.

Conflict resolutions:
- Directory.Build.props: keep version7's 7.0.0 / prerelease.
- LoggingBuilderExtensions.cs: keep version7's side. #5595 removed the
  generic AddSentry<TOptions> that #5640 changed; version7 fixes the same
  Blazor bug in AddSentryBlazor.
- LoggingBuilderExtensionsTests.cs: drop #5640's two
  AddSentry_DerivedOptions_* tests (merged cleanly, don't compile on
  version7).
- ServiceCollectionExtensions.cs: keep version7's side and move #5646's
  issue 103 comment to the registrations in AddSentryHub.
- ServiceCollectionExtensionsTests.cs: keep version7's tests and port
  #5646's two tests to run on both version7 init paths: the host
  (AddSentry<TestHostOptions>(initializeSdk: true)) and SentrySdk.Init
  plus Logging.AddSentry().
- .github/workflows/build.yml: keep version7's .NET 11 integration-test
  steps with #5662's integration-test 3.4.1 pin and #5667's
  ubuntu-24.04 runners.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium skip-changelog Suppress automatic changelog generation via Craft

Projects

None yet

2 participants