Skip to content

Commit 16578ab

Browse files
author
cod1k
committed
Add tests for ensuring flush is called after all waitUntil promises resolve
1 parent e9fad91 commit 16578ab

File tree

3 files changed

+192
-22
lines changed

3 files changed

+192
-22
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import { type ExecutionContext } from '@cloudflare/workers-types';
2+
import * as SentryCore from '@sentry/core';
3+
import { describe, expect, it, onTestFinished, vi } from 'vitest';
4+
import { makeFlushAfterAll } from '../src/flush';
5+
6+
describe('Flush buffer test', () => {
7+
const waitUntilPromises: Promise<void>[] = [];
8+
const mockExecutionContext: ExecutionContext = {
9+
waitUntil: vi.fn(prmise => {
10+
waitUntilPromises.push(prmise);
11+
}),
12+
passThroughOnException: vi.fn(),
13+
};
14+
it('should flush buffer immediately if no waitUntil were called', () => {
15+
const coreFlush = vi.spyOn(SentryCore, 'flush');
16+
const flush = makeFlushAfterAll(mockExecutionContext);
17+
flush();
18+
expect(coreFlush).toBeCalled();
19+
});
20+
it('should flush buffer only after all waitUntil were finished', async () => {
21+
vi.useFakeTimers();
22+
onTestFinished(() => {
23+
vi.useRealTimers();
24+
});
25+
const task = new Promise(resolve => setTimeout(resolve, 100));
26+
const coreFlush = vi.spyOn(SentryCore, 'flush');
27+
const flush = makeFlushAfterAll(mockExecutionContext);
28+
mockExecutionContext.waitUntil(task);
29+
flush();
30+
expect(coreFlush).not.toBeCalled();
31+
vi.advanceTimersToNextTimer();
32+
await Promise.all(waitUntilPromises);
33+
expect(coreFlush).toBeCalled();
34+
});
35+
});

packages/cloudflare/test/handler.test.ts

Lines changed: 154 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
11
// Note: These tests run the handler in Node.js, which has some differences to the cloudflare workers runtime.
22
// Although this is not ideal, this is the best we can do until we have a better way to test cloudflare workers.
33

4-
import type { ForwardableEmailMessage, MessageBatch, ScheduledController, TraceItem } from '@cloudflare/workers-types';
4+
import type {
5+
ExecutionContext,
6+
ForwardableEmailMessage,
7+
MessageBatch,
8+
ScheduledController,
9+
TraceItem,
10+
} from '@cloudflare/workers-types';
511
import type { Event } from '@sentry/core';
612
import * as SentryCore from '@sentry/core';
7-
import { beforeEach, describe, expect, test, vi } from 'vitest';
13+
import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vitest';
814
import { CloudflareClient } from '../src/client';
915
import { withSentry } from '../src/handler';
1016
import { markAsInstrumented } from '../src/instrument';
@@ -24,6 +30,10 @@ const MOCK_ENV = {
2430
SENTRY_RELEASE: '1.1.1',
2531
};
2632

33+
function addDelayedWaitUntil(context: ExecutionContext) {
34+
context.waitUntil(new Promise<void>(resolve => setTimeout(() => resolve())));
35+
}
36+
2737
describe('withSentry', () => {
2838
beforeEach(() => {
2939
vi.clearAllMocks();
@@ -122,6 +132,32 @@ describe('withSentry', () => {
122132

123133
expect(sentryEvent.release).toEqual('2.0.0');
124134
});
135+
136+
test('flush must be called when all waitUntil are done', async () => {
137+
const flush = vi.spyOn(SentryCore, 'flush');
138+
vi.useFakeTimers();
139+
onTestFinished(() => {
140+
vi.useRealTimers();
141+
});
142+
const handler = {
143+
fetch(_request, _env, _context) {
144+
addDelayedWaitUntil(_context);
145+
return new Response('test');
146+
},
147+
} satisfies ExportedHandler<typeof MOCK_ENV>;
148+
149+
const wrappedHandler = withSentry(vi.fn(), handler);
150+
const waits: Promise<unknown>[] = [];
151+
const waitUntil = vi.fn(promise => waits.push(promise));
152+
await wrappedHandler.fetch?.(new Request('https://example.com'), MOCK_ENV, {
153+
waitUntil,
154+
} as unknown as ExecutionContext);
155+
expect(flush).not.toBeCalled();
156+
expect(waitUntil).toBeCalled();
157+
vi.advanceTimersToNextTimer();
158+
await Promise.all(waits);
159+
expect(flush).toHaveBeenCalledOnce();
160+
});
125161
});
126162

127163
describe('scheduled handler', () => {
@@ -198,13 +234,12 @@ describe('withSentry', () => {
198234
} satisfies ExportedHandler<typeof MOCK_ENV>;
199235

200236
const context = createMockExecutionContext();
237+
const waitUntilSpy = vi.spyOn(context, 'waitUntil');
201238
const wrappedHandler = withSentry(env => ({ dsn: env.SENTRY_DSN }), handler);
202239
await wrappedHandler.scheduled?.(createMockScheduledController(), MOCK_ENV, context);
203240

204-
// eslint-disable-next-line @typescript-eslint/unbound-method
205-
expect(context.waitUntil).toHaveBeenCalledTimes(1);
206-
// eslint-disable-next-line @typescript-eslint/unbound-method
207-
expect(context.waitUntil).toHaveBeenLastCalledWith(expect.any(Promise));
241+
expect(waitUntilSpy).toHaveBeenCalledTimes(1);
242+
expect(waitUntilSpy).toHaveBeenLastCalledWith(expect.any(Promise));
208243
});
209244

210245
test('creates a cloudflare client and sets it on the handler', async () => {
@@ -337,6 +372,32 @@ describe('withSentry', () => {
337372
});
338373
});
339374
});
375+
376+
test('flush must be called when all waitUntil are done', async () => {
377+
const flush = vi.spyOn(SentryCore, 'flush');
378+
vi.useFakeTimers();
379+
onTestFinished(() => {
380+
vi.useRealTimers();
381+
});
382+
const handler = {
383+
scheduled(_controller, _env, _context) {
384+
addDelayedWaitUntil(_context);
385+
return;
386+
},
387+
} satisfies ExportedHandler<typeof MOCK_ENV>;
388+
389+
const wrappedHandler = withSentry(vi.fn(), handler);
390+
const waits: Promise<unknown>[] = [];
391+
const waitUntil = vi.fn(promise => waits.push(promise));
392+
await wrappedHandler.scheduled?.(createMockScheduledController(), MOCK_ENV, {
393+
waitUntil,
394+
} as unknown as ExecutionContext);
395+
expect(flush).not.toBeCalled();
396+
expect(waitUntil).toBeCalled();
397+
vi.advanceTimersToNextTimer();
398+
await Promise.all(waits);
399+
expect(flush).toHaveBeenCalledOnce();
400+
});
340401
});
341402

342403
describe('email handler', () => {
@@ -413,13 +474,12 @@ describe('withSentry', () => {
413474
} satisfies ExportedHandler<typeof MOCK_ENV>;
414475

415476
const context = createMockExecutionContext();
477+
const waitUntilSpy = vi.spyOn(context, 'waitUntil');
416478
const wrappedHandler = withSentry(env => ({ dsn: env.SENTRY_DSN }), handler);
417479
await wrappedHandler.email?.(createMockEmailMessage(), MOCK_ENV, context);
418480

419-
// eslint-disable-next-line @typescript-eslint/unbound-method
420-
expect(context.waitUntil).toHaveBeenCalledTimes(1);
421-
// eslint-disable-next-line @typescript-eslint/unbound-method
422-
expect(context.waitUntil).toHaveBeenLastCalledWith(expect.any(Promise));
481+
expect(waitUntilSpy).toHaveBeenCalledTimes(1);
482+
expect(waitUntilSpy).toHaveBeenLastCalledWith(expect.any(Promise));
423483
});
424484

425485
test('creates a cloudflare client and sets it on the handler', async () => {
@@ -551,6 +611,32 @@ describe('withSentry', () => {
551611
});
552612
});
553613
});
614+
615+
test('flush must be called when all waitUntil are done', async () => {
616+
const flush = vi.spyOn(SentryCore, 'flush');
617+
vi.useFakeTimers();
618+
onTestFinished(() => {
619+
vi.useRealTimers();
620+
});
621+
const handler = {
622+
email(_controller, _env, _context) {
623+
addDelayedWaitUntil(_context);
624+
return;
625+
},
626+
} satisfies ExportedHandler<typeof MOCK_ENV>;
627+
628+
const wrappedHandler = withSentry(vi.fn(), handler);
629+
const waits: Promise<unknown>[] = [];
630+
const waitUntil = vi.fn(promise => waits.push(promise));
631+
await wrappedHandler.email?.(createMockEmailMessage(), MOCK_ENV, {
632+
waitUntil,
633+
} as unknown as ExecutionContext);
634+
expect(flush).not.toBeCalled();
635+
expect(waitUntil).toBeCalled();
636+
vi.advanceTimersToNextTimer();
637+
await Promise.all(waits);
638+
expect(flush).toHaveBeenCalledOnce();
639+
});
554640
});
555641

556642
describe('queue handler', () => {
@@ -627,13 +713,12 @@ describe('withSentry', () => {
627713
} satisfies ExportedHandler<typeof MOCK_ENV>;
628714

629715
const context = createMockExecutionContext();
716+
const waitUntilSpy = vi.spyOn(context, 'waitUntil');
630717
const wrappedHandler = withSentry(env => ({ dsn: env.SENTRY_DSN }), handler);
631718
await wrappedHandler.queue?.(createMockQueueBatch(), MOCK_ENV, context);
632719

633-
// eslint-disable-next-line @typescript-eslint/unbound-method
634-
expect(context.waitUntil).toHaveBeenCalledTimes(1);
635-
// eslint-disable-next-line @typescript-eslint/unbound-method
636-
expect(context.waitUntil).toHaveBeenLastCalledWith(expect.any(Promise));
720+
expect(waitUntilSpy).toHaveBeenCalledTimes(1);
721+
expect(waitUntilSpy).toHaveBeenLastCalledWith(expect.any(Promise));
637722
});
638723

639724
test('creates a cloudflare client and sets it on the handler', async () => {
@@ -769,6 +854,32 @@ describe('withSentry', () => {
769854
});
770855
});
771856
});
857+
858+
test('flush must be called when all waitUntil are done', async () => {
859+
const flush = vi.spyOn(SentryCore, 'flush');
860+
vi.useFakeTimers();
861+
onTestFinished(() => {
862+
vi.useRealTimers();
863+
});
864+
const handler = {
865+
queue(_controller, _env, _context) {
866+
addDelayedWaitUntil(_context);
867+
return;
868+
},
869+
} satisfies ExportedHandler<typeof MOCK_ENV>;
870+
871+
const wrappedHandler = withSentry(vi.fn(), handler);
872+
const waits: Promise<unknown>[] = [];
873+
const waitUntil = vi.fn(promise => waits.push(promise));
874+
await wrappedHandler.queue?.(createMockQueueBatch(), MOCK_ENV, {
875+
waitUntil,
876+
} as unknown as ExecutionContext);
877+
expect(flush).not.toBeCalled();
878+
expect(waitUntil).toBeCalled();
879+
vi.advanceTimersToNextTimer();
880+
await Promise.all(waits);
881+
expect(flush).toHaveBeenCalledOnce();
882+
});
772883
});
773884

774885
describe('tail handler', () => {
@@ -845,13 +956,12 @@ describe('withSentry', () => {
845956
} satisfies ExportedHandler<typeof MOCK_ENV>;
846957

847958
const context = createMockExecutionContext();
959+
const waitUntilSpy = vi.spyOn(context, 'waitUntil');
848960
const wrappedHandler = withSentry(env => ({ dsn: env.SENTRY_DSN }), handler);
849961
await wrappedHandler.tail?.(createMockTailEvent(), MOCK_ENV, context);
850962

851-
// eslint-disable-next-line @typescript-eslint/unbound-method
852-
expect(context.waitUntil).toHaveBeenCalledTimes(1);
853-
// eslint-disable-next-line @typescript-eslint/unbound-method
854-
expect(context.waitUntil).toHaveBeenLastCalledWith(expect.any(Promise));
963+
expect(waitUntilSpy).toHaveBeenCalledTimes(1);
964+
expect(waitUntilSpy).toHaveBeenLastCalledWith(expect.any(Promise));
855965
});
856966

857967
test('creates a cloudflare client and sets it on the handler', async () => {
@@ -941,6 +1051,32 @@ describe('withSentry', () => {
9411051
expect(thrownError).toBe(error);
9421052
});
9431053
});
1054+
1055+
test('flush must be called when all waitUntil are done', async () => {
1056+
const flush = vi.spyOn(SentryCore, 'flush');
1057+
vi.useFakeTimers();
1058+
onTestFinished(() => {
1059+
vi.useRealTimers();
1060+
});
1061+
const handler = {
1062+
tail(_controller, _env, _context) {
1063+
addDelayedWaitUntil(_context);
1064+
return;
1065+
},
1066+
} satisfies ExportedHandler<typeof MOCK_ENV>;
1067+
1068+
const wrappedHandler = withSentry(vi.fn(), handler);
1069+
const waits: Promise<unknown>[] = [];
1070+
const waitUntil = vi.fn(promise => waits.push(promise));
1071+
await wrappedHandler.tail?.(createMockTailEvent(), MOCK_ENV, {
1072+
waitUntil,
1073+
} as unknown as ExecutionContext);
1074+
expect(flush).not.toBeCalled();
1075+
expect(waitUntil).toBeCalled();
1076+
vi.advanceTimersToNextTimer();
1077+
await Promise.all(waits);
1078+
expect(flush).toHaveBeenCalledOnce();
1079+
});
9441080
});
9451081

9461082
describe('hono errorHandler', () => {

packages/cloudflare/test/request.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,14 @@ describe('withSentry', () => {
3333

3434
test('flushes the event after the handler is done using the cloudflare context.waitUntil', async () => {
3535
const context = createMockExecutionContext();
36+
const waitUntilSpy = vi.spyOn(context, 'waitUntil');
3637
await wrapRequestHandler(
3738
{ options: MOCK_OPTIONS, request: new Request('https://example.com'), context },
3839
() => new Response('test'),
3940
);
4041

41-
// eslint-disable-next-line @typescript-eslint/unbound-method
42-
expect(context.waitUntil).toHaveBeenCalledTimes(1);
43-
// eslint-disable-next-line @typescript-eslint/unbound-method
44-
expect(context.waitUntil).toHaveBeenLastCalledWith(expect.any(Promise));
42+
expect(waitUntilSpy).toHaveBeenCalledTimes(1);
43+
expect(waitUntilSpy).toHaveBeenLastCalledWith(expect.any(Promise));
4544
});
4645

4746
test("doesn't error if context is undefined", () => {

0 commit comments

Comments
 (0)