feat(core): Warn on repeated Sentry.init() and unbind the client on close() - #24962
Merged
isaacs merged 7 commits intoOct 5, 2026
Conversation
Member
Author
|
bugbot run |
Contributor
size-limit report 📦
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
isaacs
marked this pull request as ready for review
October 1, 2026 22:00
isaacs
requested review from
JPeer264,
logaretm,
mydea,
nicohrubec and
s1gr1d
and removed request for
a team
October 1, 2026 22:00
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
JPeer264
approved these changes
Oct 2, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
Nice change. Got some questions throughout. The Sentry bot comment might be worth to check out
isaacs
force-pushed
the
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
branch
3 times, most recently
from
October 2, 2026 20:04
1386927 to
2f4c9d9
Compare
isaacs
force-pushed
the
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
branch
from
October 3, 2026 04:51
2df3d49 to
f20a5b1
Compare
Member
Author
|
@JPeer264 Thanks for the review! Bugbot and SentryBot caught a few subtle interesting issues, which led to a bit of refactoring. If you want to take another look, it might be worth doing. Or just rubber-stamp if it still LGTY, that's fine too :) Also: the size increase is 99% a result of just adding a new human-readable error message, so not much to be done for it. |
isaacs
force-pushed
the
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
branch
from
October 5, 2026 14:20
a9c4fae to
429dbf9
Compare
JPeer264
approved these changes
Oct 5, 2026
JPeer264
left a comment
JPeer264
left a comment
Member
There was a problem hiding this comment.
nice. Some tests fail though, but it doesn't seem related to this PR
… `close()` A repeated `Sentry.init()` call was mostly undefined behavior, and each SDK handled it in its own way. Most SDKs built a new client and replaced the old one without a warning. Nothing closed the old client, so its buffers, timers, and hooks stayed alive, and `setupOnce` kept the settings of the first call. The goal is one rule for all SDKs: the first `init()` wins, a later call returns the active client, and `close()` lets you start over. Most SDKs cannot switch to "first wins" before a major version, so this change adds the parts that are safe now: - `initAndBind`, Node's `_init`, and Vercel Edge's `init` print a warning (with or without `debug`) when a client is already bound. They still replace the client for now. Wrappers that expect a repeated call (Next.js server, Remix server, Nuxt server, Hono) keep their own guard and return early, so they do not warn. Cloudflare's `cacheClient: false` asks for a new client on each call, so it unbinds the old one first and does not warn. The Next.js client drops its own warning, which used a flag that never reset and so also fired after `close()`. Its config-file hint moves to the docs. - `Sentry.close()` unbinds the client after it closes it. Before, a guard based on `getClient()` treated the closed client as active, so `close(); init()` returned the closed client. Nuxt's server guard now also checks for a bound client, for the same reason. Cloudflare also caches its client for the isolate, and the cache handed the closed client back to every later `init()`, so the isolate sent nothing until it was recycled. Closing the cached client now clears the cache. - Next.js server and Remix server return the active client from a repeated call, not `undefined`. A caller could not tell "already initialized" from "failed". `docs/repeated-init.md` records the rule, the current behavior of each SDK, and the plan for the next major. The warning text should point apps that share a page to the isolated client helper from PR #24883 once that helper has a final name. The Next.js and Nuxt server `init()` added their event processors to the global scope. Since `close()` now unbinds the client, a later `init()` would run the full setup again, and each cycle would add another copy of each processor to the global scope. So instead, add them to the client, so that they go away with a closed client, and a later `init()` uses its own options. Client processors run before all scope processors, so these now also run before any global scope processor that user code added before `init()`. They only drop events or fix stack frames, so the earlier position does not change which events are sent. Update the write-tests skill to reset a bound client between tests that call `init()`. Fixes #24960 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`close()` unbinds the client only from the scope that is current when it runs. Other scopes can still hold the closed client: the root scope when `close()` runs inside a request or `withScope`, scopes forked before `close()` ran, and every scope when code calls `client.close()` directly. The "already initialized" guards in Next.js server, Remix server, Nuxt server, and Hono, and the repeated-init warning, used `getClient()`, so they returned the closed, disabled client or warned for no reason. Core now records closed clients, and a new internal `getActiveClient()` returns the bound client only if it is not closed. The guards and the warning use it. A client created with `enabled: false` still counts as active, because only `close()` marks a client as closed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`isClientClosed()` used a `WeakSet` local to the `client` module. When two copies of `@sentry/core` at the same version load (for example the CJS and ESM builds in one Node process), they share the carrier and so the bound client, but not that set. A client closed through one copy still looked active to the other, so its init guards returned the closed client. Store the mark on the client under a `Symbol.for` key instead, so every copy reads the same mark. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
isaacs
force-pushed
the
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
branch
from
October 5, 2026 20:36
429dbf9 to
ec00267
Compare
…tryinit-behavior-into-alignment
isaacs
enabled auto-merge (squash)
October 5, 2026 22:57
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 60932ab. Configure here.
The Next.js client `init()` added `NextRedirectErrorFilter`, and `devErrorSymbolicationEventProcessor` in development, to the isolation scope. Each repeated `init()`, including one after `close()`, added another copy that outlived the old client. Add them to the client instead, as the server `init()` already does. Client processors run before scope processors. These only drop redirect errors or fix stack frames, so the earlier position does not change which events are sent. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
isaacs
deleted the
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
branch
October 5, 2026 23:33
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.
A repeated
Sentry.init()call was mostly undefined behavior, and each SDK handled it in its own way. Most SDKs built a new client and replaced the old one without a warning. Nothing closed the old client, so its buffers, timers, and hooks stayed alive, andsetupOncekept the settings of the first call.The goal is one rule for all SDKs: the first
init()wins, a later call returns the active client, andclose()lets you start over. Most SDKs cannot switch to "first wins" before a major version, so this change adds the parts that are safe now:initAndBind, Node's_init, and Vercel Edge'sinitprint a warning (with or withoutdebug) when a client is already bound. They still replace the client for now. Wrappers that expect a repeated call (Next.js server, Remix server, Nuxt server, Hono) keep their own guard and return early, so they do not warn. Cloudflare'scacheClient: falseasks for a new client on each call, so it unbinds the old one first and does not warn. The Next.js client drops its own warning, which used a flag that never reset and so also fired afterclose(). Its config-file hint moves to the docs.Sentry.close()unbinds the client after it closes it. Before, a guard based ongetClient()treated the closed client as active, soclose(); init()returned the closed client. Nuxt's server guard now also checks for a bound client, for the same reason. Cloudflare also caches its client for the isolate, and the cache handed the closed client back to every laterinit(), so the isolate sent nothing until it was recycled. Closing the cached client now clears the cache.undefined. A caller could not tell "already initialized" from "failed".The Next.js and Nuxt server
init()added their event processors to the global scope. Sinceclose()now unbinds the client, a laterinit()would run the full setup again, and each cycle would add another copy of each processor to the global scope. So instead, add them to the client, so that they go away with a closed client, and a laterinit()uses its own options. Client processors run before all scope processors, so these now also run before any global scope processor that user code added beforeinit(). They only drop events or fix stack frames, so the earlier position does not change which events are sent.docs/repeated-init.mdrecords the rule, the current behavior of each SDK, and the plan for the next major. The warning text should point apps that share a page to the isolated client helper from #24883 once that helper has a final name.Fixes #24960