Skip to content

Commit 3e10df6

Browse files
committed
Revert "feat(cloudflare): Introduce lock instrumentation for context.waitUntil to prevent multiple instrumentation (#17539)"
This reverts commit 8927921.
1 parent 108ed16 commit 3e10df6

14 files changed

+158
-300
lines changed

packages/cloudflare/src/client.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { ClientOptions, Options, ServerRuntimeClientOptions } from '@sentry/core';
22
import { applySdkMetadata, ServerRuntimeClient } from '@sentry/core';
3+
import type { makeFlushLock } from './flush';
34
import type { CloudflareTransportOptions } from './transport';
4-
import type { makeFlushLock } from './utils/flushLock';
55

66
/**
77
* The Sentry Cloudflare SDK Client.

packages/cloudflare/src/durableobject.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import { isInstrumented, markAsInstrumented } from './instrument';
1818
import { getFinalOptions } from './options';
1919
import { wrapRequestHandler } from './request';
2020
import { init } from './sdk';
21-
import { copyExecutionContext } from './utils/copyExecutionContext';
2221

2322
type MethodWrapperOptions = {
2423
spanName?: string;
@@ -193,11 +192,9 @@ export function instrumentDurableObjectWithSentry<
193192
C extends new (state: DurableObjectState, env: E) => T,
194193
>(optionsCallback: (env: E) => CloudflareOptions, DurableObjectClass: C): C {
195194
return new Proxy(DurableObjectClass, {
196-
construct(target, [ctx, env]) {
195+
construct(target, [context, env]) {
197196
setAsyncLocalStorageAsyncContextStrategy();
198197

199-
const context = copyExecutionContext(ctx);
200-
201198
const options = getFinalOptions(optionsCallback(env), env);
202199

203200
const obj = new target(context, env);

packages/cloudflare/src/flush.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import type { ExecutionContext } from '@cloudflare/workers-types';
2+
3+
type FlushLock = {
4+
readonly ready: Promise<void>;
5+
readonly finalize: () => Promise<void>;
6+
};
7+
8+
/**
9+
* Enhances the given execution context by wrapping its `waitUntil` method with a proxy
10+
* to monitor pending tasks, and provides a flusher function to ensure all tasks
11+
* have been completed before executing any subsequent logic.
12+
*
13+
* @param {ExecutionContext} context - The execution context to be enhanced. If no context is provided, the function returns undefined.
14+
* @return {FlushLock} Returns a flusher function if a valid context is provided, otherwise undefined.
15+
*/
16+
export function makeFlushLock(context: ExecutionContext): FlushLock {
17+
let resolveAllDone: () => void = () => undefined;
18+
const allDone = new Promise<void>(res => {
19+
resolveAllDone = res;
20+
});
21+
let pending = 0;
22+
const originalWaitUntil = context.waitUntil.bind(context) as typeof context.waitUntil;
23+
context.waitUntil = promise => {
24+
pending++;
25+
return originalWaitUntil(
26+
promise.finally(() => {
27+
if (--pending === 0) resolveAllDone();
28+
}),
29+
);
30+
};
31+
return Object.freeze({
32+
ready: allDone,
33+
finalize: () => {
34+
if (pending === 0) resolveAllDone();
35+
return allDone;
36+
},
37+
});
38+
}

packages/cloudflare/src/handler.ts

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { getFinalOptions } from './options';
1414
import { wrapRequestHandler } from './request';
1515
import { addCloudResourceContext } from './scope-utils';
1616
import { init } from './sdk';
17-
import { copyExecutionContext } from './utils/copyExecutionContext';
1817

1918
/**
2019
* Wrapper for Cloudflare handlers.
@@ -38,11 +37,9 @@ export function withSentry<Env = unknown, QueueHandlerMessage = unknown, CfHostM
3837
if ('fetch' in handler && typeof handler.fetch === 'function' && !isInstrumented(handler.fetch)) {
3938
handler.fetch = new Proxy(handler.fetch, {
4039
apply(target, thisArg, args: Parameters<ExportedHandlerFetchHandler<Env, CfHostMetadata>>) {
41-
const [request, env, ctx] = args;
40+
const [request, env, context] = args;
4241

4342
const options = getFinalOptions(optionsCallback(env), env);
44-
const context = copyExecutionContext(ctx);
45-
args[2] = context;
4643

4744
return wrapRequestHandler({ options, request, context }, () => target.apply(thisArg, args));
4845
},
@@ -74,9 +71,7 @@ export function withSentry<Env = unknown, QueueHandlerMessage = unknown, CfHostM
7471
if ('scheduled' in handler && typeof handler.scheduled === 'function' && !isInstrumented(handler.scheduled)) {
7572
handler.scheduled = new Proxy(handler.scheduled, {
7673
apply(target, thisArg, args: Parameters<ExportedHandlerScheduledHandler<Env>>) {
77-
const [event, env, ctx] = args;
78-
const context = copyExecutionContext(ctx);
79-
args[2] = context;
74+
const [event, env, context] = args;
8075
return withIsolationScope(isolationScope => {
8176
const options = getFinalOptions(optionsCallback(env), env);
8277
const waitUntil = context.waitUntil.bind(context);
@@ -119,9 +114,7 @@ export function withSentry<Env = unknown, QueueHandlerMessage = unknown, CfHostM
119114
if ('email' in handler && typeof handler.email === 'function' && !isInstrumented(handler.email)) {
120115
handler.email = new Proxy(handler.email, {
121116
apply(target, thisArg, args: Parameters<EmailExportedHandler<Env>>) {
122-
const [emailMessage, env, ctx] = args;
123-
const context = copyExecutionContext(ctx);
124-
args[2] = context;
117+
const [emailMessage, env, context] = args;
125118
return withIsolationScope(isolationScope => {
126119
const options = getFinalOptions(optionsCallback(env), env);
127120
const waitUntil = context.waitUntil.bind(context);
@@ -162,9 +155,7 @@ export function withSentry<Env = unknown, QueueHandlerMessage = unknown, CfHostM
162155
if ('queue' in handler && typeof handler.queue === 'function' && !isInstrumented(handler.queue)) {
163156
handler.queue = new Proxy(handler.queue, {
164157
apply(target, thisArg, args: Parameters<ExportedHandlerQueueHandler<Env, QueueHandlerMessage>>) {
165-
const [batch, env, ctx] = args;
166-
const context = copyExecutionContext(ctx);
167-
args[2] = context;
158+
const [batch, env, context] = args;
168159

169160
return withIsolationScope(isolationScope => {
170161
const options = getFinalOptions(optionsCallback(env), env);
@@ -214,9 +205,7 @@ export function withSentry<Env = unknown, QueueHandlerMessage = unknown, CfHostM
214205
if ('tail' in handler && typeof handler.tail === 'function' && !isInstrumented(handler.tail)) {
215206
handler.tail = new Proxy(handler.tail, {
216207
apply(target, thisArg, args: Parameters<ExportedHandlerTailHandler<Env>>) {
217-
const [, env, ctx] = args;
218-
const context = copyExecutionContext(ctx);
219-
args[2] = context;
208+
const [, env, context] = args;
220209

221210
return withIsolationScope(async isolationScope => {
222211
const options = getFinalOptions(optionsCallback(env), env);

packages/cloudflare/src/sdk.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import {
1212
} from '@sentry/core';
1313
import type { CloudflareClientOptions, CloudflareOptions } from './client';
1414
import { CloudflareClient } from './client';
15+
import { makeFlushLock } from './flush';
1516
import { fetchIntegration } from './integrations/fetch';
1617
import { setupOpenTelemetryTracer } from './opentelemetry/tracer';
1718
import { makeCloudflareTransport } from './transport';
18-
import { makeFlushLock } from './utils/flushLock';
1919
import { defaultStackParser } from './vendor/stacktrace';
2020

2121
/** Get the default integrations for the Cloudflare SDK. */

packages/cloudflare/src/utils/copyExecutionContext.ts

Lines changed: 0 additions & 47 deletions
This file was deleted.

packages/cloudflare/src/utils/flushLock.ts

Lines changed: 0 additions & 57 deletions
This file was deleted.

packages/cloudflare/src/utils/makePromiseResolver.ts

Lines changed: 0 additions & 26 deletions
This file was deleted.

packages/cloudflare/test/copy-execution-context.test.ts

Lines changed: 0 additions & 47 deletions
This file was deleted.

packages/cloudflare/test/durableobject.test.ts

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import type { ExecutionContext } from '@cloudflare/workers-types';
22
import * as SentryCore from '@sentry/core';
3-
import { afterEach, describe, expect, it, vi } from 'vitest';
3+
import { afterEach, describe, expect, it, onTestFinished, vi } from 'vitest';
44
import { instrumentDurableObjectWithSentry } from '../src';
55
import { isInstrumented } from '../src/instrument';
6-
import { createPromiseResolver } from '../src/utils/makePromiseResolver';
76

87
describe('instrumentDurableObjectWithSentry', () => {
98
afterEach(() => {
@@ -123,13 +122,15 @@ describe('instrumentDurableObjectWithSentry', () => {
123122
});
124123

125124
it('flush performs after all waitUntil promises are finished', async () => {
125+
vi.useFakeTimers();
126+
onTestFinished(() => {
127+
vi.useRealTimers();
128+
});
126129
const flush = vi.spyOn(SentryCore.Client.prototype, 'flush');
127130
const waitUntil = vi.fn();
128-
const { promise, resolve } = createPromiseResolver();
129-
process.nextTick(resolve);
130131
const testClass = vi.fn(context => ({
131132
fetch: () => {
132-
context.waitUntil(promise);
133+
context.waitUntil(new Promise(res => setTimeout(res)));
133134
return new Response('test');
134135
},
135136
}));
@@ -141,7 +142,8 @@ describe('instrumentDurableObjectWithSentry', () => {
141142
expect(() => dObject.fetch(new Request('https://example.com'))).not.toThrow();
142143
expect(flush).not.toBeCalled();
143144
expect(waitUntil).toHaveBeenCalledOnce();
144-
await Promise.all(waitUntil.mock.calls.map(call => call[0]));
145+
vi.advanceTimersToNextTimer();
146+
await Promise.all(waitUntil.mock.calls.map(([p]) => p));
145147
expect(flush).toBeCalled();
146148
});
147149

0 commit comments

Comments
 (0)