Skip to content

Commit 10b6d86

Browse files
authored
fix(cloudflare): Recreate client when previous one was disposed (#19727)
I think this is even the actual fix for #19589 This could happen right now for alarms. When an alarm is being executed the first Client is getting disposed. Once the alarm is getting triggered it might be that it wants to reuse the previous Client, which didn't work as it got disposed. With that fix we actually check if the client is also disposed (by checking if there is a transport), if there is none we just create a new client.
1 parent 1c249d9 commit 10b6d86

File tree

2 files changed

+108
-10
lines changed

2 files changed

+108
-10
lines changed

packages/cloudflare/src/wrapMethodWithSentry.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,17 @@ export function wrapMethodWithSentry<T extends OriginalMethod>(
6868

6969
const waitUntil = context?.waitUntil?.bind?.(context);
7070

71-
const currentClient = scope.getClient();
72-
if (!currentClient) {
71+
let currentClient = scope.getClient();
72+
// Check if client exists AND is still usable (transport not disposed)
73+
// This handles the case where a previous handler disposed the client
74+
// but the scope still holds a reference to it (e.g., alarm handlers in Durable Objects)
75+
if (!currentClient || !currentClient.getTransport()) {
7376
const client = init({ ...wrapperOptions.options, ctx: context as unknown as ExecutionContext | undefined });
7477
scope.setClient(client);
78+
currentClient = client;
7579
}
7680

77-
const clientToDispose = currentClient || scope.getClient();
81+
const clientToDispose = currentClient;
7882

7983
if (!wrapperOptions.spanName) {
8084
try {

packages/cloudflare/test/wrapMethodWithSentry.test.ts

Lines changed: 101 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,40 @@
11
import * as sentryCore from '@sentry/core';
22
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
33
import { isInstrumented } from '../src/instrument';
4+
import * as sdk from '../src/sdk';
45
import { wrapMethodWithSentry } from '../src/wrapMethodWithSentry';
56

6-
// Mock the SDK init to avoid actual SDK initialization
7-
vi.mock('../src/sdk', () => ({
8-
init: vi.fn(() => ({
7+
function createMockClient(hasTransport: boolean = true) {
8+
return {
99
getOptions: () => ({}),
1010
on: vi.fn(),
1111
dispose: vi.fn(),
12-
})),
12+
getTransport: vi.fn().mockReturnValue(hasTransport ? { send: vi.fn() } : undefined),
13+
};
14+
}
15+
16+
// Mock the SDK init to avoid actual SDK initialization
17+
vi.mock('../src/sdk', () => ({
18+
init: vi.fn(() => createMockClient(true)),
1319
}));
1420

1521
// Mock sentry/core functions
1622
vi.mock('@sentry/core', async importOriginal => {
17-
const actual = await importOriginal();
23+
const actual = await importOriginal<object>();
1824
return {
1925
...actual,
2026
getClient: vi.fn(),
21-
withIsolationScope: vi.fn((callback: (scope: any) => any) => callback(createMockScope())),
22-
withScope: vi.fn((callback: (scope: any) => any) => callback(createMockScope())),
27+
withIsolationScope: vi.fn((callback: (scope: unknown) => unknown) => callback(createMockScope())),
28+
withScope: vi.fn((callback: (scope: unknown) => unknown) => callback(createMockScope())),
2329
startSpan: vi.fn((opts, callback) => callback(createMockSpan())),
2430
captureException: vi.fn(),
2531
flush: vi.fn().mockResolvedValue(true),
2632
getActiveSpan: vi.fn(),
2733
};
2834
});
2935

36+
const mockedWithIsolationScope = vi.mocked(sentryCore.withIsolationScope);
37+
3038
function createMockScope() {
3139
return {
3240
getClient: vi.fn(),
@@ -307,4 +315,90 @@ describe('wrapMethodWithSentry', () => {
307315
expect(handler.mock.instances[0]).toBe(thisArg);
308316
});
309317
});
318+
319+
describe('client re-initialization', () => {
320+
it('creates a new client when scope has no client', async () => {
321+
const scope = new sentryCore.Scope();
322+
323+
mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope)));
324+
325+
const spyClient = vi.spyOn(scope, 'setClient');
326+
const handler = vi.fn().mockResolvedValue('result');
327+
const options = {
328+
options: { dsn: 'https://test@sentry.io/123' },
329+
context: createMockContext(),
330+
};
331+
332+
const wrapped = wrapMethodWithSentry(options, handler);
333+
334+
await wrapped();
335+
336+
expect(sdk.init).toHaveBeenCalledWith(
337+
expect.objectContaining({
338+
dsn: 'https://test@sentry.io/123',
339+
}),
340+
);
341+
expect(spyClient).toHaveBeenCalled();
342+
});
343+
344+
it('creates a new client when existing client has no transport (disposed)', async () => {
345+
const disposedClient = {
346+
getOptions: () => ({}),
347+
on: vi.fn(),
348+
dispose: vi.fn(),
349+
getTransport: vi.fn().mockReturnValue(undefined),
350+
} as unknown as sentryCore.Client;
351+
352+
const scope = new sentryCore.Scope();
353+
354+
scope.setClient(disposedClient);
355+
mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope)));
356+
357+
const spyClient = vi.spyOn(scope, 'setClient');
358+
const handler = vi.fn().mockResolvedValue('result');
359+
const options = {
360+
options: { dsn: 'https://test@sentry.io/123' },
361+
context: createMockContext(),
362+
};
363+
364+
const wrapped = wrapMethodWithSentry(options, handler);
365+
await wrapped();
366+
367+
expect(sdk.init).toHaveBeenCalledWith(
368+
expect.objectContaining({
369+
dsn: 'https://test@sentry.io/123',
370+
}),
371+
);
372+
expect(spyClient).toHaveBeenCalled();
373+
});
374+
375+
it('does not create a new client when existing client has valid transport', async () => {
376+
const validClient = {
377+
getOptions: () => ({}),
378+
on: vi.fn(),
379+
dispose: vi.fn(),
380+
getTransport: vi.fn().mockReturnValue({ send: vi.fn() }),
381+
} as unknown as sentryCore.Client;
382+
383+
const scope = new sentryCore.Scope();
384+
385+
scope.setClient(validClient);
386+
mockedWithIsolationScope.mockImplementation(vi.fn(callback => callback(scope)));
387+
vi.mocked(sdk.init).mockClear();
388+
389+
const spyClient = vi.spyOn(scope, 'setClient');
390+
const handler = vi.fn().mockResolvedValue('result');
391+
const options = {
392+
options: { dsn: 'https://test@sentry.io/123' },
393+
context: createMockContext(),
394+
};
395+
396+
const wrapped = wrapMethodWithSentry(options, handler);
397+
398+
await wrapped();
399+
400+
expect(sdk.init).not.toHaveBeenCalled();
401+
expect(spyClient).not.toHaveBeenCalled();
402+
});
403+
});
310404
});

0 commit comments

Comments
 (0)