diff --git a/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/index.test.ts b/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/index.test.ts index df8e655416fb..ad63c1a0d307 100644 --- a/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/index.test.ts +++ b/dev-packages/e2e-tests/test-applications/cloudflare-workers/tests/index.test.ts @@ -40,7 +40,7 @@ test("Request processed by DurableObject's fetch is recorded", async ({ baseURL test('Websocket.webSocketMessage', async ({ baseURL }) => { const eventWaiter = waitForError('cloudflare-workers', event => { - return event.exception?.values?.[0]?.mechanism?.type === 'auto.faas.cloudflare.durable_object'; + return !!event.exception?.values?.[0]; }); const url = new URL('/pass-to-object/ws', baseURL); url.protocol = url.protocol.replace('http', 'ws'); @@ -51,11 +51,12 @@ test('Websocket.webSocketMessage', async ({ baseURL }) => { const event = await eventWaiter; socket.close(); expect(event.exception?.values?.[0]?.value).toBe('Should be recorded in Sentry: webSocketMessage'); + expect(event.exception?.values?.[0]?.mechanism?.type).toBe('auto.faas.cloudflare.durable_object'); }); test('Websocket.webSocketClose', async ({ baseURL }) => { const eventWaiter = waitForError('cloudflare-workers', event => { - return event.exception?.values?.[0]?.mechanism?.type === 'auto.faas.cloudflare.durable_object'; + return !!event.exception?.values?.[0]; }); const url = new URL('/pass-to-object/ws', baseURL); url.protocol = url.protocol.replace('http', 'ws'); @@ -66,4 +67,5 @@ test('Websocket.webSocketClose', async ({ baseURL }) => { }); const event = await eventWaiter; expect(event.exception?.values?.[0]?.value).toBe('Should be recorded in Sentry: webSocketClose'); + expect(event.exception?.values?.[0]?.mechanism?.type).toBe('auto.faas.cloudflare.durable_object'); }); diff --git a/packages/cloudflare/src/client.ts b/packages/cloudflare/src/client.ts index 6da36cc1721c..2de5147d3d5a 100644 --- a/packages/cloudflare/src/client.ts +++ b/packages/cloudflare/src/client.ts @@ -1,7 +1,7 @@ import type { ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core'; import { applySdkMetadata, ServerRuntimeClient } from '@sentry/core'; +import type { makeFlushLock } from './flush'; import type { CloudflareTransportOptions } from './transport'; -import type { makeFlushLock } from './utils/flushLock'; /** * The Sentry Cloudflare SDK Client. diff --git a/packages/cloudflare/src/durableobject.ts b/packages/cloudflare/src/durableobject.ts index 7fbb0592970a..0f139a80ccd0 100644 --- a/packages/cloudflare/src/durableobject.ts +++ b/packages/cloudflare/src/durableobject.ts @@ -18,7 +18,6 @@ import { isInstrumented, markAsInstrumented } from './instrument'; import { getFinalOptions } from './options'; import { wrapRequestHandler } from './request'; import { init } from './sdk'; -import { copyExecutionContext } from './utils/copyExecutionContext'; type MethodWrapperOptions = { spanName?: string; @@ -193,11 +192,9 @@ export function instrumentDurableObjectWithSentry< C extends new (state: DurableObjectState, env: E) => T, >(optionsCallback: (env: E) => CloudflareOptions, DurableObjectClass: C): C { return new Proxy(DurableObjectClass, { - construct(target, [ctx, env]) { + construct(target, [context, env]) { setAsyncLocalStorageAsyncContextStrategy(); - const context = copyExecutionContext(ctx); - const options = getFinalOptions(optionsCallback(env), env); const obj = new target(context, env); diff --git a/packages/cloudflare/src/flush.ts b/packages/cloudflare/src/flush.ts new file mode 100644 index 000000000000..f38c805d0f8b --- /dev/null +++ b/packages/cloudflare/src/flush.ts @@ -0,0 +1,38 @@ +import type { ExecutionContext } from '@cloudflare/workers-types'; + +type FlushLock = { + readonly ready: Promise; + readonly finalize: () => Promise; +}; + +/** + * Enhances the given execution context by wrapping its `waitUntil` method with a proxy + * to monitor pending tasks, and provides a flusher function to ensure all tasks + * have been completed before executing any subsequent logic. + * + * @param {ExecutionContext} context - The execution context to be enhanced. If no context is provided, the function returns undefined. + * @return {FlushLock} Returns a flusher function if a valid context is provided, otherwise undefined. + */ +export function makeFlushLock(context: ExecutionContext): FlushLock { + let resolveAllDone: () => void = () => undefined; + const allDone = new Promise(res => { + resolveAllDone = res; + }); + let pending = 0; + const originalWaitUntil = context.waitUntil.bind(context) as typeof context.waitUntil; + context.waitUntil = promise => { + pending++; + return originalWaitUntil( + promise.finally(() => { + if (--pending === 0) resolveAllDone(); + }), + ); + }; + return Object.freeze({ + ready: allDone, + finalize: () => { + if (pending === 0) resolveAllDone(); + return allDone; + }, + }); +} diff --git a/packages/cloudflare/src/handler.ts b/packages/cloudflare/src/handler.ts index 0275e01e1777..a6e5983902c6 100644 --- a/packages/cloudflare/src/handler.ts +++ b/packages/cloudflare/src/handler.ts @@ -14,7 +14,6 @@ import { getFinalOptions } from './options'; import { wrapRequestHandler } from './request'; import { addCloudResourceContext } from './scope-utils'; import { init } from './sdk'; -import { copyExecutionContext } from './utils/copyExecutionContext'; /** * Wrapper for Cloudflare handlers. @@ -38,11 +37,9 @@ export function withSentry>) { - const [request, env, ctx] = args; + const [request, env, context] = args; const options = getFinalOptions(optionsCallback(env), env); - const context = copyExecutionContext(ctx); - args[2] = context; return wrapRequestHandler({ options, request, context }, () => target.apply(thisArg, args)); }, @@ -74,9 +71,7 @@ export function withSentry>) { - const [event, env, ctx] = args; - const context = copyExecutionContext(ctx); - args[2] = context; + const [event, env, context] = args; return withIsolationScope(isolationScope => { const options = getFinalOptions(optionsCallback(env), env); const waitUntil = context.waitUntil.bind(context); @@ -119,9 +114,7 @@ export function withSentry>) { - const [emailMessage, env, ctx] = args; - const context = copyExecutionContext(ctx); - args[2] = context; + const [emailMessage, env, context] = args; return withIsolationScope(isolationScope => { const options = getFinalOptions(optionsCallback(env), env); const waitUntil = context.waitUntil.bind(context); @@ -162,9 +155,7 @@ export function withSentry>) { - const [batch, env, ctx] = args; - const context = copyExecutionContext(ctx); - args[2] = context; + const [batch, env, context] = args; return withIsolationScope(isolationScope => { const options = getFinalOptions(optionsCallback(env), env); @@ -214,9 +205,7 @@ export function withSentry>) { - const [, env, ctx] = args; - const context = copyExecutionContext(ctx); - args[2] = context; + const [, env, context] = args; return withIsolationScope(async isolationScope => { const options = getFinalOptions(optionsCallback(env), env); diff --git a/packages/cloudflare/src/sdk.ts b/packages/cloudflare/src/sdk.ts index 416ae47f312f..9d4fb8d749ae 100644 --- a/packages/cloudflare/src/sdk.ts +++ b/packages/cloudflare/src/sdk.ts @@ -12,10 +12,10 @@ import { } from '@sentry/core'; import type { CloudflareClientOptions, CloudflareOptions } from './client'; import { CloudflareClient } from './client'; +import { makeFlushLock } from './flush'; import { fetchIntegration } from './integrations/fetch'; import { setupOpenTelemetryTracer } from './opentelemetry/tracer'; import { makeCloudflareTransport } from './transport'; -import { makeFlushLock } from './utils/flushLock'; import { defaultStackParser } from './vendor/stacktrace'; /** Get the default integrations for the Cloudflare SDK. */ diff --git a/packages/cloudflare/src/utils/copyExecutionContext.ts b/packages/cloudflare/src/utils/copyExecutionContext.ts deleted file mode 100644 index 737a622d7a1c..000000000000 --- a/packages/cloudflare/src/utils/copyExecutionContext.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { type DurableObjectState, type ExecutionContext } from '@cloudflare/workers-types'; - -const kBound = Symbol.for('kBound'); - -const defaultPropertyOptions: PropertyDescriptor = { - enumerable: true, - configurable: true, - writable: true, -}; - -/** - * Clones the given execution context by creating a shallow copy while ensuring the binding of specific methods. - * - * @param {ExecutionContext|DurableObjectState|void} ctx - The execution context to clone. Can be void. - * @return {ExecutionContext|DurableObjectState|void} A cloned execution context with bound methods, or the original void value if no context was provided. - */ -export function copyExecutionContext(ctx: T): T { - if (!ctx) return ctx; - return Object.create(ctx, { - waitUntil: { ...defaultPropertyOptions, value: copyAndBindMethod(ctx, 'waitUntil') }, - ...('passThroughOnException' in ctx && { - passThroughOnException: { ...defaultPropertyOptions, value: copyAndBindMethod(ctx, 'passThroughOnException') }, - }), - }); -} - -/** - * Copies a method from the given object and ensures the copied method remains bound to the original object's context. - * - * @param {object} obj - The object containing the method to be copied and bound. - * @param {string|symbol} method - The key of the method within the object to be copied and bound. - * @return {Function} - The copied and bound method, or the original property if it is not a function. - */ -function copyAndBindMethod(obj: T, method: K): T[K] { - const methodImpl = obj[method]; - if (typeof methodImpl !== 'function') return methodImpl; - if ((methodImpl as T[K] & { [kBound]?: boolean })[kBound]) return methodImpl; - const bound = methodImpl.bind(obj); - - return new Proxy(bound, { - get: (target, prop, receiver) => { - if (kBound === prop) return true; - if ('bind' === prop) return () => receiver; - return Reflect.get(target, prop, receiver); - }, - }); -} diff --git a/packages/cloudflare/src/utils/flushLock.ts b/packages/cloudflare/src/utils/flushLock.ts deleted file mode 100644 index dbb8d3666749..000000000000 --- a/packages/cloudflare/src/utils/flushLock.ts +++ /dev/null @@ -1,57 +0,0 @@ -import type { ExecutionContext } from '@cloudflare/workers-types'; -import { createPromiseResolver } from './makePromiseResolver'; - -type FlushLock = { - readonly ready: Promise; - readonly finalize: () => Promise; -}; -type MaybeLockable = T & { [kFlushLock]?: FlushLock }; - -const kFlushLock = Symbol.for('kFlushLock'); - -function getInstrumentedLock(o: MaybeLockable): FlushLock | undefined { - return o[kFlushLock]; -} - -function storeInstrumentedLock(o: MaybeLockable, lock: FlushLock): void { - o[kFlushLock] = lock; -} - -/** - * Enhances the given execution context by wrapping its `waitUntil` method with a proxy - * to monitor pending tasks and provides a flusher function to ensure all tasks - * have been completed before executing any subsequent logic. - * - * @param {ExecutionContext} context - The execution context to be enhanced. If no context is provided, the function returns undefined. - * @return {FlushLock} Returns a flusher function if a valid context is provided, otherwise undefined. - */ -export function makeFlushLock(context: ExecutionContext): FlushLock { - // eslint-disable-next-line @typescript-eslint/unbound-method - let lock = getInstrumentedLock(context.waitUntil); - if (lock) { - // It is fine to return the same lock multiple times because this means the context has already been instrumented. - return lock; - } - let pending = 0; - const originalWaitUntil = context.waitUntil.bind(context) as typeof context.waitUntil; - const { promise, resolve } = createPromiseResolver(); - const hijackedWaitUntil: typeof originalWaitUntil = promise => { - pending++; - return originalWaitUntil( - promise.finally(() => { - if (--pending === 0) resolve(); - }), - ); - }; - lock = Object.freeze({ - ready: promise, - finalize: () => { - if (pending === 0) resolve(); - return promise; - }, - }) as FlushLock; - storeInstrumentedLock(hijackedWaitUntil, lock); - context.waitUntil = hijackedWaitUntil; - - return lock; -} diff --git a/packages/cloudflare/src/utils/makePromiseResolver.ts b/packages/cloudflare/src/utils/makePromiseResolver.ts deleted file mode 100644 index f772951406b1..000000000000 --- a/packages/cloudflare/src/utils/makePromiseResolver.ts +++ /dev/null @@ -1,26 +0,0 @@ -type PromiseWithResolvers = { - readonly promise: Promise; - readonly resolve: (value?: T | PromiseLike) => void; - readonly reject: (reason?: E) => void; -}; -/** - * Creates an object containing a promise, along with its corresponding resolve and reject functions. - * - * This method provides a convenient way to create a promise and access its resolvers externally. - * - * @template T - The type of the resolved value of the promise. - * @template E - The type of the rejected value of the promise. Defaults to `unknown`. - * @return {PromiseWithResolvers} An object containing the promise and its resolve and reject functions. - */ -export function createPromiseResolver(): PromiseWithResolvers { - if ('withResolvers' in Promise && typeof Promise.withResolvers === 'function') { - return Promise.withResolvers(); - } - let resolve; - let reject; - const promise = new Promise((res, rej) => { - resolve = res; - reject = rej; - }); - return { promise, resolve, reject } as unknown as PromiseWithResolvers; -} diff --git a/packages/cloudflare/test/copy-execution-context.test.ts b/packages/cloudflare/test/copy-execution-context.test.ts deleted file mode 100644 index 61757cf1f355..000000000000 --- a/packages/cloudflare/test/copy-execution-context.test.ts +++ /dev/null @@ -1,47 +0,0 @@ -import { type ExecutionContext } from '@cloudflare/workers-types'; -import { type Mocked, describe, expect, it, vi } from 'vitest'; -import { copyExecutionContext } from '../src/utils/copyExecutionContext'; - -describe('Copy of the execution context', () => { - describe.for(['waitUntil', 'passThroughOnException'])('%s', method => { - it('Was not bound more than once', async () => { - const context = makeExecutionContextMock(); - const copy = copyExecutionContext(context); - const copy_of_copy = copyExecutionContext(copy); - - expect(copy[method]).toBe(copy_of_copy[method]); - }); - it('Copied method is bound to the original', async () => { - const context = makeExecutionContextMock(); - const copy = copyExecutionContext(context); - - expect(copy[method]()).toBe(context); - }); - it('Copied method "rebind" prevention', async () => { - const context = makeExecutionContextMock(); - const copy = copyExecutionContext(context); - expect(copy[method].bind('test')).toBe(copy[method]); - }); - }); - - it('No side effects', async () => { - const context = makeExecutionContextMock(); - expect(() => copyExecutionContext(Object.freeze(context))).not.toThrow( - /Cannot define property \w+, object is not extensible/, - ); - }); - it('Respects symbols', async () => { - const s = Symbol('test'); - const context = makeExecutionContextMock(); - context[s] = {}; - const copy = copyExecutionContext(context); - expect(copy[s]).toBe(context[s]); - }); -}); - -function makeExecutionContextMock() { - return { - waitUntil: vi.fn().mockReturnThis(), - passThroughOnException: vi.fn().mockReturnThis(), - } as unknown as Mocked; -} diff --git a/packages/cloudflare/test/durableobject.test.ts b/packages/cloudflare/test/durableobject.test.ts index 4c4d5c63bfd3..ce794dc7fb69 100644 --- a/packages/cloudflare/test/durableobject.test.ts +++ b/packages/cloudflare/test/durableobject.test.ts @@ -1,9 +1,8 @@ import type { ExecutionContext } from '@cloudflare/workers-types'; import * as SentryCore from '@sentry/core'; -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest'; import { instrumentDurableObjectWithSentry } from '../src'; import { isInstrumented } from '../src/instrument'; -import { createPromiseResolver } from '../src/utils/makePromiseResolver'; describe('instrumentDurableObjectWithSentry', () => { afterEach(() => { @@ -123,13 +122,15 @@ describe('instrumentDurableObjectWithSentry', () => { }); it('flush performs after all waitUntil promises are finished', async () => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); const waitUntil = vi.fn(); - const { promise, resolve } = createPromiseResolver(); - process.nextTick(resolve); const testClass = vi.fn(context => ({ fetch: () => { - context.waitUntil(promise); + context.waitUntil(new Promise(res => setTimeout(res))); return new Response('test'); }, })); @@ -141,7 +142,8 @@ describe('instrumentDurableObjectWithSentry', () => { expect(() => dObject.fetch(new Request('https://example.com'))).not.toThrow(); expect(flush).not.toBeCalled(); expect(waitUntil).toHaveBeenCalledOnce(); - await Promise.all(waitUntil.mock.calls.map(call => call[0])); + vi.advanceTimersToNextTimer(); + await Promise.all(waitUntil.mock.calls.map(([p]) => p)); expect(flush).toBeCalled(); }); diff --git a/packages/cloudflare/test/flush-lock.test.ts b/packages/cloudflare/test/flush-lock.test.ts deleted file mode 100644 index cc40fd3a7e03..000000000000 --- a/packages/cloudflare/test/flush-lock.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { type ExecutionContext } from '@cloudflare/workers-types'; -import { describe, expect, it, vi } from 'vitest'; -import { makeFlushLock } from '../src/utils/flushLock'; -import { createPromiseResolver } from '../src/utils/makePromiseResolver'; - -describe('Flush buffer test', () => { - const mockExecutionContext: ExecutionContext = { - waitUntil: vi.fn(), - passThroughOnException: vi.fn(), - props: null, - }; - it('should flush buffer immediately if no waitUntil were called', async () => { - const { finalize } = makeFlushLock(mockExecutionContext); - await expect(finalize()).resolves.toBeUndefined(); - }); - it('waitUntil should not be wrapped mose than once', () => { - expect(makeFlushLock(mockExecutionContext), 'Execution context wrapped twice').toBe( - makeFlushLock(mockExecutionContext), - ); - }); - it('should flush buffer only after all waitUntil were finished', async () => { - const { promise, resolve } = createPromiseResolver(); - const lock = makeFlushLock(mockExecutionContext); - mockExecutionContext.waitUntil(promise); - process.nextTick(resolve); - await expect(lock.finalize()).resolves.toBeUndefined(); - }); -}); diff --git a/packages/cloudflare/test/flush.test.ts b/packages/cloudflare/test/flush.test.ts new file mode 100644 index 000000000000..34714711c682 --- /dev/null +++ b/packages/cloudflare/test/flush.test.ts @@ -0,0 +1,30 @@ +import { type ExecutionContext } from '@cloudflare/workers-types'; +import { describe, expect, it, onTestFinished, vi } from 'vitest'; +import { makeFlushLock } from '../src/flush'; + +describe('Flush buffer test', () => { + const waitUntilPromises: Promise[] = []; + const mockExecutionContext: ExecutionContext = { + waitUntil: vi.fn(prmise => { + waitUntilPromises.push(prmise); + }), + passThroughOnException: vi.fn(), + }; + it('should flush buffer immediately if no waitUntil were called', async () => { + const { finalize } = makeFlushLock(mockExecutionContext); + await expect(finalize()).resolves.toBeUndefined(); + }); + it('should flush buffer only after all waitUntil were finished', async () => { + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + const task = new Promise(resolve => setTimeout(resolve, 100)); + const lock = makeFlushLock(mockExecutionContext); + mockExecutionContext.waitUntil(task); + void lock.finalize(); + vi.advanceTimersToNextTimer(); + await Promise.all(waitUntilPromises); + await expect(lock.ready).resolves.toBeUndefined(); + }); +}); diff --git a/packages/cloudflare/test/handler.test.ts b/packages/cloudflare/test/handler.test.ts index 930e508cfddb..97e93199ea31 100644 --- a/packages/cloudflare/test/handler.test.ts +++ b/packages/cloudflare/test/handler.test.ts @@ -10,11 +10,10 @@ import type { } from '@cloudflare/workers-types'; import type { Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; -import { beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { CloudflareClient } from '../src/client'; import { withSentry } from '../src/handler'; import { markAsInstrumented } from '../src/instrument'; -import { createPromiseResolver } from '../src/utils/makePromiseResolver'; // Custom type for hono-like apps (cloudflare handlers) that include errorHandler and onError type HonoLikeApp = ExportedHandler< @@ -31,29 +30,8 @@ const MOCK_ENV = { SENTRY_RELEASE: '1.1.1', }; -function makeWaitUntilAndTask< - M extends ReturnType, - T extends M & { - readonly ready: Promise; - readonly task: { readonly promise: Promise; readonly resolve: M }; - }, ->(): T { - const waitUntil = vi.fn(); - const { promise, resolve } = createPromiseResolver(); - const resolver = vi.fn().mockImplementation(resolve).mockName('waitUntil.ready'); - Object.defineProperties(waitUntil, { - ready: { - get: () => Promise.all(waitUntil.mock.calls.map(call => call[0])).then(() => resolver), - enumerable: true, - configurable: true, - }, - task: { - value: { promise, resolve: resolver }, - enumerable: true, - configurable: true, - }, - }); - return waitUntil as T; +function addDelayedWaitUntil(context: ExecutionContext) { + context.waitUntil(new Promise(resolve => setTimeout(() => resolve()))); } describe('withSentry', () => { @@ -157,22 +135,27 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = makeWaitUntilAndTask(); - process.nextTick(waitUntil.task.resolve); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); const handler = { - fetch(...args) { - args[2].waitUntil(waitUntil.task.promise); + fetch(_request, _env, _context) { + addDelayedWaitUntil(_context); return new Response('test'); }, } satisfies ExportedHandler; const wrappedHandler = withSentry(vi.fn(), handler); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.fetch?.(new Request('https://example.com'), MOCK_ENV, { waitUntil, } as unknown as ExecutionContext); expect(flush).not.toBeCalled(); - const ready = await waitUntil.ready; - expect(flush).toHaveBeenCalledAfter(ready); + expect(waitUntil).toBeCalled(); + vi.advanceTimersToNextTimer().runAllTimers(); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); }); }); @@ -392,23 +375,28 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = makeWaitUntilAndTask(); - process.nextTick(waitUntil.task.resolve); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); const handler = { - scheduled(...args) { - args[2].waitUntil(waitUntil.task.promise); + scheduled(_controller, _env, _context) { + addDelayedWaitUntil(_context); return; }, } satisfies ExportedHandler; const wrappedHandler = withSentry(vi.fn(), handler); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.scheduled?.(createMockScheduledController(), MOCK_ENV, { waitUntil, } as unknown as ExecutionContext); expect(flush).not.toBeCalled(); - await waitUntil.ready; + expect(waitUntil).toBeCalled(); + vi.advanceTimersToNextTimer().runAllTimers(); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); - expect(flush).toHaveBeenCalledAfter(waitUntil); }); }); @@ -626,24 +614,28 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = makeWaitUntilAndTask(); - - process.nextTick(waitUntil.task.resolve); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); const handler = { - email(...args) { - args[2].waitUntil(waitUntil.task.promise); + email(_controller, _env, _context) { + addDelayedWaitUntil(_context); return; }, } satisfies ExportedHandler; const wrappedHandler = withSentry(vi.fn(), handler); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.email?.(createMockEmailMessage(), MOCK_ENV, { waitUntil, } as unknown as ExecutionContext); expect(flush).not.toBeCalled(); - const ready = await waitUntil.ready; + expect(waitUntil).toBeCalled(); + vi.advanceTimersToNextTimer().runAllTimers(); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); - expect(flush).toHaveBeenCalledAfter(ready); }); }); @@ -865,22 +857,27 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = makeWaitUntilAndTask(); - process.nextTick(waitUntil.task.resolve); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); const handler = { - queue(...args) { - args[2].waitUntil(waitUntil.task.promise); + queue(_controller, _env, _context) { + addDelayedWaitUntil(_context); return; }, } satisfies ExportedHandler; const wrappedHandler = withSentry(vi.fn(), handler); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.queue?.(createMockQueueBatch(), MOCK_ENV, { waitUntil, } as unknown as ExecutionContext); expect(flush).not.toBeCalled(); - - expect(flush).toHaveBeenCalledAfter(await waitUntil.ready); + expect(waitUntil).toBeCalled(); + vi.advanceTimersToNextTimer().runAllTimers(); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); }); }); @@ -1057,23 +1054,29 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = makeWaitUntilAndTask(); - process.nextTick(waitUntil.task.resolve); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + flush.mockRestore(); + }); const handler = { - tail(...args) { - args[2].waitUntil(waitUntil.task.promise); + tail(_controller, _env, _context) { + addDelayedWaitUntil(_context); return; }, } satisfies ExportedHandler; const wrappedHandler = withSentry(vi.fn(), handler); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); await wrappedHandler.tail?.(createMockTailEvent(), MOCK_ENV, { waitUntil, } as unknown as ExecutionContext); expect(flush).not.toBeCalled(); - const ready = await waitUntil.ready; + expect(waitUntil).toBeCalled(); + vi.advanceTimersToNextTimer().runAllTimers(); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); - expect(flush).toHaveBeenCalledAfter(ready); }); }); diff --git a/packages/cloudflare/test/request.test.ts b/packages/cloudflare/test/request.test.ts index eaf06f8f7f3e..d6d0de5824a1 100644 --- a/packages/cloudflare/test/request.test.ts +++ b/packages/cloudflare/test/request.test.ts @@ -4,17 +4,20 @@ import type { ExecutionContext } from '@cloudflare/workers-types'; import type { Event } from '@sentry/core'; import * as SentryCore from '@sentry/core'; -import { beforeAll, beforeEach, describe, expect, test, vi } from 'vitest'; +import { beforeAll, beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest'; import { setAsyncLocalStorageAsyncContextStrategy } from '../src/async'; import type { CloudflareOptions } from '../src/client'; import { CloudflareClient } from '../src/client'; import { wrapRequestHandler } from '../src/request'; -import { createPromiseResolver } from '../src/utils/makePromiseResolver'; const MOCK_OPTIONS: CloudflareOptions = { dsn: 'https://public@dsn.ingest.sentry.io/1337', }; +function addDelayedWaitUntil(context: ExecutionContext) { + context.waitUntil(new Promise(resolve => setTimeout(() => resolve()))); +} + describe('withSentry', () => { beforeAll(() => { setAsyncLocalStorageAsyncContextStrategy(); @@ -67,24 +70,25 @@ describe('withSentry', () => { test('flush must be called when all waitUntil are done', async () => { const flush = vi.spyOn(SentryCore.Client.prototype, 'flush'); - const waitUntil = vi.fn(); + vi.useFakeTimers(); + onTestFinished(() => { + vi.useRealTimers(); + }); + const waits: Promise[] = []; + const waitUntil = vi.fn(promise => waits.push(promise)); const context = { waitUntil, } as unknown as ExecutionContext; - const { promise, resolve } = createPromiseResolver(); - const resolved = vi.fn(() => resolve()); - process.nextTick(resolved); - await wrapRequestHandler({ options: MOCK_OPTIONS, request: new Request('https://example.com'), context }, () => { - context.waitUntil(promise); + addDelayedWaitUntil(context); return new Response('test'); }); expect(flush).not.toBeCalled(); expect(waitUntil).toBeCalled(); - await Promise.all(waitUntil.mock.calls.map(call => call[0])); - expect(flush).toHaveBeenCalledAfter(resolved); + vi.advanceTimersToNextTimerAsync().then(() => vi.runAllTimers()); + await Promise.all(waits); expect(flush).toHaveBeenCalledOnce(); }); @@ -333,5 +337,5 @@ function createMockExecutionContext(): ExecutionContext { return { waitUntil: vi.fn(), passThroughOnException: vi.fn(), - } as unknown as ExecutionContext; + }; }