diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/index.ts b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/index.ts new file mode 100644 index 000000000000..37be3907a11c --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/index.ts @@ -0,0 +1,50 @@ +import * as Sentry from '@sentry/cloudflare'; +import { WorkflowEntrypoint } from 'cloudflare:workers'; +import type { WorkflowEvent, WorkflowStep } from 'cloudflare:workers'; +import { lastSend } from './lastSend'; + +interface Env { + SERVER_URL: string; + ISSUE_WORKFLOW: Workflow; +} + +// The Workflow from https://github.com/getsentry/sentry-javascript/issues/24482. Each step flushes its span to an +// ingest that never answers. The run reports to SERVER_URL once the SDK has aborted one of those pending sends. +export class IssueWorkflow extends WorkflowEntrypoint { + async run(_event: WorkflowEvent, step: WorkflowStep): Promise { + const stepSendAborted = new Promise(resolve => (lastSend.onAbort = resolve)); + + for (let index = 0; index < 100; index++) { + await step.do(`step-${index}`, async () => index); + } + + await stepSendAborted; + await fetch(`${this.env.SERVER_URL}/result`, { method: 'POST', body: JSON.stringify({ send: 'aborted' }) }); + } +} + +export default { + async fetch(request, env, ctx) { + const url = new URL(request.url); + + if (url.pathname === '/workflow/trigger') { + const instance = await env.ISSUE_WORKFLOW.create(); + return Response.json({ id: instance.id }); + } + + // The flush runs inside the invocation, so the send is still pending when its drain times out. + if (url.pathname === '/flush-with-timeout') { + Sentry.captureException(new Error('Captured on /flush-with-timeout')); + lastSend.aborted = false; + const flushed = await Sentry.flush(500); + return Response.json({ flushed, send: lastSend.aborted ? 'aborted' : 'not aborted' }); + } + + if (url.pathname === '/pending-wait-until') { + ctx.waitUntil(new Promise(resolve => setTimeout(resolve, 120_000))); + Sentry.captureException(new Error('Captured on /pending-wait-until')); + } + + return new Response('ok'); + }, +} satisfies ExportedHandler; diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/instrument.server.ts b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/instrument.server.ts new file mode 100644 index 000000000000..3e8e2ff64d7b --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/instrument.server.ts @@ -0,0 +1,28 @@ +import { defineCloudflareOptions } from '@sentry/cloudflare'; +import { lastSend } from './lastSend'; + +interface Env { + SENTRY_DSN: string; + SERVER_URL: string; + // "true" sends envelopes to SERVER_URL, a server that never answers + SLOW_INGEST?: string; + // "false" creates one client per invocation, which waits for the invocation's flush lock + CACHE_CLIENT?: string; + // "true" samples every trace, so the Workflow steps create spans to send + TRACING?: string; +} + +export default defineCloudflareOptions((env: Env) => ({ + dsn: env.SLOW_INGEST === 'true' ? `${env.SERVER_URL.replace('://', '://public@')}/1337` : env.SENTRY_DSN, + cacheClient: env.CACHE_CLIENT !== 'false', + tracesSampleRate: env.TRACING === 'true' ? 1 : undefined, + transportOptions: { + fetch: (input, init) => { + init?.signal?.addEventListener('abort', () => { + lastSend.aborted = true; + lastSend.onAbort?.(); + }); + return fetch(input, init); + }, + }, +})); diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/lastSend.ts b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/lastSend.ts new file mode 100644 index 000000000000..6c5cf6cfdaf8 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/lastSend.ts @@ -0,0 +1,3 @@ +// `aborted`: whether the transport aborted an envelope fetch since it was last reset. +// `onAbort`: called each time the transport aborts an envelope fetch. +export const lastSend: { aborted: boolean; onAbort?: () => void } = { aborted: false }; diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/test.ts b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/test.ts new file mode 100644 index 000000000000..83b5376fca80 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/test.ts @@ -0,0 +1,76 @@ +import type { Envelope, Event } from '@sentry/core'; +import { createServer } from 'node:http'; +import type { AddressInfo } from 'node:net'; +import { expect, it, onTestFinished } from 'vitest'; +import { createRunner } from '../../runner'; + +// Starts an ingest server that never answers envelope requests, so every send stays pending. The Workflow +// posts its result to `/result`, which resolves the returned promise with the posted body. +async function startSilentIngest(): Promise<{ url: string; result: Promise }> { + let resolveResult!: (body: unknown) => void; + const result = new Promise(resolve => (resolveResult = resolve)); + + const server = createServer((req, res) => { + if (req.url !== '/result') { + return; + } + let body = ''; + req.on('data', chunk => (body += chunk)); + req.on('end', () => { + res.end(); + resolveResult(JSON.parse(body)); + }); + }); + await new Promise(resolve => server.listen(0, resolve)); + onTestFinished(() => { + server.closeAllConnections(); + server.close(); + }); + + return { url: `http://localhost:${(server.address() as AddressInfo).port}`, result }; +} + +it.for([true, false])( + 'cacheClient: %s - aborts a send that is still pending when the flush times out', + async (cacheClient, { signal }) => { + const ingest = await startSilentIngest(); + + const runner = createRunner(__dirname) + .withServerUrl(ingest.url) + .withWranglerArgs('--var', 'SLOW_INGEST:true', '--var', `CACHE_CLIENT:${cacheClient}`) + .start(signal); + + const result = await runner.makeRequest('get', '/flush-with-timeout'); + expect(result).toEqual({ flushed: false, send: 'aborted' }); + }, +); + +// The local runtime does not settle a cached client's step drains while the run waits, so this runs with one +// client per invocation. The transport abort itself is covered for both modes by the test above. +it('cacheClient: false - the Workflow from #24482 aborts the pending send of a step when its flush times out', async ({ + signal, +}) => { + const ingest = await startSilentIngest(); + + const runner = createRunner(__dirname) + .withServerUrl(ingest.url) + .withWranglerArgs('--var', 'SLOW_INGEST:true', '--var', 'CACHE_CLIENT:false', '--var', 'TRACING:true') + .start(signal); + + await runner.makeRequest('get', '/workflow/trigger'); + expect(await ingest.result).toEqual({ send: 'aborted' }); +}); + +it('cacheClient: false - delivers events while a user waitUntil task is still running', async ({ signal }) => { + const runner = createRunner(__dirname) + .withWranglerArgs('--var', 'CACHE_CLIENT:false') + .expect((envelope: Envelope) => { + const event = envelope[1]?.[0]?.[1] as Event; + expect(event.exception?.values?.[0]?.value).toBe('Captured on /pending-wait-until'); + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/pending-wait-until'); + await runner.completed(); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/vite.config.mts b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/vite.config.mts new file mode 100644 index 000000000000..005f4448f6cb --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/vite.config.mts @@ -0,0 +1,7 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [cloudflare(), sentryCloudflareVitePlugin()], +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/flush-timeout/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/wrangler.jsonc new file mode 100644 index 000000000000..a74a1ff6059c --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/wrangler.jsonc @@ -0,0 +1,14 @@ +{ + "$schema": "../../node_modules/wrangler/config-schema.json", + "name": "cloudflare-flush-timeout", + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], + "workflows": [ + { + "name": "issue-workflow", + "binding": "ISSUE_WORKFLOW", + "class_name": "IssueWorkflow", + }, + ], +} diff --git a/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/index.ts b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/index.ts new file mode 100644 index 000000000000..4c955babf0fa --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/index.ts @@ -0,0 +1,28 @@ +import { WorkflowEntrypoint } from 'cloudflare:workers'; +import type { WorkflowEvent, WorkflowStep } from 'cloudflare:workers'; + +interface Env { + SENTRY_DSN: string; + SLEEP_WORKFLOW: Workflow; +} + +export class SleepWorkflow extends WorkflowEntrypoint { + async run(_event: WorkflowEvent, step: WorkflowStep): Promise { + await step.do('before-sleep', async () => 'done'); + await step.sleep('pause', '1 hour'); + await step.do('after-sleep', async () => 'done'); + } +} + +export default { + async fetch(request: Request, env: Env): Promise { + const url = new URL(request.url); + + if (url.pathname === '/workflow/trigger') { + const instance = await env.SLEEP_WORKFLOW.create(); + return Response.json({ id: instance.id }); + } + + return new Response('Not found', { status: 404 }); + }, +} satisfies ExportedHandler; diff --git a/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/instrument.server.ts b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/instrument.server.ts new file mode 100644 index 000000000000..e577550ae0b8 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/instrument.server.ts @@ -0,0 +1,6 @@ +import { defineCloudflareOptions } from '@sentry/cloudflare'; + +export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1.0, +})); diff --git a/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/test.ts b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/test.ts new file mode 100644 index 000000000000..780061d6b400 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/test.ts @@ -0,0 +1,27 @@ +import type { SerializedStreamedSpanContainer } from '@sentry/core'; +import { expect, it } from 'vitest'; +import { createRunner } from '../../../runner'; + +it('sends the span of a step before the Workflow goes to sleep', async ({ signal }) => { + const runner = createRunner(__dirname) + .expect(envelope => { + const container = envelope[1].find(item => item[0].type === 'span')?.[1] as SerializedStreamedSpanContainer; + + expect(container.items).toHaveLength(1); + expect(container.items[0]!.name).toBe('before-sleep'); + expect(envelope[0].trace).toEqual({ + environment: 'production', + public_key: 'public', + trace_id: container.items[0]!.trace_id, + transaction: 'before-sleep', + sampled: 'true', + sample_rand: expect.any(String), + sample_rate: '1', + }); + }) + .unordered() + .start(signal); + + await runner.makeRequest('get', '/workflow/trigger'); + await runner.completed(); +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/vite.config.mts b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/vite.config.mts new file mode 100644 index 000000000000..005f4448f6cb --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/vite.config.mts @@ -0,0 +1,7 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [cloudflare(), sentryCloudflareVitePlugin()], +}); diff --git a/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/wrangler.jsonc b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/wrangler.jsonc new file mode 100644 index 000000000000..bf2379b11195 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/wrangler.jsonc @@ -0,0 +1,14 @@ +{ + "$schema": "../../../node_modules/wrangler/config-schema.json", + "name": "cloudflare-workflow-step-flush", + "main": "index.ts", + "compatibility_date": "2025-06-17", + "compatibility_flags": ["nodejs_compat"], + "workflows": [ + { + "name": "sleep-workflow", + "binding": "SLEEP_WORKFLOW", + "class_name": "SleepWorkflow", + }, + ], +} diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/deployed-worker.mjs b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/deployed-worker.mjs index da689df28552..9dacbe052b8b 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/deployed-worker.mjs +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/deployed-worker.mjs @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process'; -import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -14,8 +14,24 @@ function wrangler(args, env = {}) { }); } +/** + * Workflow names are unique per Cloudflare account, so every worker gets its own. The Vite build writes the + * config wrangler deploys from, and `.wrangler/deploy/config.json` points to it. + */ +function nameWorkflowsAfterWorker(name) { + const redirect = JSON.parse(readFileSync(join(__dirname, '.wrangler/deploy/config.json'), 'utf8')); + const configPath = join(__dirname, '.wrangler/deploy', redirect.configPath); + const config = JSON.parse(readFileSync(configPath, 'utf8')); + + for (const workflow of config.workflows ?? []) { + workflow.name = name; + } + writeFileSync(configPath, JSON.stringify(config, null, 2)); +} + /** Deploys the worker under `name` and returns its workers.dev URL. */ export function deployWorker(name, dsn) { + nameWorkflowsAfterWorker(name); const outputDir = mkdtempSync(join(tmpdir(), 'wrangler-output-')); const outputFile = join(outputDir, 'output.ndjson'); diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/env.d.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/env.d.ts index eb80bafb4834..aaf72be7640b 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/env.d.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/env.d.ts @@ -1,3 +1,4 @@ interface Env { E2E_TEST_DSN: string; + SLEEP_WORKFLOW: Workflow; } diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/index.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/index.ts index c48a38137713..93f1c24f124e 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/index.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/src/index.ts @@ -1,7 +1,20 @@ import * as Sentry from '@sentry/cloudflare'; +import { WorkflowEntrypoint } from 'cloudflare:workers'; +import type { WorkflowEvent, WorkflowStep } from 'cloudflare:workers'; + +export class SleepWorkflow extends WorkflowEntrypoint { + async run(_event: WorkflowEvent, step: WorkflowStep): Promise { + for (let index = 0; index < 3; index++) { + await step.do(`before-sleep-${index}`, async () => index); + } + + await step.sleep('pause', '10 minutes'); + await step.do('after-sleep', async () => 'done'); + } +} export default { - async fetch(request) { + async fetch(request, env) { const url = new URL(request.url); // The handler runs inside the request span the Vite plugin's `withSentry` wrapper starts, so // this is the `http.server` span. @@ -16,6 +29,14 @@ export default { throw new Error('E2E test unhandled error'); case '/test-span': return Response.json({ spanId: spanContext?.spanId, traceId: spanContext?.traceId }); + case '/test-workflow-sleep': { + const instance = await env.SLEEP_WORKFLOW.create({ id: crypto.randomUUID() }); + return Response.json({ instanceId: instance.id, traceId: instance.id.replace(/-/g, '') }); + } + case '/test-workflow-status': { + const instance = await env.SLEEP_WORKFLOW.get(url.searchParams.get('id') ?? ''); + return Response.json(await instance.status()); + } default: return new Response('Hello World!'); } diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/tests/send-to-sentry.test.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/tests/send-to-sentry.test.ts index 49813f134b10..ab642bf55b1a 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/tests/send-to-sentry.test.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/tests/send-to-sentry.test.ts @@ -1,6 +1,13 @@ import { randomBytes } from 'node:crypto'; import { expect, test } from '@playwright/test'; -import { EVENT_POLLING_OPTIONS, findErrorInTrace, findSpanInTrace, traceTarget } from '@sentry-internal/test-utils/cli'; +import { + EVENT_POLLING_OPTIONS, + fetchTrace, + findErrorInTrace, + findSpanInTrace, + flattenTrace, + traceTarget, +} from '@sentry-internal/test-utils/cli'; // Set by global-setup.mjs once the worker for this run is deployed. const workerUrl = process.env.E2E_TEST_WORKER_URL; @@ -43,3 +50,24 @@ test('Sends a request span to Sentry', async () => { .poll(() => findSpanInTrace(traceId, 'http.server'), EVENT_POLLING_OPTIONS) .toMatchObject({ event_id: spanId }); }); + +test('Sends the spans of Workflow steps before the Workflow goes to sleep', async () => { + const response = await fetch(`${workerUrl}/test-workflow-sleep`); + expect(response.status).toBe(200); + const { instanceId, traceId } = await response.json(); + + console.log(`Polling for the Workflow step spans: sentry trace view ${traceTarget(traceId)}`); + + await expect + .poll( + () => + flattenTrace(fetchTrace(traceId)).filter( + item => item.event_type === 'span' && item.op === 'function' && item.description?.startsWith('before-sleep-'), + ).length, + EVENT_POLLING_OPTIONS, + ) + .toBe(3); + + const { status } = await fetch(`${workerUrl}/test-workflow-status?id=${instanceId}`).then(res => res.json()); + expect(['running', 'waiting']).toContain(status); +}); diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/wrangler.jsonc b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/wrangler.jsonc index cf5ad9bee22b..351a68bd023f 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/wrangler.jsonc +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers-send-to-sentry/wrangler.jsonc @@ -8,4 +8,8 @@ "workers_dev": true, // Workers Logs keep the invocations of the last 7 days, so a failed CI run can still be inspected. "observability": { "enabled": true }, + // Workflow names are unique per account, so deployWorker() renames it to the worker name. + "workflows": [ + { "name": "cloudflare-workers-send-to-sentry", "binding": "SLEEP_WORKFLOW", "class_name": "SleepWorkflow" }, + ], } diff --git a/dev-packages/test-utils/src/cli.ts b/dev-packages/test-utils/src/cli.ts index b35e37bed6c9..d45c80232239 100644 --- a/dev-packages/test-utils/src/cli.ts +++ b/dev-packages/test-utils/src/cli.ts @@ -15,6 +15,8 @@ export interface TraceItem { event_id?: string; event_type?: 'span' | 'error' | 'occurrence' | 'uptime_check'; op?: string | null; + /** On a span this is the span name. */ + description?: string | null; children?: TraceItem[] | null; errors?: TraceItem[] | null; occurrences?: TraceItem[] | null; diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index a71445b03f92..1a968cfe66b2 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -107,19 +107,30 @@ export class CloudflareClient extends ServerRuntimeClient { /** * Flushes pending operations and ensures all data is processed. - * If a timeout is provided, the operation will be completed within the specified time limit. * - * It will wait for all pending spans to complete before flushing. + * Each phase waits at most `timeout`: the flush lock of a per-invocation client, pending spans, event + * processing and the transport drain. So a flush can take a small multiple of `timeout`, which stays well + * below Cloudflare's 30 second `waitUntil` limit for the timeouts the SDK uses. Sends still pending when + * the drain times out are aborted. * - * @param {number} [timeout] - Optional timeout in milliseconds to force the completion of the flush operation. + * @param {number} [timeout] - Maximum time in milliseconds for each phase of the flush. * @return {Promise} A promise that resolves to a boolean indicating whether the flush operation was successful. */ public async flush(timeout?: number): Promise { // Wait for user waitUntil-registered work to settle before draining, so events // captured in that work are still in the buffer. Without this the final flush // can drain (and the client be disposed) before background captures land. + // + // Only per-invocation clients (`cacheClient: false`) have a flush lock; remove this with them in v12. + // The wait is bounded by `timeout` because a user `waitUntil` task that outlives the invocation + // would otherwise keep the flush from draining until the runtime cancels it. if (this._flushLock) { - await this._flushLock.finalize(); + let timer: ReturnType | undefined; + await Promise.race([ + this._flushLock.finalize(), + ...(timeout ? [new Promise(resolve => (timer = setTimeout(resolve, timeout)))] : []), + ]); + clearTimeout(timer); } if (this._pendingSpans.size > 0 && this._spanCompletionPromise) { diff --git a/packages/cloudflare/src/transport.ts b/packages/cloudflare/src/transport.ts index 25d9e05572b9..b7f5a3181680 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -29,6 +29,12 @@ export class IsolatedPromiseBuffer { // If we ever remove it from the interface we should also remove it here. public $: Array>; + /** + * Abort signal of the drain that is starting its requests. It is set only while `drain()` runs the task + * producers, so a request reads the signal of the drain that sends it. + */ + public drainSignal: AbortSignal | undefined; + private _taskProducers: (() => PromiseLike)[]; private readonly _bufferSize: number; @@ -58,9 +64,21 @@ export class IsolatedPromiseBuffer { const oldTaskProducers = [...this._taskProducers]; this._taskProducers = []; + const drainController = new AbortController(); + this.drainSignal = drainController.signal; + let tasks: PromiseLike[]; + try { + tasks = oldTaskProducers.map(taskProducer => taskProducer()); + } finally { + this.drainSignal = undefined; + } + return new Promise(resolve => { const timer = setTimeout(() => { if (timeout && timeout > 0) { + // Requests still pending when the drain times out are aborted. Otherwise Cloudflare keeps them + // until it cancels the invocation's `waitUntil` work and logs a warning. + drainController.abort(); resolve(false); } }, timeout); @@ -68,8 +86,8 @@ export class IsolatedPromiseBuffer { // This cannot reject // eslint-disable-next-line @typescript-eslint/no-floating-promises Promise.all( - oldTaskProducers.map(taskProducer => - taskProducer().then(null, () => { + tasks.map(task => + task.then(null, () => { // catch all failed requests }), ), @@ -86,12 +104,20 @@ export class IsolatedPromiseBuffer { * Creates a Transport that uses the native fetch API to send events to Sentry. */ export function makeCloudflareTransport(options: CloudflareTransportOptions): Transport { + const buffer = new IsolatedPromiseBuffer(options.bufferSize); + function makeRequest(request: TransportRequest): PromiseLike { + const drainSignal = buffer.drainSignal; + const callerSignal = options.fetchOptions?.signal ?? undefined; + const signal = + drainSignal && callerSignal ? AbortSignal.any([drainSignal, callerSignal]) : (drainSignal ?? callerSignal); + const requestOptions: RequestInit = { body: request.body as BodyInit, method: 'POST', headers: options.headers, ...options.fetchOptions, + ...(signal ? { signal } : {}), }; return suppressTracing(() => { @@ -118,5 +144,5 @@ export function makeCloudflareTransport(options: CloudflareTransportOptions): Tr }); } - return createTransport(options, makeRequest, new IsolatedPromiseBuffer(options.bufferSize)); + return createTransport(options, makeRequest, buffer); } diff --git a/packages/cloudflare/src/workflows.ts b/packages/cloudflare/src/workflows.ts index c46470c54355..62442cadf32d 100644 --- a/packages/cloudflare/src/workflows.ts +++ b/packages/cloudflare/src/workflows.ts @@ -31,6 +31,7 @@ import { addCloudResourceContext } from './scope-utils'; import { init } from './sdk'; import { instrumentContext } from './utils/instrumentContext'; import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types'; +import { getInvocationState } from './utils/invocationContext'; import { withInvocationIsolationScope } from './utils/invocationScope'; const UUID_REGEX = /^[0-9a-f]{8}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{12}$/i; @@ -124,6 +125,12 @@ class WrappedWorkflowStep implements WorkflowStep { // run's isolation scope (and with it the invocation state that ties eager sends // to this invocation's `waitUntil`) has to be restored explicitly. return withIsolationScope(this._isolationScope, () => { + const invocationState = getInvocationState(); + + if (invocationState) { + invocationState.flushPointReached = false; + } + const stepResult = startSpan( { name, @@ -157,14 +164,16 @@ class WrappedWorkflowStep implements WorkflowStep { }, ); // Deliver after the step span has ended, so the span rides this flush instead of - // starting an eager drain (same ordering as `wrapMethodWithSentry`'s teardown). + // starting an eager drain (same ordering as `wrapMethodWithSentry`'s teardown). The flush runs on the + // step's scope: the engine calls the step callback outside of `run()`, so the current scope here has + // neither the client nor the run's trace, and envelopes without a DSC are dropped by Relay. return stepResult.then( result => { - this._waitUntil(flush(2000)); + this._waitUntil(withScope(scopeForStep, () => flush(2000))); return result; }, error => { - this._waitUntil(flush(2000)); + this._waitUntil(withScope(scopeForStep, () => flush(2000))); throw error; }, ); diff --git a/packages/cloudflare/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 09bff574e479..2d7828be238d 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -268,6 +268,31 @@ describe('CloudflareClient', () => { await flushPromise; expect(privateClient._transport.flush).toHaveBeenCalledWith(1000); }); + + it('drains the transport when the flush lock does not settle within the timeout', async () => { + vi.useFakeTimers(); + try { + const client = new CloudflareClient({ + ...MOCK_CLIENT_OPTIONS, + flushLock: { ready: Promise.resolve(), finalize: () => new Promise(() => undefined) }, + }); + + const privateClient = client as unknown as { + _transport: { flush: ReturnType }; + }; + + void client.flush(1000); + + await vi.advanceTimersByTimeAsync(999); + expect(privateClient._transport.flush).not.toHaveBeenCalled(); + + // The lock wait ends at 1000 ms; the client processing check after it also runs on timers. + await vi.advanceTimersByTimeAsync(100); + expect(privateClient._transport.flush).toHaveBeenCalledWith(1000); + } finally { + vi.useRealTimers(); + } + }); }); describe('span lifecycle tracking', () => { diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index 052147f84bc4..baf9282f9e15 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -1051,6 +1051,7 @@ describe('Durable Object (DO) context', () => { // Teardown is registered via waitUntil on error too expect(waitUntilSpy).toHaveBeenCalled(); + await Promise.all(waitUntilSpy.mock.calls.map(([promise]) => promise)); // And flush runs as part of that teardown expect(flushSpy).toHaveBeenCalled(); @@ -1072,6 +1073,7 @@ describe('Durable Object (DO) context', () => { ); expect(waitUntilSpy).toHaveBeenCalled(); + await Promise.all(waitUntilSpy.mock.calls.map(([promise]) => promise)); expect(flushSpy).toHaveBeenCalled(); flushSpy.mockRestore(); @@ -1092,6 +1094,7 @@ describe('Durable Object (DO) context', () => { ); expect(waitUntilSpy).toHaveBeenCalled(); + await Promise.all(waitUntilSpy.mock.calls.map(([promise]) => promise)); expect(flushSpy).toHaveBeenCalled(); flushSpy.mockRestore(); diff --git a/packages/cloudflare/test/transport.test.ts b/packages/cloudflare/test/transport.test.ts index fdb9fbc5e30f..1750176f5591 100644 --- a/packages/cloudflare/test/transport.test.ts +++ b/packages/cloudflare/test/transport.test.ts @@ -52,6 +52,7 @@ describe('Edge Transport', () => { expect(mockFetch).toHaveBeenLastCalledWith(DEFAULT_EDGE_TRANSPORT_OPTIONS.url, { body: serializeEnvelope(ERROR_ENVELOPE), method: 'POST', + signal: expect.any(AbortSignal), }); }); @@ -104,6 +105,7 @@ describe('Edge Transport', () => { body: serializeEnvelope(ERROR_ENVELOPE), method: 'POST', ...REQUEST_OPTIONS, + signal: expect.any(AbortSignal), }); }); @@ -249,4 +251,107 @@ describe('IsolatedPromiseBuffer', () => { await transport.flush(); expect(customFetch).toHaveBeenCalledTimes(1); }); + + it('aborts a request that is still pending when its drain times out', async () => { + vi.useFakeTimers(); + try { + let signal: AbortSignal | undefined; + const customFetch = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit): Promise => + new Promise((_resolve, reject) => { + signal = init?.signal ?? undefined; + signal?.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError')), { once: true }); + }), + ); + const transport = makeCloudflareTransport({ ...DEFAULT_EDGE_TRANSPORT_OPTIONS, fetch: customFetch }); + + await transport.send(ERROR_ENVELOPE); + const flush = transport.flush(1000); + + await vi.advanceTimersByTimeAsync(999); + expect(signal?.aborted).toBe(false); + + await vi.advanceTimersByTimeAsync(1); + await expect(flush).resolves.toBe(false); + expect(signal?.aborted).toBe(true); + } finally { + vi.useRealTimers(); + } + }); + + it('does not abort requests of a drain without a timeout', async () => { + vi.useFakeTimers(); + try { + let signal: AbortSignal | undefined; + const customFetch = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit): Promise => + new Promise(() => { + signal = init?.signal ?? undefined; + }), + ); + const transport = makeCloudflareTransport({ ...DEFAULT_EDGE_TRANSPORT_OPTIONS, fetch: customFetch }); + + await transport.send(ERROR_ENVELOPE); + void transport.flush(); + + await vi.advanceTimersByTimeAsync(60_000); + expect(signal?.aborted).toBe(false); + } finally { + vi.useRealTimers(); + } + }); + + it('preserves a caller-provided abort signal', async () => { + let signal: AbortSignal | undefined; + const callerController = new AbortController(); + const customFetch = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit): Promise => + new Promise((_resolve, reject) => { + signal = init?.signal ?? undefined; + signal?.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError')), { once: true }); + }), + ); + const transport = makeCloudflareTransport({ + ...DEFAULT_EDGE_TRANSPORT_OPTIONS, + fetch: customFetch, + fetchOptions: { signal: callerController.signal }, + }); + + await transport.send(ERROR_ENVELOPE); + const flush = transport.flush(); + callerController.abort(); + + await expect(flush).resolves.toBe(true); + expect(signal?.aborted).toBe(true); + }); + + it('does not abort requests belonging to another drain', async () => { + vi.useFakeTimers(); + try { + const signals: AbortSignal[] = []; + const customFetch = vi.fn( + (_input: RequestInfo | URL, init?: RequestInit): Promise => + new Promise((_resolve, reject) => { + const signal = init?.signal as AbortSignal; + signals.push(signal); + signal.addEventListener('abort', () => reject(new DOMException('Aborted', 'AbortError')), { once: true }); + }), + ); + const transport = makeCloudflareTransport({ ...DEFAULT_EDGE_TRANSPORT_OPTIONS, fetch: customFetch }); + + await transport.send(ERROR_ENVELOPE); + void transport.flush(2000); + await transport.send(ERROR_ENVELOPE); + void transport.flush(1000); + + await vi.advanceTimersByTimeAsync(1000); + expect(signals[0]?.aborted).toBe(false); + expect(signals[1]?.aborted).toBe(true); + + await vi.advanceTimersByTimeAsync(1000); + expect(signals[0]?.aborted).toBe(true); + } finally { + vi.useRealTimers(); + } + }); }); diff --git a/packages/cloudflare/test/workflow.test.ts b/packages/cloudflare/test/workflow.test.ts index f2ddfae9d8f0..2dc5e1ceb02d 100644 --- a/packages/cloudflare/test/workflow.test.ts +++ b/packages/cloudflare/test/workflow.test.ts @@ -458,9 +458,8 @@ describe.skipIf(NODE_MAJOR_VERSION < 20)('workflows', () => { expect(mockStep.do).toHaveBeenCalledTimes(1); expect(mockStep.do).toHaveBeenCalledWith('sometimes error step', expect.any(Function)); - // One flush per attempt (failed and retried, past the span end) and one at end of - // run, plus one eager registration for the envelope of the error captured mid-run - expect(mockContext.waitUntil).toHaveBeenCalledTimes(4); + // One flush per attempt (failed and retried) and one at the end of the run + expect(mockContext.waitUntil).toHaveBeenCalledTimes(3); expect(mockContext.waitUntil).toHaveBeenCalledWith(expect.any(Promise)); // No error event (not final attempt), only failed transaction + successful retry transaction expect(mockTransport.send).toHaveBeenCalledTimes(2);