fix(browser): Clean up pageload readystatechange listener#21632
Draft
andreiborza wants to merge 1 commit into
Draft
fix(browser): Clean up pageload readystatechange listener#21632andreiborza wants to merge 1 commit into
andreiborza wants to merge 1 commit into
Conversation
The auto-finish `readystatechange` listener registered for each pageload
span was never removed. Because it closes over the `idleSpan` (and the
rest of the route-span closure), every pageload leaked a listener and the
span it retained. The previous anonymous handler could not be removed, and
the suggested `{ once: true }` would not help in the common case where the
document is already loaded when the span starts, since the listener never
fires.
Pass `emitFinish` directly as the handler and remove it as soon as the
auto-finish signal is emitted, covering both the already-loaded and
load-later cases.
Fixes #21630
Co-Authored-By: Opus 4.8 <noreply@anthropic.com>
Contributor
size-limit report 📦
|
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
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.
The auto-finish
readystatechangelistener registered for each pageload span inbrowserTracingIntegrationwas never removed. Since the handler closes over theidleSpan(and the rest of the route-span closure), every pageload leaked a listener and kept the span it retained alive — the growing set of Sentry functions reported in #21630.The previous handler was an anonymous arrow function, so it could not be removed via
removeEventListener. The{ once: true }option suggested in the issue does not fully address the leak either: in the common case where the document is already loaded when the span starts, the listener never fires and therefore is never auto-removed.This change passes
emitFinishdirectly as the handler and removes it as soon as the auto-finish signal is emitted. This covers both cases:readystatechange, emits, and removes itself.Since the framework integrations (Vue, Astro, Next.js, Remix, SvelteKit, Ember) all wrap the browser package's
browserTracingIntegration, this single fix covers them too.Root cause
optionalWindowDocument.addEventListener('readystatechange', () => { emitFinish(); })added a fresh, unremovable listener for every pageload span without ever detaching it.Fixes #21630