Skip to content

Commit 6424081

Browse files
JPeer264claude
JPeer264
and
claude
committed
fix(cloudflare): Skip SDK initialization for OPTIONS/HEAD requests
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent f53c493 commit 6424081

6 files changed

Lines changed: 158 additions & 68 deletions

File tree

‎packages/cloudflare/src/instrumentations/worker/instrumentFetch.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ export function instrumentExportedHandlerFetch<T extends ExportedHandler<any, an
2525
new Proxy(original, {
2626
apply(target, thisArg, args: Parameters<NonNullable<T['fetch']>>) {
2727
const [request, env, ctx] = args;
28+
29+
if (request.method === 'OPTIONS' || request.method === 'HEAD') {
30+
return target.apply(thisArg, args);
31+
}
32+
2833
const context = instrumentContext(ctx);
2934
const options = getFinalOptions(optionsCallback(env), env);
3035
args[1] = instrumentEnv(env, options);
@@ -53,6 +58,10 @@ export function instrumentWorkerEntrypointFetch<T extends WorkerEntrypoint>(
5358
apply(target, thisArg, args: [Request]) {
5459
const [request] = args;
5560

61+
if (request.method === 'OPTIONS' || request.method === 'HEAD') {
62+
return Reflect.apply(target, thisArg, args);
63+
}
64+
5665
return wrapRequestHandler({ options, request, context }, () => Reflect.apply(target, thisArg, args));
5766
},
5867
});

‎packages/cloudflare/src/pages-plugin.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ export function sentryPagesPlugin<
4848
): PagesPluginFunction<Env, Params, Data, PluginParams> {
4949
setAsyncLocalStorageAsyncContextStrategy();
5050
return context => {
51+
if (context.request.method === 'OPTIONS' || context.request.method === 'HEAD') {
52+
return context.next();
53+
}
54+
5155
const options = typeof handlerOrOptions === 'function' ? handlerOrOptions(context) : handlerOrOptions;
5256
return wrapRequestHandler({ options, request: context.request, context: { ...context, props: {} } }, () =>
5357
context.next(),

‎packages/cloudflare/src/request.ts‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -85,20 +85,6 @@ export function wrapRequestHandler(
8585
}
8686
}
8787

88-
// Do not capture spans for OPTIONS and HEAD requests
89-
if (request.method === 'OPTIONS' || request.method === 'HEAD') {
90-
try {
91-
return await handler();
92-
} catch (e) {
93-
if (captureErrors) {
94-
captureException(e, { mechanism: { handled: false, type: 'auto.http.cloudflare' } });
95-
}
96-
throw e;
97-
} finally {
98-
waitUntil?.(flushAndDispose(client));
99-
}
100-
}
101-
10288
if (client) {
10389
await captureIncomingRequestBody(client, request);
10490
}

‎packages/cloudflare/test/instrumentations/worker/instrumentFetch.test.ts‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,4 +174,85 @@ describe('instrumentFetch', () => {
174174
await Promise.all(waits);
175175
expect(flush).toHaveBeenCalledOnce();
176176
});
177+
178+
test('OPTIONS requests bypass SDK instrumentation', async () => {
179+
const originalFetch = vi.fn().mockReturnValue(new Response('options response'));
180+
const handler = {
181+
fetch: originalFetch,
182+
} satisfies ExportedHandler<typeof MOCK_ENV>;
183+
184+
const optionsCallback = vi.fn().mockReturnValue({ dsn: MOCK_ENV.SENTRY_DSN });
185+
186+
const wrappedHandler = withSentry(optionsCallback, handler);
187+
const result = await wrappedHandler.fetch?.(
188+
new Request('https://example.com', { method: 'OPTIONS' }),
189+
MOCK_ENV,
190+
createMockExecutionContext(),
191+
);
192+
193+
expect(originalFetch).toHaveBeenCalledTimes(1);
194+
expect(optionsCallback).not.toHaveBeenCalled();
195+
expect(result?.status).toBe(200);
196+
if (result) {
197+
expect(await result.text()).toBe('options response');
198+
}
199+
});
200+
201+
test('HEAD requests bypass SDK instrumentation', async () => {
202+
const originalFetch = vi.fn().mockReturnValue(new Response('head response'));
203+
const handler = {
204+
fetch: originalFetch,
205+
} satisfies ExportedHandler<typeof MOCK_ENV>;
206+
207+
const optionsCallback = vi.fn().mockReturnValue({ dsn: MOCK_ENV.SENTRY_DSN });
208+
209+
const wrappedHandler = withSentry(optionsCallback, handler);
210+
const result = await wrappedHandler.fetch?.(
211+
new Request('https://example.com', { method: 'HEAD' }),
212+
MOCK_ENV,
213+
createMockExecutionContext(),
214+
);
215+
216+
expect(originalFetch).toHaveBeenCalledTimes(1);
217+
expect(optionsCallback).not.toHaveBeenCalled();
218+
expect(result?.status).toBe(200);
219+
});
220+
221+
test('GET requests are instrumented', async () => {
222+
const originalFetch = vi.fn().mockReturnValue(new Response('get response'));
223+
const handler = {
224+
fetch: originalFetch,
225+
} satisfies ExportedHandler<typeof MOCK_ENV>;
226+
227+
const optionsCallback = vi.fn().mockReturnValue({ dsn: MOCK_ENV.SENTRY_DSN });
228+
229+
const wrappedHandler = withSentry(optionsCallback, handler);
230+
await wrappedHandler.fetch?.(
231+
new Request('https://example.com', { method: 'GET' }),
232+
MOCK_ENV,
233+
createMockExecutionContext(),
234+
);
235+
236+
expect(originalFetch).toHaveBeenCalledTimes(1);
237+
expect(optionsCallback).toHaveBeenCalledTimes(1);
238+
});
239+
240+
test('POST requests are instrumented', async () => {
241+
const originalFetch = vi.fn().mockReturnValue(new Response('post response'));
242+
const handler = {
243+
fetch: originalFetch,
244+
} satisfies ExportedHandler<typeof MOCK_ENV>;
245+
246+
const optionsCallback = vi.fn().mockReturnValue({ dsn: MOCK_ENV.SENTRY_DSN });
247+
248+
const wrappedHandler = withSentry(optionsCallback, handler);
249+
await wrappedHandler.fetch?.(
250+
new Request('https://example.com', { method: 'POST' }),
251+
MOCK_ENV,
252+
createMockExecutionContext(),
253+
);
254+
255+
expect(originalFetch).toHaveBeenCalledTimes(1);
256+
expect(optionsCallback).toHaveBeenCalledTimes(1);
257+
});
177258
});

‎packages/cloudflare/test/pages-plugin.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,68 @@ describe('sentryPagesPlugin', () => {
5656
expect(result.status).toBe(response.status);
5757
expect(await result.text()).toBe('test');
5858
});
59+
60+
test('OPTIONS requests bypass SDK instrumentation', async () => {
61+
const mockOptionsHandler = vi.fn().mockReturnValue(MOCK_OPTIONS);
62+
const mockOnRequest = sentryPagesPlugin(mockOptionsHandler);
63+
const mockNext = vi.fn().mockResolvedValue(new Response('options response'));
64+
65+
const result = await mockOnRequest({
66+
request: new Request('https://example.com', { method: 'OPTIONS' }),
67+
functionPath: 'test',
68+
waitUntil: vi.fn(),
69+
passThroughOnException: vi.fn(),
70+
next: mockNext,
71+
env: { ASSETS: { fetch: vi.fn() } },
72+
params: {},
73+
data: {},
74+
pluginArgs: MOCK_OPTIONS,
75+
});
76+
77+
expect(mockOptionsHandler).not.toHaveBeenCalled();
78+
expect(mockNext).toHaveBeenCalledTimes(1);
79+
expect(result.status).toBe(200);
80+
});
81+
82+
test('HEAD requests bypass SDK instrumentation', async () => {
83+
const mockOptionsHandler = vi.fn().mockReturnValue(MOCK_OPTIONS);
84+
const mockOnRequest = sentryPagesPlugin(mockOptionsHandler);
85+
const mockNext = vi.fn().mockResolvedValue(new Response('head response'));
86+
87+
const result = await mockOnRequest({
88+
request: new Request('https://example.com', { method: 'HEAD' }),
89+
functionPath: 'test',
90+
waitUntil: vi.fn(),
91+
passThroughOnException: vi.fn(),
92+
next: mockNext,
93+
env: { ASSETS: { fetch: vi.fn() } },
94+
params: {},
95+
data: {},
96+
pluginArgs: MOCK_OPTIONS,
97+
});
98+
99+
expect(mockOptionsHandler).not.toHaveBeenCalled();
100+
expect(mockNext).toHaveBeenCalledTimes(1);
101+
expect(result.status).toBe(200);
102+
});
103+
104+
test('GET requests are instrumented', async () => {
105+
const mockOptionsHandler = vi.fn().mockReturnValue(MOCK_OPTIONS);
106+
const mockOnRequest = sentryPagesPlugin(mockOptionsHandler);
107+
const mockNext = vi.fn().mockResolvedValue(new Response('get response'));
108+
109+
await mockOnRequest({
110+
request: new Request('https://example.com', { method: 'GET' }),
111+
functionPath: 'test',
112+
waitUntil: vi.fn(),
113+
passThroughOnException: vi.fn(),
114+
next: mockNext,
115+
env: { ASSETS: { fetch: vi.fn() } },
116+
params: {},
117+
data: {},
118+
pluginArgs: MOCK_OPTIONS,
119+
});
120+
121+
expect(mockOptionsHandler).toHaveBeenCalledTimes(1);
122+
});
59123
});

‎packages/cloudflare/test/request.test.ts‎

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -563,60 +563,6 @@ describe('flushAndDispose', () => {
563563
disposeSpy.mockRestore();
564564
});
565565

566-
test('dispose is called for OPTIONS requests', async () => {
567-
const context = createMockExecutionContext();
568-
const waits: Promise<unknown>[] = [];
569-
const waitUntil = vi.fn(promise => waits.push(promise));
570-
(context as any).waitUntil = waitUntil;
571-
572-
const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose');
573-
const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true);
574-
575-
await wrapRequestHandler(
576-
{
577-
options: MOCK_OPTIONS,
578-
request: new Request('https://example.com', { method: 'OPTIONS' }),
579-
context,
580-
},
581-
() => new Response('', { status: 200 }),
582-
);
583-
584-
// Wait for all waitUntil promises to resolve
585-
await Promise.all(waits);
586-
587-
expect(disposeSpy).toHaveBeenCalled();
588-
589-
flushSpy.mockRestore();
590-
disposeSpy.mockRestore();
591-
});
592-
593-
test('dispose is called for HEAD requests', async () => {
594-
const context = createMockExecutionContext();
595-
const waits: Promise<unknown>[] = [];
596-
const waitUntil = vi.fn(promise => waits.push(promise));
597-
(context as any).waitUntil = waitUntil;
598-
599-
const disposeSpy = vi.spyOn(CloudflareClient.prototype, 'dispose');
600-
const flushSpy = vi.spyOn(SentryCore.Client.prototype, 'flush').mockResolvedValue(true);
601-
602-
await wrapRequestHandler(
603-
{
604-
options: MOCK_OPTIONS,
605-
request: new Request('https://example.com', { method: 'HEAD' }),
606-
context,
607-
},
608-
() => new Response('', { status: 200 }),
609-
);
610-
611-
// Wait for all waitUntil promises to resolve
612-
await Promise.all(waits);
613-
614-
expect(disposeSpy).toHaveBeenCalled();
615-
616-
flushSpy.mockRestore();
617-
disposeSpy.mockRestore();
618-
});
619-
620566
test('dispose is called after streaming response completes', async () => {
621567
const context = createMockExecutionContext();
622568
const waits: Promise<unknown>[] = [];

0 commit comments

Comments
 (0)