Skip to content

feat(core): Warn on repeated Sentry.init() and unbind the client on close() - #24962

Open
isaacs wants to merge 1 commit into
developfrom
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment
Open

isaacs wants to merge 1 commit into
developfrom
isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment

Conversation

@isaacs

@isaacs isaacs commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

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".

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.

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 #24883 once that helper has a final name.

Fixes #24960

@linear-code

linear-code Bot commented Oct 1, 2026

Copy link
Copy Markdown

JS-3867

@isaacs

isaacs commented Oct 1, 2026

Copy link
Copy Markdown
Member Author

bugbot run

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

size-limit report 📦

⚠️ Warning: Base artifact is not the latest one, because the latest workflow run is not done yet. This may lead to incorrect results. Try to re-run all tests to get up to date results.

Path Size % Change Change
@sentry/browser 29.64 kB +0.42% +123 B 🔺
@sentry/browser - with treeshaking flags 27.81 kB +0.47% +129 B 🔺
@sentry/browser - with treeshaking flags tracing without tracing 27.71 kB +0.49% +134 B 🔺
@sentry/browser (incl. Tracing) 51.58 kB +0.27% +135 B 🔺
@sentry/browser (incl. Tracing + Span Streaming) 51.59 kB +0.26% +131 B 🔺
@sentry/browser (incl. Tracing, Profiling) 54.57 kB +0.21% +114 B 🔺
@sentry/browser (incl. Tracing, Replay) 91.17 kB +0.15% +128 B 🔺
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 80.16 kB +0.16% +127 B 🔺
@sentry/browser (incl. Tracing, Replay with Canvas) 95.87 kB +0.14% +125 B 🔺
@sentry/browser (incl. Tracing, Replay, Feedback) 108.83 kB +0.12% +120 B 🔺
@sentry/browser (incl. Feedback) 47.16 kB +0.26% +121 B 🔺
@sentry/browser (incl. sendFeedback) 34.69 kB +0.35% +120 B 🔺
@sentry/browser (incl. FeedbackAsync) 39.8 kB +0.3% +119 B 🔺
@sentry/browser (incl. Metrics) 30.66 kB +0.4% +122 B 🔺
@sentry/browser (incl. Logs) 30.95 kB +0.4% +123 B 🔺
@sentry/browser (incl. Metrics & Logs) 31.6 kB +0.39% +122 B 🔺
@sentry/react 31.48 kB +0.38% +118 B 🔺
@sentry/react (incl. Tracing) 53.92 kB +0.23% +120 B 🔺
@sentry/vue 37.62 kB +0.31% +113 B 🔺
@sentry/vue (incl. Tracing) 54.45 kB +0.23% +121 B 🔺
@sentry/svelte 29.67 kB +0.41% +121 B 🔺
@sentry/remix (Remix 3 client bundle) 55.93 kB +0.27% +148 B 🔺
CDN Bundle 31.38 kB +0.5% +155 B 🔺
CDN Bundle (incl. Tracing) 52.13 kB +0.3% +152 B 🔺
CDN Bundle (incl. Logs, Metrics) 33.62 kB +0.49% +161 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) 54.07 kB +0.29% +153 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) 74.36 kB +0.21% +151 B 🔺
CDN Bundle (incl. Tracing, Replay) 89.71 kB +0.17% +152 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 91.69 kB +0.18% +156 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) 95.87 kB +0.16% +153 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 97.86 kB +0.16% +151 B 🔺
CDN Bundle - uncompressed 92.51 kB +0.4% +362 B 🔺
CDN Bundle (incl. Tracing) - uncompressed 154.86 kB +0.24% +362 B 🔺
CDN Bundle (incl. Logs, Metrics) - uncompressed 99.08 kB +0.37% +362 B 🔺
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 160.81 kB +0.23% +362 B 🔺
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 228.65 kB +0.16% +362 B 🔺
CDN Bundle (incl. Tracing, Replay) - uncompressed 274.59 kB +0.14% +362 B 🔺
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 280.52 kB +0.13% +362 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 288.29 kB +0.13% +362 B 🔺
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 294.22 kB +0.13% +362 B 🔺
@sentry/nextjs (client) 56.3 kB -0.04% -21 B 🔽
@sentry/sveltekit (client) 51.99 kB +0.23% +119 B 🔺
@sentry/core/server 40.59 kB - -
@sentry/core/browser 13.63 kB - -
@sentry/node 144.88 kB +0.09% +127 B 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection) 83.22 kB - -
@sentry/node - without tracing 93.43 kB +0.13% +115 B 🔺
@sentry/node - without channel injection 123.07 kB +0.1% +111 B 🔺
@sentry/aws-serverless 101.69 kB +0.11% +109 B 🔺
@sentry/cloudflare (withSentry) - minified 209.07 kB +0.22% +453 B 🔺
@sentry/cloudflare (withSentry) 518.2 kB +0.16% +797 B 🔺

View base workflow run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 78c4faa. Configure here.

Comment thread packages/remix/src/server/sdk.ts
@isaacs
isaacs marked this pull request as ready for review October 1, 2026 22:00
@isaacs
isaacs requested review from a team as code owners October 1, 2026 22:00
@isaacs
isaacs requested review from JPeer264, logaretm, mydea, nicohrubec and s1gr1d and removed request for a team October 1, 2026 22:00
Comment thread packages/nuxt/src/server/sdk.ts

@JPeer264 JPeer264 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice change. Got some questions throughout. The Sentry bot comment might be worth to check out


describe('BrowserProfilingIntegration', () => {
beforeEach(() => {
getCurrentScope().setClient(undefined);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

q: Just for my understanding. We have to do this now, as otherwise we would just reuse the client from before, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not quite. The beforeEach only stops the new warning, which fires because the previous test's client is still bound. So the test runs the same either way, and the reset just keeps the output clean.

The reset will matter more at the next major, though. Once "first wins" lands, a test that skips the reset will get the previous test's client back, with the previous test's options. So this prepares these tests for that change.

// A closed client drops all data, so the next `init()` must not reuse it.
client.on('close', () => {
if (getCachedClient() === client) {
_clearGlobalClientCache();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

q: When would a Sentry.close() be called? At least in Cloudflare that would be only when a user calls it manually, right? Maybe I'm missing something

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right, the SDK never closes a cached client itself (flushAndDispose only disposes per-invocation clients), so this only fires on a manual Sentry.close() / client.close().

The case I had in mind is someone calling await Sentry.close() at the end of a handler, thinking it means "flush". Without this, the first request in the isolate reports, and every later request gets the closed, disabled client back from the cache and drops everything silently until the isolate is recycled. With it, close() works as the new rule says: the next init() sets up a fresh client.

Comment thread AGENTS.md
Runtime packages (`node`, `cloudflare`, ...) re-export from
`@sentry/server-utils` rather than defining their own.

- `Sentry.init()` follows one rule for repeated calls: the first call wins. See

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

q: You think it is worth it to also add a note in write-tests/SKILLS.md to remove the client in a beforeEach or afterEach when writing unit tests?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, yes, good idea!

Looking at it, the "Test isolation" example there is already stale (Scope#clear() and clearGlobalScope() don't exist anymore), so I'll replace it with the reset patterns we actually use (ie, getCurrentScope().setClient(undefined) before/after init()), plus a note on asserting the warning via originalConsoleMethods.warn and a link to docs/repeated-init.md.

const client = initAndBind(CloudflareClient, clientOptions) as CloudflareClient;

if (cacheEnabled && client) {
cacheClient(client);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just thinking loud, no action item now. I wonder if the cacheClient strategy within Cloudflare could be simplified, given that we reuse clients now per definition. We might run into the warnings though 🤔

… `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.

Fixes #24960

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@isaacs
isaacs force-pushed the isaacschlueter/js-3867-bring-repeated-sentryinit-behavior-into-alignment branch from 9208e17 to eb60271 Compare October 2, 2026 17:40

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bring repeated Sentry.init() behavior into alignment

2 participants