Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions .agents/skills/write-tests/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,16 +307,31 @@ it.each([
### Test isolation

Tests must never depend on execution order or share mutable state. For this codebase, many tests
need to reset global Sentry state:
need to reset global Sentry state. Use whatever reset the existing tests in your package use (many
packages have a `resetSdk()` or `cleanupOtel()` helper). Otherwise, pick one:

```typescript
beforeEach(() => {
clearGlobalScope();
getCurrentScope().clear();
getIsolationScope().clear();
});
// Unbind the client. Cheap, and enough for most tests that call `init()`.
getCurrentScope().setClient(undefined);

// Reset the global, isolation, and current scopes, and the client bound to them.
// `setupOnce` still does not run again, because installed integrations are tracked
// outside the carrier.
getMainCarrier().__SENTRY__ = undefined;

// Close and unbind the client. Also flushes, so it is slower.
await Sentry.close();
```

**Reset the client in every test that calls `init()`.** A second `init()` while a client is bound
prints a "`Sentry.init()` was called more than once" warning. After the next major version, it will
return the bound client instead of a new one, so a test without a reset gets the previous test's
client and options. See `docs/repeated-init.md`.

To assert on that warning (or any `consoleSandbox` output), stub `originalConsoleMethods.warn` from
`@sentry/core`. `consoleSandbox` calls the stored original method, so a spy on `console.warn` misses
it once the console integration is set up.

### Grouping

1-2 levels of `describe` is usually enough. Deeper nesting makes tests harder to find and read.
Expand Down Expand Up @@ -526,6 +541,7 @@ Before you're done, verify each test against these criteria:
- [ ] Description reads as a behavior specification (no "should", no "works correctly")
- [ ] No dependency on other tests' execution or state
- [ ] Mocks and spies are restored (via `beforeEach`)
- [ ] Tests that call `init()` reset the bound client between tests
- [ ] Edge cases covered: empty inputs, boundaries, error paths, null/undefined
- [ ] Realistic test data (not `"foo"`, `"test"`, `123`)
- [ ] No try/catch for error testing — `toThrow` / `rejects.toThrow` only
Expand Down
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ Uses **Git Flow** (see `docs/gitflow.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
Comment thread
isaacs marked this conversation as resolved.
`docs/repeated-init.md` before you add or change an `init()`.

## Linting & Formatting

- This project uses **Oxlint** and **Oxfmt** — NOT ESLint or Prettier
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@

Work in this release was contributed by @Shubham-Padkonde and @tobias-schnabel. Thank you for your contributions!

- **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`, later requests no longer reuse the isolate's cached client once it is closed or closing. On the server, `@sentry/nextjs` and `@sentry/remix` now return the active client from a repeated `init()` call, not `undefined`.

## 11.2.0

### Important Changes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

async function run(): Promise<void> {
Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
transport: loggingTransport,
});

// The root scope still holds the closed client after this.
await Sentry.withIsolationScope(() => Sentry.close());

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '2.0',
transport: loggingTransport,
});

Sentry.captureMessage('after close in a child scope and init');
}

run();
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

async function run(): Promise<void> {
Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
transport: loggingTransport,
});

await Sentry.close();

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '2.0',
transport: loggingTransport,
});

Sentry.captureMessage('after close and init');
}

run();
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
transport: loggingTransport,
});

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '2.0',
transport: loggingTransport,
});

Sentry.captureMessage('after repeated init');
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { afterAll, expect, test } from 'vitest';
import { cleanupChildProcesses, createRunner } from '../../../utils/runner';

afterAll(() => {
cleanupChildProcesses();
});

const REPEATED_INIT_WARNING = '`Sentry.init()` was called more than once';

test('warns on a repeated init and sends events with the new client', async () => {
const runner = createRunner(__dirname, 'scenario-repeated.ts')
.expect({ event: { message: 'after repeated init', release: '2.0' } })
.start();

await runner.completed();

expect(runner.getLogs().join('\n')).toContain(REPEATED_INIT_WARNING);
});

test('does not warn on init after close and sends events with the new client', async () => {
const runner = createRunner(__dirname, 'scenario-close.ts')
.expect({ event: { message: 'after close and init', release: '2.0' } })
.start();

await runner.completed();

expect(runner.getLogs().join('\n')).not.toContain(REPEATED_INIT_WARNING);
});

test('does not warn on init after close in a child scope', async () => {
const runner = createRunner(__dirname, 'scenario-close-in-scope.ts')
.expect({ event: { message: 'after close in a child scope and init', release: '2.0' } })
.start();

await runner.completed();

expect(runner.getLogs().join('\n')).not.toContain(REPEATED_INIT_WARNING);
});
77 changes: 77 additions & 0 deletions docs/repeated-init.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Repeated `Sentry.init()` Calls

This document records how `Sentry.init()` should act when an app calls it
more than once. Use it when you add or change an `init()` in any SDK.

## The rule

> One active client per `init()` target. The first `init()` wins. A later
> `init()` changes nothing, returns the active client, and warns. To
> reconfigure, call `close()` first.

"Active" means a client is bound to the current scope and is not closed.
`Sentry.close()` closes the client and unbinds it, so a later `init()` sets
up a new client. Other scopes can still hold the closed client (for example,
when `close()` runs inside a request, or when code calls `client.close()`
directly), so a check for "already initialized" must not use `getClient()`
alone.

A repeated `init()` is not supported. Until the next major version, most
SDKs still replace the client (see below). Do not depend on that.

## Why

When `init()` replaces a client, nothing closes the old one:

- Buffered logs, metrics, spans, client reports, and flush timers stay on
the old client.
- `setupOnce` runs only for the first client, because the list of
installed integrations is global. The result mixes settings from both
calls.
- `initialScope` merges into the current scope on each call.

"First wins" never gives a user half of one config and half of another.

## Current behavior

| Entry point | Repeated call |
| ------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `initAndBind` (browser and its wrappers, Deno), Node `_init`, Vercel Edge | Warns, then replaces the client. Returns the new client. |
| Next.js server, Remix server, Hono Node | Keeps the first client and returns it. Logs in debug mode only. |
| Hono Bun and Deno | Keeps the first client and returns it. Warns with its own text. |
| Nuxt server | Keeps the first client and returns it. Logs that a `--import` preload is no longer needed. |
| Cloudflare (default) | Keeps the first client of the isolate and returns it, unless that client is closed or closing. `cacheClient: false` makes a new client on each call, with no warning. |
| Next.js edge | Warns, then replaces the client. Returns `void`. |

The shared warning lives in `warnIfClientIsActive()` in
`packages/core/src/sdk.ts`. Core exports it as
`_INTERNAL_warnIfClientIsActive` for SDKs that build their client without
`initAndBind`.

A wrapper that expects a repeated call, such as a server bundle and a
`--import` preload that both run the config, keeps its own guard and
returns early. The shared warning then does not show. Guards use
`getActiveClient()` (exported as `_INTERNAL_getActiveClient`), which
returns the bound client only if it is not closed.

## TODO(v12): Plan for the next major version

1. Move the "first wins" guard into `initAndBind`, Node's `_init`, and
Vercel Edge's `init`. Remove the guards in each wrapper.
2. Switch browser to "first wins". For two apps on one page, point users
to separate clients that are not bound with `init()` (see #24883).
3. Make Next.js edge return the client.

## Tests

A test that calls `init()` again without a reset now prints the warning.
Reset between tests with one of these:

- `getCurrentScope().setClient(undefined)`, or a helper that clears the
carrier (`getMainCarrier().__SENTRY__ = undefined`).
- `await Sentry.close()`. This also flushes, so it is slower.

The warning goes through `consoleSandbox`, which calls the method stored
in `originalConsoleMethods`. To assert on it, replace
`originalConsoleMethods.warn` with a mock. A spy on `console.warn` misses
it once the console integration is set up.
6 changes: 5 additions & 1 deletion packages/angular/test/sdk.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
import * as SentryBrowser from '@sentry/browser';
import { vi } from 'vitest';
import { afterEach, vi } from 'vitest';
import { getDefaultIntegrations, init } from '../src/sdk';

describe('init', () => {
afterEach(() => {
SentryBrowser.getCurrentScope().setClient(undefined);
});

it('sets the Angular version (if available) in the global scope', () => {
const setContextSpy = vi.spyOn(SentryBrowser, 'setContext');

Expand Down
8 changes: 6 additions & 2 deletions packages/angular/test/tracing.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ElementRef } from '@angular/core';
import type { ActivatedRouteSnapshot } from '@angular/router';
import { getMainCarrier, SentrySpan, spanToJSON, startSpan } from '@sentry/core';
import { describe, it } from 'vitest';
import { getCurrentScope, getMainCarrier, SentrySpan, spanToJSON, startSpan } from '@sentry/core';
import { beforeEach, describe, it } from 'vitest';
import { browserTracingIntegration, init, TraceClass, TraceDirective } from '../src/index';
import { _updateSpanAttributesForParametrizedUrl, getParameterizedRouteFromSnapshot } from '../src/tracing';
import {
Expand Down Expand Up @@ -74,6 +74,10 @@ describe('Angular Tracing', () => {
});

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

it('change the span name to route name if the the source is `url`', async () => {
init({ integrations: [browserTracingIntegration()] });

Expand Down
1 change: 1 addition & 0 deletions packages/astro/test/client/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe('Sentry client SDK', () => {
expect.objectContaining({ routeProvider: expect.objectContaining({ resolveRoute: expect.any(Function) }) }),
);

getMainCarrier().__SENTRY__ = undefined;
const routeProvider = { resolveRoute: () => '/custom', resolveCurrentRoute: () => '/custom' };
init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', routeProvider });
expect(browserInit).toHaveBeenLastCalledWith(expect.objectContaining({ routeProvider }));
Expand Down
4 changes: 4 additions & 0 deletions packages/browser/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,10 @@ describe('SentryBrowser', () => {
});

describe('SentryBrowser initialization', () => {
beforeEach(() => {
getCurrentScope().setClient(undefined);
});

it('should use window.SENTRY_RELEASE to set release on initialization if available', () => {
global.SENTRY_RELEASE = { id: 'foobar' };
init({ dsn });
Expand Down
8 changes: 3 additions & 5 deletions packages/browser/test/profiling/UIProfiler.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ function getBaseOptionsForTraceLifecycle(sendMock: Mock<any>, enableTracing = tr

describe('Browser Profiling v2 trace lifecycle', () => {
afterEach(async () => {
const client = Sentry.getClient();
await client?.close();
await Sentry.close();
// reset profiler constructor
(window as any).Profiler = undefined;
vi.restoreAllMocks();
Expand Down Expand Up @@ -568,7 +567,7 @@ describe('Browser Profiling v2 trace lifecycle', () => {
}

// End Session 1
await client?.close();
await Sentry.close();

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

describe('Browser Profiling v2 manual lifecycle', () => {
afterEach(async () => {
const client = Sentry.getClient();
await client?.close();
await Sentry.close();
// reset profiler constructor
(window as any).Profiler = undefined;
vi.restoreAllMocks();
Expand Down
7 changes: 6 additions & 1 deletion packages/browser/test/profiling/integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,19 @@ import {
getClient,
spanToStaticSpanJSON,
getActiveSpan,
getCurrentScope,
browserTracingIntegration,
browserProfilingIntegration,
} from '../../src/index';
import { debug } from '@sentry/core';
import { describe, expect, it, vi } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { BrowserClient } from '../../src/index';

describe('BrowserProfilingIntegration', () => {
beforeEach(() => {
getCurrentScope().setClient(undefined);
Comment thread
isaacs marked this conversation as resolved.
});

it('profiles an already active pageload span in trace lifecycle mode', async () => {
const stopProfile = vi.fn().mockResolvedValue({
frames: [{ name: 'pageload_fn', line: 1, column: 1 }],
Expand Down
1 change: 1 addition & 0 deletions packages/browser/test/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export class MockIntegration implements Integration {

describe('init', () => {
afterEach(() => {
SentryCore.getCurrentScope().setClient(undefined);
vi.restoreAllMocks();
});

Expand Down
3 changes: 2 additions & 1 deletion packages/bun/test/init.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { type Integration } from '@sentry/core';
import { getCurrentScope, type Integration } from '@sentry/core';
import * as sentryServerUtils from '@sentry/server-utils';
import type { Mock } from 'bun:test';
import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from 'bun:test';
Expand All @@ -25,6 +25,7 @@ describe('init()', () => {
let mockGetTracingIntegrations: Mock<() => Integration[]>;

beforeEach(() => {
getCurrentScope().setClient(undefined);
mockGetTracingIntegrations = spyOn(sentryServerUtils, 'getTracingIntegrations');
});

Expand Down
1 change: 1 addition & 0 deletions packages/bun/test/integrations/bunHttpServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ function currentTraceId(): string | undefined {

describe('Bun HTTP Server Integration', () => {
beforeAll(() => {
getCurrentScope().setClient(undefined);
init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
tracesSampleRate: 1.0,
Expand Down
1 change: 1 addition & 0 deletions packages/bun/test/integrations/bunserver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe('Bun Serve Integration', () => {
});

const setupClient = (options?: BunOptions): void => {
SentryCore.getCurrentScope().setClient(undefined);
init({
dsn: 'https://username@domain/123',
defaultIntegrations: false,
Expand Down
Loading
Loading