Skip to content

Commit 78c4faa

Browse files
isaacsclaude
andcommitted
feat(core): Warn on repeated Sentry.init() and unbind the client on 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 #24883 once that helper has a final name. Fixes #24960 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent c1e182c commit 78c4faa

51 files changed

Lines changed: 459 additions & 54 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎AGENTS.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ Uses **Git Flow** (see `docs/gitflow.md`).
7676
Runtime packages (`node`, `cloudflare`, ...) re-export from
7777
`@sentry/server-utils` rather than defining their own.
7878

79+
- `Sentry.init()` follows one rule for repeated calls: the first call wins. See
80+
`docs/repeated-init.md` before you add or change an `init()`.
81+
7982
## Linting & Formatting
8083

8184
- This project uses **Oxlint** and **Oxfmt** — NOT ESLint or Prettier

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
- **fix(cloudflare)**: Durable Object constructors now run in their own isolation scope. When work that the constructor starts, for example a `blockConcurrencyWhile` callback, calls a method of the instance, that call is no longer treated as an incoming RPC call, so an error the instance catches itself is no longer reported as unhandled. As a result, `Sentry.setTag()`, `setUser()` and `setContext()` in a constructor now apply only to that constructor work. They no longer reach later `fetch`, RPC or `alarm` invocations, because the scope they used to write to is shared by every Durable Object and handler in the isolate. Use `initialScope` for static data, and set per-instance data in each handler.
88

9+
- **feat(core)**: `Sentry.init()` now warns when it runs while a client is still active. For now, the new client still replaces the active client, but the active client is not closed, so state from both can mix. Call `Sentry.init()` once, or call `await Sentry.close()` before you call it again. `Sentry.close()` now unbinds the client it closes, so after `close()`, `getClient()` returns `undefined`, `isInitialized()` returns `false`, and a later `init()` sets up a new client. In `@sentry/cloudflare`, closing the client also clears the isolate's client cache, so later requests no longer reuse the closed client. On the server, `@sentry/nextjs` and `@sentry/remix` now return the active client from a repeated `init()` call, not `undefined`.
10+
911
## 11.2.0
1012

1113
### Important Changes

‎docs/repeated-init.md‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
# Repeated `Sentry.init()` Calls
2+
3+
This document records how `Sentry.init()` should act when an app calls it
4+
more than once. Use it when you add or change an `init()` in any SDK.
5+
6+
## The rule
7+
8+
> One active client per `init()` target. The first `init()` wins. A later
9+
> `init()` changes nothing, returns the active client, and warns. To
10+
> reconfigure, call `close()` first.
11+
12+
"Active" means a client is bound to the current scope. `Sentry.close()`
13+
closes the client and unbinds it, so a later `init()` sets up a new client.
14+
15+
A repeated `init()` is not supported. Until the next major version, most
16+
SDKs still replace the client (see below). Do not depend on that.
17+
18+
## Why
19+
20+
When `init()` replaces a client, nothing closes the old one:
21+
22+
- Buffered logs, metrics, spans, client reports, and flush timers stay on
23+
the old client.
24+
- `setupOnce` runs only for the first client, because the list of
25+
installed integrations is global. The result mixes settings from both
26+
calls.
27+
- `initialScope` merges into the current scope on each call.
28+
29+
"First wins" never gives a user half of one config and half of another.
30+
31+
## Current behavior
32+
33+
| Entry point | Repeated call |
34+
| ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
35+
| `initAndBind` (browser and its wrappers, Deno), Node `_init`, Vercel Edge | Warns, then replaces the client. Returns the new client. |
36+
| Next.js server, Remix server, Hono Node | Keeps the first client and returns it. Logs in debug mode only. |
37+
| Hono Bun and Deno | Keeps the first client and returns it. Warns with its own text. |
38+
| Nuxt server | Keeps the first client and returns it. Logs that a `--import` preload is no longer needed. |
39+
| Cloudflare (default) | Keeps the first client of the isolate and returns it. Closing that client clears the cache. `cacheClient: false` makes a new client on each call, with no warning. |
40+
| Next.js edge | Warns, then replaces the client. Returns `void`. |
41+
42+
The shared warning lives in `warnIfClientIsActive()` in
43+
`packages/core/src/sdk.ts`. Core exports it as
44+
`_INTERNAL_warnIfClientIsActive` for SDKs that build their client without
45+
`initAndBind`.
46+
47+
A wrapper that expects a repeated call, such as a server bundle and a
48+
`--import` preload that both run the config, keeps its own guard and
49+
returns early. The shared warning then does not show.
50+
51+
## TODO(v12): Plan for the next major version
52+
53+
1. Move the "first wins" guard into `initAndBind`, Node's `_init`, and
54+
Vercel Edge's `init`. Remove the guards in each wrapper.
55+
2. Switch browser to "first wins". For two apps on one page, point users
56+
to separate clients that are not bound with `init()` (see #24883).
57+
3. Make Next.js edge return the client.
58+
59+
## Tests
60+
61+
A test that calls `init()` again without a reset now prints the warning.
62+
Reset between tests with one of these:
63+
64+
- `getCurrentScope().setClient(undefined)`, or a helper that clears the
65+
carrier (`getMainCarrier().__SENTRY__ = undefined`).
66+
- `await Sentry.close()`. This also flushes, so it is slower.
67+
68+
The warning goes through `consoleSandbox`, which calls the method stored
69+
in `originalConsoleMethods`. To assert on it, replace
70+
`originalConsoleMethods.warn` with a mock. A spy on `console.warn` misses
71+
it once the console integration is set up.

‎packages/angular/test/sdk.test.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import * as SentryBrowser from '@sentry/browser';
2-
import { vi } from 'vitest';
2+
import { afterEach, vi } from 'vitest';
33
import { getDefaultIntegrations, init } from '../src/sdk';
44

55
describe('init', () => {
6+
afterEach(() => {
7+
SentryBrowser.getCurrentScope().setClient(undefined);
8+
});
9+
610
it('sets the Angular version (if available) in the global scope', () => {
711
const setContextSpy = vi.spyOn(SentryBrowser, 'setContext');
812

‎packages/angular/test/tracing.test.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import { ElementRef } from '@angular/core';
22
import type { ActivatedRouteSnapshot } from '@angular/router';
3-
import { getMainCarrier, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SentrySpan, spanToJSON, startSpan } from '@sentry/core';
4-
import { describe, it } from 'vitest';
3+
import {
4+
getCurrentScope,
5+
getMainCarrier,
6+
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
7+
SentrySpan,
8+
spanToJSON,
9+
startSpan,
10+
} from '@sentry/core';
11+
import { beforeEach, describe, it } from 'vitest';
512
import { browserTracingIntegration, init, TraceClass, TraceDirective } from '../src/index';
613
import { _updateSpanAttributesForParametrizedUrl, getParameterizedRouteFromSnapshot } from '../src/tracing';
714
import { SENTRY_SEGMENT_NAME_SOURCE, URL_FULL, URL_PATH, URL_TEMPLATE } from '@sentry/conventions/attributes';
@@ -68,6 +75,10 @@ describe('Angular Tracing', () => {
6875
});
6976

7077
describe('TraceService', () => {
78+
beforeEach(() => {
79+
getCurrentScope().setClient(undefined);
80+
});
81+
7182
it('change the span name to route name if the the source is `url`', async () => {
7283
init({ integrations: [browserTracingIntegration()] });
7384

‎packages/astro/test/client/sdk.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ describe('Sentry client SDK', () => {
9898
expect.objectContaining({ routeProvider: expect.objectContaining({ resolveRoute: expect.any(Function) }) }),
9999
);
100100

101+
getMainCarrier().__SENTRY__ = undefined;
101102
const routeProvider = { resolveRoute: () => '/custom', resolveCurrentRoute: () => '/custom' };
102103
init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', routeProvider });
103104
expect(browserInit).toHaveBeenLastCalledWith(expect.objectContaining({ routeProvider }));

‎packages/browser/test/index.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,10 @@ describe('SentryBrowser', () => {
391391
});
392392

393393
describe('SentryBrowser initialization', () => {
394+
beforeEach(() => {
395+
getCurrentScope().setClient(undefined);
396+
});
397+
394398
it('should use window.SENTRY_RELEASE to set release on initialization if available', () => {
395399
global.SENTRY_RELEASE = { id: 'foobar' };
396400
init({ dsn });

‎packages/browser/test/profiling/UIProfiler.test.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,7 @@ function getBaseOptionsForTraceLifecycle(sendMock: Mock<any>, enableTracing = tr
2121

2222
describe('Browser Profiling v2 trace lifecycle', () => {
2323
afterEach(async () => {
24-
const client = Sentry.getClient();
25-
await client?.close();
24+
await Sentry.close();
2625
// reset profiler constructor
2726
(window as any).Profiler = undefined;
2827
vi.restoreAllMocks();
@@ -568,7 +567,7 @@ describe('Browser Profiling v2 trace lifecycle', () => {
568567
}
569568

570569
// End Session 1
571-
await client?.close();
570+
await Sentry.close();
572571

573572
// Session 2 (new init simulates new user session)
574573
const send2 = vi.fn().mockResolvedValue(undefined);
@@ -737,8 +736,7 @@ function getBaseOptionsForManualLifecycle(sendMock: Mock<any>, enableTracing = t
737736

738737
describe('Browser Profiling v2 manual lifecycle', () => {
739738
afterEach(async () => {
740-
const client = Sentry.getClient();
741-
await client?.close();
739+
await Sentry.close();
742740
// reset profiler constructor
743741
(window as any).Profiler = undefined;
744742
vi.restoreAllMocks();

‎packages/browser/test/profiling/integration.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,19 @@ import {
77
getClient,
88
spanToStaticSpanJSON,
99
getActiveSpan,
10+
getCurrentScope,
1011
browserTracingIntegration,
1112
browserProfilingIntegration,
1213
} from '../../src/index';
1314
import { debug } from '@sentry/core';
14-
import { describe, expect, it, vi } from 'vitest';
15+
import { beforeEach, describe, expect, it, vi } from 'vitest';
1516
import type { BrowserClient } from '../../src/index';
1617

1718
describe('BrowserProfilingIntegration', () => {
19+
beforeEach(() => {
20+
getCurrentScope().setClient(undefined);
21+
});
22+
1823
it('profiles an already active pageload span in trace lifecycle mode', async () => {
1924
const stopProfile = vi.fn().mockResolvedValue({
2025
frames: [{ name: 'pageload_fn', line: 1, column: 1 }],

‎packages/browser/test/sdk.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ export class MockIntegration implements Integration {
3434

3535
describe('init', () => {
3636
afterEach(() => {
37+
SentryCore.getCurrentScope().setClient(undefined);
3738
vi.restoreAllMocks();
3839
});
3940

0 commit comments

Comments
 (0)