Skip to content

Commit cef14dc

Browse files
JPeer264claude
andauthored
fix(cloudflare): Reject unknown keys in options callbacks (#23290)
It seemed that TypeScript didn't bother on wrong types in the options. This enables it and the following throws now a TS error: ```js withSentry(() => ({ something: null }), handler) ``` It also seemed that we forgot to add `serverName` in the available options for Cloudflare Some workaround to make the types stricter within functions. See: https://www.typescriptlang.org/play/?#code/C4TwDgpgBA8mwEsD2A7AzlAvFA3gKCkKlQgH4AuKARgG4CjgB3JCqAJjoF889RIoAysABOCAMbA4iVGgA8MAHxZYUCAA9gEFABMMACgB0RgIbCA5mkooIANwjCA2gF0AlFiUBXFAGsUSRij0pFDWdsL0lDBQAGSw8MjoMVAAShBiSMLasgCiamIANh7aELLeECBIAGawADRQZRXVUgloCnWh9gp0eOnowFAAsiAwAEYAVmn92HoA+pHxMm6YSjicdL1o-UMAYl4SCcqzlHpLSs2L7rhrPTJbIEKiErso+6jK8spevv4oCkdQJ0uD3EkgW6HkClOV26AHoYVBjP18hBjJtiNYoAgUDZjPkENp6uUoABaVRqSASCDaSgAIj8kBpqmEwgyaDwQ1GEwkehw6IglCo7SQkEoAHJRVBOC46HCEUiUWiSFAcXiCQ0SSEkP11BTNNSoAADekQA1QNAACyQHnyBPsLPCOz20hQekBywBvJIAqFIqg4slLmleFlfmVuPxhJAdVs9hAwHNWLMUEqxgQ+UsUDApkQuPyICg6WZk3ZIGerxdbqUPM1vv9UqDssqCDUVIMUGyYTjCZQSZTaYwqLJuqpJeBTydCVdUOrXuoPv5fol9ZoQA Related: microsoft/TypeScript#7547 (comment) --- Clanker description: TypeScript's excess property check does not survive a function boundary, so typos in the options callback went unreported: ```js withSentry(() => ({ dsn, tracesSampleRte: 1 }), handler) // compiled fine ``` `StrictCloudflareOptions<O>` infers the returned literal into `O` and intersects it with a `never` map over its unknown keys, restoring the error. Applied to every entry point taking options. Note this is top level only, nested option objects stay unchecked, and it also rejects a pre-built object carrying extra keys, which a plain excess property check would allow. Two bugs this surfaced: the Hono Cloudflare middleware spread the middleware-only `shouldHandleError` into the SDK options, and `serverName` was missing from `CloudflareOptions` despite being consumed by `ServerRuntimeClient`. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent ffd3b65 commit cef14dc

12 files changed

Lines changed: 240 additions & 13 deletions

File tree

dev-packages/cloudflare-integration-tests/suites/types/withsentry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ interface ManualEnv {
2323
export const reproduction = withSentry(
2424
env => {
2525
expectTypeOf(env).toEqualTypeOf<ManualEnv>();
26-
return { dsn: env.SENTRY_DATA_SOURCE_NAME, sendDefaultPii: true };
26+
return { dsn: env.SENTRY_DATA_SOURCE_NAME };
2727
},
2828
{
2929
fetch(_, env) {

packages/cloudflare/src/client.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,13 @@ interface BaseCloudflareOptions {
281281
*/
282282
durableObjectStorageSpanAllowlist?: Array<string | RegExp>;
283283

284+
/**
285+
* Sets an optional server name (device name).
286+
*
287+
* This is useful for identifying which server or instance is sending events.
288+
*/
289+
serverName?: string;
290+
284291
/**
285292
* If you use Spotlight by Sentry during development, use
286293
* this option to forward captured Sentry events to Spotlight.

packages/cloudflare/src/defineCloudflareOptions.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { CloudflareOptions } from './client';
2-
import type { DefaultEnv } from './types';
2+
import type { DefaultEnv, StrictCloudflareOptions } from './types';
33

44
/**
55
* Define the Sentry options for a Cloudflare Worker in a dedicated module.
@@ -35,6 +35,16 @@ import type { DefaultEnv } from './types';
3535
* export default defineCloudflareOptions({ tracesSampleRate: 1.0 });
3636
* ```
3737
*/
38+
// Overloads rather than a union parameter: TypeScript does not infer `O` out of a union member,
39+
// so a union signature falls back to the default and the unknown-key check never runs. The object
40+
// overload stays on plain `CloudflareOptions` — a direct object literal is still excess property
41+
// checked, and a callback cannot match it.
42+
export function defineCloudflareOptions<Env = DefaultEnv, O = unknown>(
43+
callback: (env: Env) => StrictCloudflareOptions<O> | undefined,
44+
): (env: Env) => CloudflareOptions | undefined;
45+
export function defineCloudflareOptions<Env = DefaultEnv>(
46+
options: CloudflareOptions,
47+
): (env: Env) => CloudflareOptions | undefined;
3848
export function defineCloudflareOptions<Env = DefaultEnv>(
3949
optionsOrCallback: CloudflareOptions | ((env: Env) => CloudflareOptions | undefined),
4050
): (env: Env) => CloudflareOptions | undefined {

packages/cloudflare/src/durableobject.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { init } from './sdk';
1111
import { instrumentContext } from './utils/instrumentContext';
1212
import { hasRpcMeta } from './utils/rpcMeta';
1313
import { instrumentCloudflareAgent } from './instrumentations/agents';
14-
import type { DefaultEnv, ResolveEnv } from './types';
14+
import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types';
1515
import { type UncheckedMethod, wrapMethodWithSentry } from './wrapMethodWithSentry';
1616

1717
/**
@@ -432,7 +432,8 @@ export function instrumentDurableObjectWithSentry<
432432
T extends DurableObject<any> = DurableObject<Env>,
433433
// oxlint-disable-next-line typescript/no-explicit-any
434434
C extends new (state: DurableObjectState, env: any) => T = new (state: DurableObjectState, env: any) => T,
435-
>(optionsCallback: (env: ResolveEnv<C, Env>) => CloudflareOptions, DurableObjectClass: C): C {
435+
O = unknown,
436+
>(optionsCallback: (env: ResolveEnv<C, Env>) => StrictCloudflareOptions<O>, DurableObjectClass: C): C {
436437
return new Proxy(DurableObjectClass, {
437438
construct(target, [ctx, env], newTarget) {
438439
const { obj, options, context, frameworkManagedMethods } = constructInstrumentedDurableObject(
@@ -494,7 +495,8 @@ export function instrumentAgentWithSentry<
494495
T extends DurableObject<any> = DurableObject<Env>,
495496
// oxlint-disable-next-line typescript/no-explicit-any
496497
C extends new (state: DurableObjectState, env: any) => T = new (state: DurableObjectState, env: any) => T,
497-
>(optionsCallback: (env: ResolveEnv<C, Env>) => CloudflareOptions, AgentClass: C): C {
498+
O = unknown,
499+
>(optionsCallback: (env: ResolveEnv<C, Env>) => StrictCloudflareOptions<O>, AgentClass: C): C {
498500
return new Proxy(AgentClass, {
499501
construct(target, [ctx, env], newTarget) {
500502
const { obj, options, context, frameworkManagedMethods } = constructInstrumentedDurableObject(

packages/cloudflare/src/instrumentations/instrumentWorkerEntrypoint.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { RpcStub, WorkerEntrypoint } from 'cloudflare:workers';
22
import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels';
33
import type { CloudflareOptions } from '../client';
44
import { getFinalOptions } from '../options';
5-
import type { DefaultEnv, ResolveEnv } from '../types';
5+
import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from '../types';
66
import { instrumentContext } from '../utils/instrumentContext';
77
import { extractRpcMeta } from '../utils/rpcMeta';
88
import { type UncheckedMethod, wrapMethodWithSentry } from '../wrapMethodWithSentry';
@@ -155,7 +155,8 @@ export function instrumentWorkerEntrypoint<
155155
T extends WorkerEntrypoint<any, any> = WorkerEntrypoint<Env, Props>,
156156
// oxlint-disable-next-line typescript/no-explicit-any
157157
C extends new (ctx: ExecutionContext, env: any) => T = new (ctx: ExecutionContext, env: any) => T,
158-
>(optionsCallback: (env: ResolveEnv<C, Env>) => CloudflareOptions | undefined, WorkerEntrypointClass: C): C {
158+
O = unknown,
159+
>(optionsCallback: (env: ResolveEnv<C, Env>) => StrictCloudflareOptions<O> | undefined, WorkerEntrypointClass: C): C {
159160
// Set up AsyncLocalStorage strategy ONCE at instrumentation time, not per-request
160161
// This is critical - calling this per-request would create a new AsyncLocalStorage
161162
// each time, breaking scope isolation for concurrent requests

packages/cloudflare/src/pages-plugin.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { CloudflareOptions } from './client';
33
import type { ExecutionContextCompat } from './executionContext';
44
import { wrapRequestHandlerWithInit } from './request';
55
import { init } from './sdk';
6+
import type { StrictCloudflareOptions } from './types';
67

78
/**
89
* Plugin middleware for Cloudflare Pages.
@@ -35,6 +36,31 @@ import { init } from './sdk';
3536
* @param handlerOrOptions Configuration options or a function that returns configuration options.
3637
* @returns A plugin function that can be used in Cloudflare Pages.
3738
*/
39+
// Overloads rather than a union parameter: TypeScript does not infer `O` out of a union member,
40+
// so a union signature falls back to the default and the unknown-key check never runs. The object
41+
// overload stays on plain `CloudflareOptions` — a direct object literal is still excess property
42+
// checked, and a callback cannot match it.
43+
export function sentryPagesPlugin<
44+
// oxlint-disable-next-line typescript/no-explicit-any
45+
Env = any,
46+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
47+
Params extends string = any,
48+
Data extends Record<string, unknown> = Record<string, unknown>,
49+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
50+
PluginParams = any,
51+
O = unknown,
52+
>(
53+
handler: (context: EventPluginContext<Env, Params, Data, PluginParams>) => StrictCloudflareOptions<O>,
54+
): PagesPluginFunction<Env, Params, Data, PluginParams>;
55+
export function sentryPagesPlugin<
56+
// oxlint-disable-next-line typescript/no-explicit-any
57+
Env = any,
58+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
59+
Params extends string = any,
60+
Data extends Record<string, unknown> = Record<string, unknown>,
61+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
62+
PluginParams = any,
63+
>(options: CloudflareOptions): PagesPluginFunction<Env, Params, Data, PluginParams>;
3864
export function sentryPagesPlugin<
3965
// oxlint-disable-next-line typescript/no-explicit-any
4066
Env = any,

packages/cloudflare/src/types.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,38 @@
11
import type { env as cloudflareEnv } from 'cloudflare:workers';
2+
import type { CloudflareOptions } from './client';
23

34
type IsAny<T> = 0 extends 1 & T ? true : false;
45

6+
/**
7+
* `CloudflareOptions` for the options *callbacks*, where the excess property check does not reach.
8+
*
9+
* A plain `CloudflareOptions` parameter already rejects unknown keys when the caller writes a
10+
* direct object literal, which is why it is enough everywhere else. That check only applies to
11+
* literals TypeScript still sees as "fresh", though, and freshness is lost across a function
12+
* boundary — the literal returned from `withSentry(() => ({ dsn, tracesSampleRte: 1 }), handler)`
13+
* is compared as part of a function type, so the typo passes. Since `env` only exists at request
14+
* time, a callback is the only way to configure these APIs, so the check has to be rebuilt:
15+
* inferring the literal into `O` and intersecting it with a `never` map over its extra keys puts
16+
* the error back on the offending property.
17+
*
18+
* `O` is deliberately unconstrained: a `CloudflareOptions` constraint makes inference fail for an
19+
* options object whose keys are *all* unknown, and TypeScript then silently falls back to the type
20+
* parameter default instead of reporting anything. The `CloudflareOptions` member of the
21+
* intersection carries the actual check on known keys.
22+
*
23+
* The function-rejecting branch keeps "returned the options factory instead of calling it" an
24+
* error. A plain `CloudflareOptions` target rejects functions via the weak type check (no shared
25+
* properties), but an intersection is only weak-type checked when every member is weak, and the
26+
* `Record` member has no properties at all — so without the guard a function would slip through.
27+
* `keyof` of a function type is `never`, so the never-map alone cannot catch it.
28+
*
29+
* Two known gaps: this only covers top level keys, and unlike a plain excess property check it
30+
* also rejects a pre-built object carrying extra keys.
31+
*/
32+
export type StrictCloudflareOptions<O> = O extends (...args: never[]) => unknown
33+
? never
34+
: O & CloudflareOptions & Record<Exclude<keyof O, keyof CloudflareOptions>, never>;
35+
536
/**
637
* A handler method of an `ExportedHandler` (`fetch`, `scheduled`, `queue`, ...).
738
*/

packages/cloudflare/src/withSentry.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import { setAsyncLocalStorageAsyncContextStrategy } from '@sentry/server-utils/no-diagnostic-channels';
2-
import type { CloudflareOptions } from './client';
32
import { instrumentExportedHandlerEmail } from './instrumentations/worker/instrumentEmail';
43
import { instrumentExportedHandlerFetch } from './instrumentations/worker/instrumentFetch';
54
import { instrumentExportedHandlerQueue } from './instrumentations/worker/instrumentQueue';
65
import { instrumentExportedHandlerScheduled } from './instrumentations/worker/instrumentScheduled';
76
import { instrumentExportedHandlerTail } from './instrumentations/worker/instrumentTail';
87
import { isCloudflareClass } from './utils/isCloudflareClass';
9-
import type { AnyExportedHandler, DefaultEnv, ResolveEnv } from './types';
8+
import type { AnyExportedHandler, DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types';
109
import {
1110
instrumentWorkerEntrypoint,
1211
type WorkerEntrypointConstructor,
@@ -31,7 +30,8 @@ export function withSentry<
3130
T extends AnyExportedHandler | WorkerEntrypointConstructor<any, any> =
3231
| ExportedHandler<Env, QueueHandlerMessage, CfHostMetadata>
3332
| WorkerEntrypointConstructor<Env>,
34-
>(optionsCallback: (env: ResolveEnv<T, Env>) => CloudflareOptions | undefined, handler: T): T {
33+
O = unknown,
34+
>(optionsCallback: (env: ResolveEnv<T, Env>) => StrictCloudflareOptions<O> | undefined, handler: T): T {
3535
if (isCloudflareClass(handler, 'WorkerEntrypoint')) {
3636
// oxlint-disable-next-line typescript/no-explicit-any
3737
return instrumentWorkerEntrypoint(optionsCallback as any, handler);

packages/cloudflare/src/workflows.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import { instrumentEnv } from './instrumentations/worker/instrumentEnv';
3030
import { addCloudResourceContext } from './scope-utils';
3131
import { init } from './sdk';
3232
import { instrumentContext } from './utils/instrumentContext';
33-
import type { DefaultEnv, ResolveEnv } from './types';
33+
import type { DefaultEnv, ResolveEnv, StrictCloudflareOptions } from './types';
3434

3535
const UUID_REGEX = /^[0-9a-f]{8}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{12}$/i;
3636

@@ -213,7 +213,8 @@ export function instrumentWorkflowWithSentry<
213213
// oxlint-disable-next-line typescript/no-explicit-any
214214
env: any,
215215
) => T, // Constructor type of the WorkflowEntrypoint class
216-
>(optionsCallback: (env: ResolveEnv<C, E>) => CloudflareOptions, WorkFlowClass: C): C {
216+
O = unknown,
217+
>(optionsCallback: (env: ResolveEnv<C, E>) => StrictCloudflareOptions<O>, WorkFlowClass: C): C {
217218
return new Proxy(WorkFlowClass, {
218219
// oxlint-disable-next-line typescript/no-explicit-any
219220
construct(target: C, args: [ctx: ExecutionContext, env: any], newTarget) {
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import { DurableObject, WorkerEntrypoint, WorkflowEntrypoint } from 'cloudflare:workers';
2+
import { describe, it } from 'vitest';
3+
import type { CloudflareOptions } from '../src/client';
4+
import { defineCloudflareOptions } from '../src/defineCloudflareOptions';
5+
import { instrumentAgentWithSentry, instrumentDurableObjectWithSentry } from '../src/durableobject';
6+
import { instrumentWorkerEntrypoint } from '../src/instrumentations/instrumentWorkerEntrypoint';
7+
import { sentryPagesPlugin } from '../src/pages-plugin';
8+
import { withSentry } from '../src/withSentry';
9+
import { instrumentWorkflowWithSentry } from '../src/workflows';
10+
11+
interface TestEnv {
12+
SENTRY_DSN: string;
13+
}
14+
15+
const dsn = 'https://public@dsn.ingest.sentry.io/1337';
16+
17+
const handler = {
18+
fetch(): Response {
19+
return new Response('ok');
20+
},
21+
} satisfies ExportedHandler<TestEnv>;
22+
23+
class TestDurableObject extends DurableObject<TestEnv> {}
24+
25+
class TestWorkerEntrypoint extends WorkerEntrypoint<TestEnv> {
26+
public ping(): string {
27+
return 'pong';
28+
}
29+
}
30+
31+
class TestWorkflow extends WorkflowEntrypoint<TestEnv> {
32+
public async run(): Promise<void> {}
33+
}
34+
35+
declare const flag: boolean;
36+
declare const preTypedOptions: CloudflareOptions;
37+
declare const makeOptions: () => CloudflareOptions;
38+
39+
// The options callback returns a *fresh* object literal across a function boundary, where
40+
// TypeScript's excess property check does not reach. `StrictCloudflareOptions` restores it —
41+
// without these assertions a typo like `tracesSampleRte` silently compiles.
42+
//
43+
// Keep each asserted call on a single line: the formatter wraps longer calls and would move
44+
// the `@ts-expect-error` directive away from the line the error is reported on.
45+
describe('options are checked for unknown keys', () => {
46+
it('rejects an unknown key alongside valid ones', () => {
47+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
48+
withSentry(env => ({ dsn: env.SENTRY_DSN, wrongKey: 123 }), handler);
49+
});
50+
51+
it('rejects an options object where every key is unknown', () => {
52+
// @ts-expect-error - `tracesSampleRte` is a typo for `tracesSampleRate`
53+
withSentry(() => ({ tracesSampleRte: 1 }), handler);
54+
});
55+
56+
it('rejects a wrong value type on a known key', () => {
57+
// @ts-expect-error - `tracesSampleRate` is a number
58+
withSentry(() => ({ dsn, tracesSampleRate: 'high' }), handler);
59+
});
60+
61+
it('rejects a callback returning a function instead of options', () => {
62+
// @ts-expect-error - the options factory was returned instead of called
63+
withSentry(() => makeOptions, handler);
64+
});
65+
66+
it('rejects unknown keys in instrumentDurableObjectWithSentry', () => {
67+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
68+
instrumentDurableObjectWithSentry(() => ({ dsn, wrongKey: 1 }), TestDurableObject);
69+
});
70+
71+
it('rejects unknown keys in instrumentAgentWithSentry', () => {
72+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
73+
instrumentAgentWithSentry(() => ({ dsn, wrongKey: 1 }), TestDurableObject);
74+
});
75+
76+
it('rejects unknown keys in instrumentWorkerEntrypoint', () => {
77+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
78+
instrumentWorkerEntrypoint(() => ({ dsn, wrongKey: 1 }), TestWorkerEntrypoint);
79+
});
80+
81+
it('rejects unknown keys in instrumentWorkflowWithSentry', () => {
82+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
83+
instrumentWorkflowWithSentry(() => ({ dsn, wrongKey: 1 }), TestWorkflow);
84+
});
85+
86+
it('rejects unknown keys in defineCloudflareOptions', () => {
87+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
88+
defineCloudflareOptions(() => ({ dsn, wrongKey: 1 }));
89+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
90+
defineCloudflareOptions({ dsn, wrongKey: 1 });
91+
});
92+
93+
it('rejects unknown keys in sentryPagesPlugin', () => {
94+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
95+
sentryPagesPlugin(() => ({ dsn, wrongKey: 1 }));
96+
// @ts-expect-error - `wrongKey` is not a CloudflareOptions key
97+
sentryPagesPlugin({ dsn, wrongKey: 1 });
98+
});
99+
});
100+
101+
describe('valid options keep compiling', () => {
102+
it('accepts known keys, including Cloudflare-specific ones', () => {
103+
withSentry(
104+
env => ({
105+
dsn: env.SENTRY_DSN,
106+
tracesSampleRate: 1,
107+
serverName: 'my-worker',
108+
enableRpcTracePropagation: false,
109+
durableObjectSqlSpanAllowlist: ['cf_my_table', /^cf_reports_/],
110+
beforeSend: event => event,
111+
integrations: [],
112+
}),
113+
handler,
114+
);
115+
});
116+
117+
it('accepts arbitrary keys under `_experiments`', () => {
118+
withSentry(() => ({ _experiments: { someExperimentalFlag: true } }), handler);
119+
});
120+
121+
it('accepts an undefined return, conditional or not', () => {
122+
withSentry(() => undefined, handler);
123+
withSentry(() => (flag ? { dsn } : undefined), handler);
124+
});
125+
126+
it('accepts a pre-typed options object and spreads of it', () => {
127+
withSentry(() => preTypedOptions, handler);
128+
withSentry(() => ({ ...preTypedOptions, dsn }), handler);
129+
});
130+
131+
it('still infers the env from the handler', () => {
132+
withSentry(env => {
133+
const envDsn: string = env.SENTRY_DSN;
134+
return { dsn: envDsn };
135+
}, handler);
136+
});
137+
});

0 commit comments

Comments
 (0)