Skip to content

fix(logs): remove Dispose from the public API surface area #4424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 14, 2025

Conversation

Flash0ver
Copy link
Member

@Flash0ver Flash0ver commented Aug 10, 2025

Summary

Remove Dispose() from SentryStructuredLogger public API surface area.

Remarks

In #4310, we added SentryStructuredLogger.Dispose(),
where only one out of three derived types actually has IDisposable fields.
This was necessary to enable cascading disposal (when Hub.IsEnabled and SentryOptions.(Experimental).EnableLogs) out of Hub.Dispose(), whose compile-time type is only the abstract SentryStructuredLogger, but not the concrete DefaultSentryStructuredLogger (because through the DisabledHub, or when EnableLogs is not true, a DisabledSentryStructuredLogger is created).

However, this allows user code to call Dispose() on the abstract SentryStructuredLogger, and therefore Dispose the concrete DefaultSentryStructuredLogger (when enabled) and its IDisposable members, without disposing any of the other Sentry instances, leaving the SDK in an inconsistent half-disposed and no longer fully usable state.

Use code can invoke:

SentrySdk.Experimental.Logger.Dispose();

The only path, where the concrete DefaultSentryStructuredLogger should be disposed, is when the CurrentHub, which creates and owns the instance of SentryStructuredLogger, is disposed.

Therefore, I'd like to remove Dispose() from the public API surface area of SentryStructuredLogger.
In order to still Dispose the DefaultSentryStructuredLogger (when enabled) when the owning Hub instance is disposed, the concrete DefaultSentryStructuredLogger still implements IDisposable and the owning Hub does a type-check on the Logger instance.

No (breaking) major release required, because SentryStructuredLogger is still [Experimental].

Changes

  • Remove IDisposable from abstract SentryStructuredLogger
  • Add IDisposable explicitly to DefaultSentryStructuredLogger, which owns disposable members
  • Hub performs a type-check before calling Dispose() on the Logger instance that it owns

@Flash0ver Flash0ver marked this pull request as ready for review August 11, 2025 14:55
@Flash0ver
Copy link
Member Author

@sentry review

Copy link
Collaborator

@jamescrosswell jamescrosswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Flash0ver Flash0ver merged commit cfbc2fd into main Aug 14, 2025
34 of 36 checks passed
@Flash0ver Flash0ver deleted the fix/logs-public-dispose branch August 14, 2025 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants