Skip to content

Commit ec00267

Browse files
committed
fix: bugbot issue
1 parent 01a2e40 commit ec00267

6 files changed

Lines changed: 31 additions & 20 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@
9090

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

93-
- **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`.
93+
- **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`.
9494

9595
## 11.2.0
9696

‎docs/repeated-init.md‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,14 @@ When `init()` replaces a client, nothing closes the old one:
3434

3535
## Current behavior
3636

37-
| Entry point | Repeated call |
38-
| ------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
39-
| `initAndBind` (browser and its wrappers, Deno), Node `_init`, Vercel Edge | Warns, then replaces the client. Returns the new client. |
40-
| Next.js server, Remix server, Hono Node | Keeps the first client and returns it. Logs in debug mode only. |
41-
| Hono Bun and Deno | Keeps the first client and returns it. Warns with its own text. |
42-
| Nuxt server | Keeps the first client and returns it. Logs that a `--import` preload is no longer needed. |
43-
| 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. |
44-
| Next.js edge | Warns, then replaces the client. Returns `void`. |
37+
| Entry point | Repeated call |
38+
| ------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
39+
| `initAndBind` (browser and its wrappers, Deno), Node `_init`, Vercel Edge | Warns, then replaces the client. Returns the new client. |
40+
| Next.js server, Remix server, Hono Node | Keeps the first client and returns it. Logs in debug mode only. |
41+
| Hono Bun and Deno | Keeps the first client and returns it. Warns with its own text. |
42+
| Nuxt server | Keeps the first client and returns it. Logs that a `--import` preload is no longer needed. |
43+
| 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. |
44+
| Next.js edge | Warns, then replaces the client. Returns `void`. |
4545

4646
The shared warning lives in `warnIfClientIsActive()` in
4747
`packages/core/src/sdk.ts`. Core exports it as

‎packages/cloudflare/src/baseSdk.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { Integration } from '@sentry/core';
2-
import { debug, getCurrentScope, setCurrentClient } from '@sentry/core';
2+
import { _INTERNAL_isClientClosed, debug, getCurrentScope, setCurrentClient } from '@sentry/core';
33
import {
44
consoleIntegration,
55
conversationIdIntegration,
@@ -16,7 +16,7 @@ import {
1616
import type { CloudflareClientOptions, CloudflareOptions } from './client';
1717
import { CloudflareClient } from './client';
1818
import { makeFlushLock } from './flush';
19-
import { _clearGlobalClientCache, cacheClient, getCachedClient } from './clientCache';
19+
import { cacheClient, getCachedClient } from './clientCache';
2020
import { DEBUG_BUILD } from './debug-build';
2121
import { fetchIntegration } from './integrations/fetch';
2222
import { httpServerIntegration } from './integrations/httpServer';
@@ -94,7 +94,9 @@ export function initWithDefaultIntegrations(
9494
const cached = getCachedClient();
9595
const cacheEnabled = options.cacheClient !== false;
9696

97-
if (cacheEnabled && cached) {
97+
// A closed client drops all data. `close()` marks the client before it
98+
// flushes, so this also skips a client that is still closing.
99+
if (cacheEnabled && cached && !_INTERNAL_isClientClosed(cached)) {
98100
if (DEBUG_BUILD && cached.getOptions().dsn !== options.dsn) {
99101
debug.warn(
100102
'[Sentry] init() was called with a different DSN than the cached client of this isolate; the cached client keeps its DSN. Pass `cacheClient: false` for per-invocation options.',
@@ -145,12 +147,6 @@ export function initWithDefaultIntegrations(
145147

146148
if (cacheEnabled && client) {
147149
cacheClient(client);
148-
// A closed client drops all data, so the next `init()` must not reuse it.
149-
client.on('close', () => {
150-
if (getCachedClient() === client) {
151-
_clearGlobalClientCache();
152-
}
153-
});
154150
}
155151

156152
// An instrumented module that first evaluates AFTER this init (e.g. a driver

‎packages/cloudflare/src/clientCache.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ export function cacheClient(client: CloudflareClient): void {
1717
(GLOBAL_OBJ as GlobalWithCloudflareClient)[GLOBAL_CLIENT_KEY] = client;
1818
}
1919

20-
/** @hidden Clears the isolate's cached Cloudflare client. */
20+
/** @hidden Only for testing - clears the isolate's cached Cloudflare client. */
2121
export function _clearGlobalClientCache(): void {
2222
(GLOBAL_OBJ as GlobalWithCloudflareClient)[GLOBAL_CLIENT_KEY] = undefined;
2323
}

‎packages/cloudflare/test/sdk.test.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,21 @@ describe('cacheClient', () => {
166166
expect(init({ ...options })).toBe(second);
167167
});
168168

169+
test('sets up a new cached client while the cached client is closing', async () => {
170+
const options = {
171+
dsn: 'https://public@dsn.ingest.sentry.io/1337',
172+
} as const;
173+
174+
const first = init({ ...options });
175+
const closing = first!.close();
176+
const second = init({ ...options });
177+
await closing;
178+
179+
expect(second).not.toBe(first);
180+
expect(second?.getOptions().enabled).not.toBe(false);
181+
expect(init({ ...options })).toBe(second);
182+
});
183+
169184
test('keeps the cached client when a client that is not cached closes', async () => {
170185
const cached = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337' });
171186
const uncached = init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', cacheClient: false });

‎packages/core/src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ export { Scope } from './scope';
5858
export type { CaptureContext, ScopeContext, ScopeData } from './scope';
5959
export { notifyEventProcessors } from './eventProcessors';
6060
export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint, SENTRY_API_VERSION } from './api';
61-
export { Client } from './client';
61+
export { Client, isClientClosed as _INTERNAL_isClientClosed } from './client';
6262
export {
6363
getActiveClient as _INTERNAL_getActiveClient,
6464
initAndBind,

0 commit comments

Comments
 (0)