diff --git a/.size-limit.js b/.size-limit.js index 3786d07b38ac..c2e30b48f72f 100644 --- a/.size-limit.js +++ b/.size-limit.js @@ -480,7 +480,7 @@ module.exports = [ ignore: [...builtinModules, ...nodePrefixedBuiltinModules], gzip: false, brotli: false, - limit: '492 KiB', + limit: '495 KiB', disablePlugins: ['@size-limit/webpack'], webpack: false, modifyEsbuildConfig: function (config) { 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..2d601466a16e --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/instrument.server.ts @@ -0,0 +1,25 @@ +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; + // "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, + 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..bcb866be5977 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/test.ts @@ -0,0 +1,68 @@ +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('aborts a send that is still pending when the flush times out', async ({ signal }) => { + const ingest = await startSilentIngest(); + + const runner = createRunner(__dirname) + .withServerUrl(ingest.url) + .withWranglerArgs('--var', 'SLOW_INGEST:true') + .start(signal); + + const result = await runner.makeRequest('get', '/flush-with-timeout'); + expect(result).toEqual({ flushed: false, send: 'aborted' }); +}); + +it('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', 'TRACING:true') + .start(signal); + + await runner.makeRequest('get', '/workflow/trigger'); + expect(await ingest.result).toEqual({ send: 'aborted' }); +}); + +it('delivers events while a user waitUntil task is still running', async ({ signal }) => { + const runner = createRunner(__dirname) + .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..ca0836767810 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/flush-timeout/vite.config.mts @@ -0,0 +1,14 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [ + cloudflare(), + sentryCloudflareVitePlugin({ + _experimental: { + autoInstrumentation: true, + }, + }), + ], +}); 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..cf59e94f26ba --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/instrument.server.ts @@ -0,0 +1,7 @@ +import { defineCloudflareOptions } from '@sentry/cloudflare'; + +export default defineCloudflareOptions((env: { SENTRY_DSN: string }) => ({ + dsn: env.SENTRY_DSN, + tracesSampleRate: 1.0, + traceLifecycle: 'stream', +})); 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..ca0836767810 --- /dev/null +++ b/dev-packages/cloudflare-integration-tests/suites/workflows/step-flush/vite.config.mts @@ -0,0 +1,14 @@ +import { cloudflare } from '@cloudflare/vite-plugin'; +import { sentryCloudflareVitePlugin } from '@sentry/cloudflare/vite'; +import { defineConfig } from 'vite'; + +export default defineConfig({ + plugins: [ + cloudflare(), + sentryCloudflareVitePlugin({ + _experimental: { + autoInstrumentation: true, + }, + }), + ], +}); 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/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index ce42b8416a93..bc13e1d470bf 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -85,16 +85,25 @@ 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 { + // 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 8e0e82aae7e0..918f80eff2eb 100644 --- a/packages/cloudflare/src/transport.ts +++ b/packages/cloudflare/src/transport.ts @@ -23,6 +23,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; @@ -52,9 +58,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); @@ -62,8 +80,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 }), ), @@ -80,12 +98,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, method: 'POST', headers: options.headers, ...options.fetchOptions, + ...(signal ? { signal } : {}), }; return suppressTracing(() => { @@ -112,5 +138,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/test/client.test.ts b/packages/cloudflare/test/client.test.ts index 8a6ac65175a0..b6aa6573cf40 100644 --- a/packages/cloudflare/test/client.test.ts +++ b/packages/cloudflare/test/client.test.ts @@ -220,6 +220,33 @@ describe('CloudflareClient', () => { }); }); + describe('flush()', () => { + 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', () => { it('tracks pending spans when spanStart is emitted', () => { const client = new CloudflareClient(MOCK_CLIENT_OPTIONS); 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(); + } + }); });