Skip to content

Commit 9b45d97

Browse files
committed
fix(cloudflare): Missing events inside waitUntil
1 parent 0591b1b commit 9b45d97

File tree

6 files changed

+229
-66
lines changed

6 files changed

+229
-66
lines changed

dev-packages/e2e-tests/test-applications/cloudflare-workers/src/index.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ export default Sentry.withSentry(
8181
},
8282
}),
8383
{
84-
async fetch(request, env) {
84+
async fetch(request, env, ctx) {
8585
const url = new URL(request.url);
8686
switch (url.pathname) {
8787
case '/rpc/throwException':
@@ -96,6 +96,25 @@ export default Sentry.withSentry(
9696
}
9797
}
9898
break;
99+
case '/waitUntil':
100+
console.log('waitUntil called');
101+
102+
const longRunningTask = async () => {
103+
await new Promise(resolve => setTimeout(resolve, 2000));
104+
105+
console.log('ʕっ•ᴥ•ʔっ');
106+
Sentry.captureException(new Error('ʕノ•ᴥ•ʔノ ︵ ┻━┻'));
107+
108+
return Sentry.startSpan({ name: 'longRunningTask' }, async () => {
109+
await new Promise(resolve => setTimeout(resolve, 1000));
110+
console.log(' /|\ ^._.^ /|\ ');
111+
});
112+
};
113+
114+
ctx.waitUntil(longRunningTask());
115+
116+
return new Response(null, { status: 200 });
117+
99118
case '/throwException':
100119
throw new Error('To be recorded in Sentry.');
101120
default:

dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/index.test.ts

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect, test } from '@playwright/test';
2-
import { waitForError, waitForRequest } from '@sentry-internal/test-utils';
2+
import { waitForError, waitForRequest, waitForTransaction } from '@sentry-internal/test-utils';
33
import { SDK_VERSION } from '@sentry/cloudflare';
44
import { WebSocket } from 'ws';
55

@@ -82,3 +82,103 @@ test('sends user-agent header with SDK name and version in envelope requests', a
8282
'user-agent': `sentry.javascript.cloudflare/${SDK_VERSION}`,
8383
});
8484
});
85+
86+
test.only('waitUntil', async ({ baseURL }) => {
87+
const errorWaiter = waitForError(
88+
'cloudflare-workers',
89+
event => event.exception?.values?.[0]?.value === 'ʕノ•ᴥ•ʔノ ︵ ┻━┻',
90+
);
91+
const httpTransactionWaiter = waitForTransaction(
92+
'cloudflare-workers',
93+
transactionEvent => transactionEvent.contexts?.trace?.op === 'http.server',
94+
);
95+
96+
const response = await fetch(`${baseURL}/waitUntil`);
97+
98+
expect(response.status).toBe(200);
99+
100+
const [errorEvent, transactionEvent] = await Promise.all([errorWaiter, httpTransactionWaiter]);
101+
102+
// ===== Error Event Assertions =====
103+
expect(errorEvent.exception?.values?.[0]).toMatchObject({
104+
type: 'Error',
105+
value: 'ʕノ•ᴥ•ʔノ ︵ ┻━┻',
106+
mechanism: {
107+
type: 'generic',
108+
handled: true,
109+
},
110+
});
111+
112+
// Error should have trace context linking it to the transaction
113+
expect(errorEvent.contexts?.trace?.trace_id).toBeDefined();
114+
expect(errorEvent.contexts?.trace?.span_id).toBeDefined();
115+
116+
// Error should have cloudflare-specific contexts
117+
expect(errorEvent.contexts?.cloud_resource).toEqual({ 'cloud.provider': 'cloudflare' });
118+
expect(errorEvent.contexts?.runtime).toEqual({ name: 'cloudflare' });
119+
120+
// Error should have request data
121+
expect(errorEvent.request).toMatchObject({
122+
method: 'GET',
123+
url: expect.stringContaining('/waitUntil'),
124+
});
125+
126+
// Error should have console breadcrumbs from before the error
127+
expect(errorEvent.breadcrumbs).toEqual([
128+
expect.objectContaining({ category: 'console', message: 'waitUntil called' }),
129+
expect.objectContaining({ category: 'console', message: 'ʕっ•ᴥ•ʔっ' }),
130+
]);
131+
132+
// ===== Transaction Event Assertions =====
133+
expect(transactionEvent.transaction).toBe('GET /waitUntil');
134+
expect(transactionEvent.type).toBe('transaction');
135+
expect(transactionEvent.transaction_info?.source).toBe('url');
136+
137+
// Transaction trace context (root span - no status/response code, those are on the fetch child span)
138+
expect(transactionEvent.contexts?.trace).toMatchObject({
139+
op: 'http.server',
140+
status: 'ok',
141+
origin: 'auto.http.cloudflare',
142+
data: expect.objectContaining({
143+
'sentry.op': 'http.server',
144+
'sentry.origin': 'auto.http.cloudflare',
145+
'http.request.method': 'GET',
146+
'url.path': '/waitUntil',
147+
'http.response.status_code': 200,
148+
}),
149+
});
150+
151+
expect(transactionEvent.spans).toEqual([
152+
expect.objectContaining({
153+
description: 'fetch',
154+
op: 'http.server',
155+
origin: 'auto.http.cloudflare',
156+
parent_span_id: transactionEvent.contexts?.trace?.span_id,
157+
}),
158+
expect.objectContaining({
159+
description: 'waitUntil',
160+
op: 'cloudflare.wait_until',
161+
origin: 'manual',
162+
parent_span_id: transactionEvent.spans?.[0]?.span_id,
163+
}),
164+
expect.objectContaining({
165+
description: 'longRunningTask',
166+
origin: 'manual',
167+
parent_span_id: transactionEvent.spans?.[0]?.span_id,
168+
}),
169+
]);
170+
171+
// Transaction should have all console breadcrumbs including the one after the span completes
172+
expect(transactionEvent.breadcrumbs).toEqual([
173+
expect.objectContaining({ category: 'console', message: 'waitUntil called' }),
174+
expect.objectContaining({ category: 'console', message: 'ʕっ•ᴥ•ʔっ' }),
175+
expect.objectContaining({ category: 'console', message: ' /|\ ^._.^ /|\ ' }),
176+
]);
177+
178+
// ===== Cross-event Assertions =====
179+
// Error and transaction should share the same trace_id
180+
expect(transactionEvent.contexts?.trace?.trace_id).toBe(errorEvent.contexts?.trace?.trace_id);
181+
182+
// The error's span_id should match the fetch span's span_id (error captured during waitUntil execution)
183+
expect(errorEvent.contexts?.trace?.span_id).toBe(transactionEvent.spans?.[0]?.span_id);
184+
});

packages/cloudflare/src/client.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,18 @@ export class CloudflareClient extends ServerRuntimeClient {
6363
});
6464
}
6565

66+
/**
67+
* Returns a promise that resolves when all waitUntil promises have completed.
68+
* This allows the root span to stay open until all waitUntil work is done.
69+
*
70+
* @return {Promise<void>} A promise that resolves when all waitUntil promises are done.
71+
*/
72+
public async waitUntilDone(): Promise<void> {
73+
if (this._flushLock) {
74+
await this._flushLock.finalize();
75+
}
76+
}
77+
6678
/**
6779
* Flushes pending operations and ensures all data is processed.
6880
* If a timeout is provided, the operation will be completed within the specified time limit.
@@ -73,9 +85,7 @@ export class CloudflareClient extends ServerRuntimeClient {
7385
* @return {Promise<boolean>} A promise that resolves to a boolean indicating whether the flush operation was successful.
7486
*/
7587
public async flush(timeout?: number): Promise<boolean> {
76-
if (this._flushLock) {
77-
await this._flushLock.finalize();
78-
}
88+
await this.waitUntilDone();
7989

8090
if (this._pendingSpans.size > 0 && this._spanCompletionPromise) {
8191
DEBUG_BUILD &&

packages/cloudflare/src/flush.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { ExecutionContext } from '@cloudflare/workers-types';
2+
import { startSpan } from '@sentry/core';
23

34
type FlushLock = {
45
readonly ready: Promise<void>;
@@ -22,9 +23,18 @@ export function makeFlushLock(context: ExecutionContext): FlushLock {
2223
const originalWaitUntil = context.waitUntil.bind(context) as typeof context.waitUntil;
2324
context.waitUntil = promise => {
2425
pending++;
26+
2527
return originalWaitUntil(
26-
promise.finally(() => {
27-
if (--pending === 0) resolveAllDone();
28+
// Wrap the promise in a new scope and transaction so spans created inside
29+
// waitUntil callbacks are properly isolated from the HTTP request transaction
30+
startSpan({ op: 'cloudflare.wait_until', name: 'waitUntil' }, async () => {
31+
// By awaiting the promise inside the new scope, all of its continuations
32+
// will execute in this isolated scope
33+
await promise;
34+
}).finally(() => {
35+
if (--pending === 0) {
36+
resolveAllDone();
37+
}
2838
}),
2939
);
3040
};

packages/cloudflare/src/request.ts

Lines changed: 66 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import type { CloudflareOptions } from './client';
1717
import { addCloudResourceContext, addCultureContext, addRequest } from './scope-utils';
1818
import { init } from './sdk';
19+
import { endSpanAfterWaitUntil } from './utils/endSpanAfterWaitUntil';
1920
import { classifyResponseStreaming } from './utils/streaming';
2021

2122
interface RequestHandlerWrapperOptions {
@@ -107,73 +108,79 @@ export function wrapRequestHandler(
107108
// See: https://developers.cloudflare.com/workers/runtime-apis/performance/
108109

109110
// Use startSpanManual to control when span ends (needed for streaming responses)
110-
return startSpanManual({ name, attributes }, async span => {
111-
let res: Response;
112-
113-
try {
114-
res = await handler();
115-
setHttpStatus(span, res.status);
116-
117-
// After the handler runs, the span name might have been updated by nested instrumentation
118-
// (e.g., Remix parameterizing routes). The span should already have the correct name
119-
// from that instrumentation, so we don't need to do anything here.
120-
} catch (e) {
121-
span.end();
122-
if (captureErrors) {
123-
captureException(e, { mechanism: { handled: false, type: 'auto.http.cloudflare' } });
124-
}
125-
waitUntil?.(flush(2000));
126-
throw e;
127-
}
111+
return startSpanManual({ name, attributes }, async rootSpan => {
112+
return startSpanManual({ name: 'fetch', attributes }, async fetchSpan => {
113+
const finishSpansAndWaitUntil = (): void => {
114+
fetchSpan.end();
115+
waitUntil?.(flush(2000));
116+
waitUntil?.(endSpanAfterWaitUntil(rootSpan));
117+
};
128118

129-
// Classify response to detect actual streaming
130-
const classification = classifyResponseStreaming(res);
119+
let res: Response;
131120

132-
if (classification.isStreaming && res.body) {
133-
// Streaming response detected - monitor consumption to keep span alive
134121
try {
135-
const [clientStream, monitorStream] = res.body.tee();
122+
res = await handler();
123+
setHttpStatus(rootSpan, res.status);
136124

137-
// Monitor stream consumption and end span when complete
138-
const streamMonitor = (async () => {
139-
const reader = monitorStream.getReader();
125+
// After the handler runs, the span name might have been updated by nested instrumentation
126+
// (e.g., Remix parameterizing routes). The span should already have the correct name
127+
// from that instrumentation, so we don't need to do anything here.
128+
} catch (e) {
129+
// For errors, we still wait for waitUntil promises before ending the span
130+
// so that any spans created in waitUntil callbacks are captured
131+
if (captureErrors) {
132+
captureException(e, { mechanism: { handled: false, type: 'auto.http.cloudflare' } });
133+
}
134+
finishSpansAndWaitUntil();
135+
throw e;
136+
}
140137

141-
try {
142-
let done = false;
143-
while (!done) {
144-
const result = await reader.read();
145-
done = result.done;
138+
// Classify response to detect actual streaming
139+
const classification = classifyResponseStreaming(res);
140+
141+
if (classification.isStreaming && res.body) {
142+
// Streaming response detected - monitor consumption to keep span alive
143+
try {
144+
const [clientStream, monitorStream] = res.body.tee();
145+
146+
// Monitor stream consumption and end span when complete
147+
const streamMonitor = (async () => {
148+
const reader = monitorStream.getReader();
149+
150+
try {
151+
let done = false;
152+
while (!done) {
153+
const result = await reader.read();
154+
done = result.done;
155+
}
156+
} catch {
157+
// Stream error or cancellation - will end span in finally
158+
} finally {
159+
reader.releaseLock();
160+
finishSpansAndWaitUntil();
146161
}
147-
} catch {
148-
// Stream error or cancellation - will end span in finally
149-
} finally {
150-
reader.releaseLock();
151-
span.end();
152-
waitUntil?.(flush(2000));
153-
}
154-
})();
155-
156-
// Keep worker alive until stream monitoring completes (otherwise span won't end)
157-
waitUntil?.(streamMonitor);
158-
159-
// Return response with client stream
160-
return new Response(clientStream, {
161-
status: res.status,
162-
statusText: res.statusText,
163-
headers: res.headers,
164-
});
165-
} catch (e) {
166-
// tee() failed (e.g stream already locked) - fall back to non-streaming handling
167-
span.end();
168-
waitUntil?.(flush(2000));
169-
return res;
162+
})();
163+
164+
// Keep worker alive until stream monitoring completes (otherwise span won't end)
165+
waitUntil?.(streamMonitor);
166+
167+
// Return response with client stream
168+
return new Response(clientStream, {
169+
status: res.status,
170+
statusText: res.statusText,
171+
headers: res.headers,
172+
});
173+
} catch (e) {
174+
// tee() failed (e.g stream already locked) - fall back to non-streaming handling
175+
finishSpansAndWaitUntil();
176+
return res;
177+
}
170178
}
171-
}
172179

173-
// Non-streaming response - end span immediately and return original
174-
span.end();
175-
waitUntil?.(flush(2000));
176-
return res;
180+
// Non-streaming response - end span after all waitUntil promises complete
181+
finishSpansAndWaitUntil();
182+
return res;
183+
});
177184
});
178185
},
179186
);
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { flush, getClient, type Span } from '@sentry/core';
2+
import type { CloudflareClient } from '../client';
3+
4+
/**
5+
* Helper to end span after all waitUntil promises complete.
6+
* This ensures spans created in waitUntil callbacks are captured in the same transaction.
7+
*/
8+
export const endSpanAfterWaitUntil = async (span: Span): Promise<void> => {
9+
const cloudflareClient = getClient<CloudflareClient>();
10+
11+
if (cloudflareClient) {
12+
await cloudflareClient.waitUntilDone();
13+
}
14+
15+
span.end();
16+
await flush(2000);
17+
};

0 commit comments

Comments
 (0)