Skip to content

Commit 685d415

Browse files
isaacsclaude
andcommitted
ref(core): Add shared ServerRequestOptions type across runtimes
Node, Deno, Bun and Cloudflare each have their own options for incoming HTTP requests, and the option sets drift. Bun.serve takes only `maxRequestBodySize`, and Cloudflare has no span options. When we add an option to one runtime, nothing reminds us to add it to the others. `ServerRequestOptions` in `@sentry/core/server` lists the options that every incoming request integration should support. Each runtime options type is checked against it with `CheckServerRequestOptions`. The check lists the options that the runtime does not support yet, with a comment that says why. It fails to compile when a shared option is neither declared nor listed, when a listed option is declared, or when a callback written for the shared type does not fit the runtime option. The runtime types do not extend the shared type. With `extends`, a new shared option would appear in every runtime type, supported or not, and nothing would fail to compile. Each runtime keeps its own declarations and docs. Node and Bun now type the request in `ignoreRequestBody` as `RequestOptions & HttpIncomingMessage`. The value was always an `IncomingMessage`. Callbacks typed with `RequestOptions` still compile. The `RequestOptions` part is deprecated, for removal in v12. A new BUGBOT rule flags an incoming request option that applies to more than one runtime but is not in the shared type. There is no runtime behavior change. Fixes JS-3968 Fixes #25570 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 3aafe9e commit 685d415

10 files changed

Lines changed: 240 additions & 12 deletions

File tree

‎.cursor/BUGBOT.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu
7878
- What to do instead: Use an existing reference to the scope or client. For example, this is possible in most `Integration` hooks.
7979
- Flag hard-coded attribute names and span ops (e.g. `'sentry.origin'`, `'sentry.op'`, `'http.request.method'`, `'db.system.name'`) when `@sentry/conventions` exports a constant for them. Use the constant from `@sentry/conventions/attributes` (e.g. `SENTRY_ORIGIN`, `SENTRY_OP`) or `@sentry/conventions/op` instead. Do not flag values that have no constant in `@sentry/conventions`.
8080
- Flag unnecessary `span.setAttribute(s)` calls: If data is already available at span start, it must be set via the `attributes` option of `startSpan`, `startSpanManual`, `startInactiveSpan` or `startIdleSpan` calls. This ensures that as much context as possible is available when `tracesSampler` or `ignoreSpans` SDK options are applied. If a `span.setAttribute(s)` call happens at a later time than right after span start and the attribute value can only be computed at that time, do not flag it.
81+
- Flag a new option on an integration for incoming HTTP requests that is not in `ServerRequestOptions` (`packages/core/src/integrations/http/server-request-options.ts`), when it applies to more than one runtime. These integrations are Node `httpIntegration`, Deno `denoHttpIntegration` and `denoServeIntegration`, Bun `bunHttpServerIntegration` and `bunServerIntegration`, and Cloudflare `httpServerIntegration`. Add the option to `ServerRequestOptions`, then support it in each runtime or list it as unsupported in that runtime's `CheckServerRequestOptions` check. Also flag a runtime option that matches a shared option in purpose but has a different name or meaning.
8182

8283
### Code quality
8384

‎packages/bun/src/integrations/bunHttpServer.ts‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,12 @@ import * as http from 'node:http';
33
import * as https from 'node:https';
44
import type { IntegrationFn, Span } from '@sentry/core';
55
import { defineIntegration } from '@sentry/core';
6-
import type { HttpIncomingMessage, HttpServerResponse } from '@sentry/core/server';
6+
import type {
7+
AssertTrue,
8+
CheckServerRequestOptions,
9+
HttpIncomingMessage,
10+
HttpServerResponse,
11+
} from '@sentry/core/server';
712
import { getHttpServerSubscriptions, HTTP_ON_SERVER_REQUEST } from '@sentry/core/server';
813

914
const INTEGRATION_NAME = 'BunHttpServer' as const;
@@ -37,8 +42,12 @@ interface BunHttpServerOptions {
3742

3843
/**
3944
* Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`.
45+
*
46+
* The `request` parameter is the incoming `node:http` `IncomingMessage`. The `RequestOptions` part of the type is
47+
* deprecated and will be removed in v12. Its fields that `IncomingMessage` does not have (such as `path` or
48+
* `hostname`) are always `undefined`.
4049
*/
41-
ignoreRequestBody?: (url: string, request: http.RequestOptions) => boolean;
50+
ignoreRequestBody?: (url: string, request: http.RequestOptions & HttpIncomingMessage) => boolean;
4251

4352
/**
4453
* Controls the maximum size of incoming HTTP request bodies attached to events.
@@ -73,6 +82,13 @@ interface BunHttpServerOptions {
7382
onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
7483
}
7584

85+
// Checks these options against the shared `ServerRequestOptions`. A new
86+
// shared field fails to compile here, until this integration supports
87+
// it or lists it as unsupported.
88+
type _ServerRequestOptionsCheck = AssertTrue<
89+
CheckServerRequestOptions<BunHttpServerOptions, HttpIncomingMessage, HttpServerResponse>
90+
>;
91+
7692
let hasPatched = false;
7793

7894
const _bunHttpServerIntegration = ((options: BunHttpServerOptions = {}) => {

‎packages/bun/src/integrations/bunserver.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable max-lines */
12
import type { Integration, IntegrationFn, MaxRequestBodySize, SpanAttributes } from '@sentry/core';
23
import {
34
captureBodyFromWinterCGRequest,
@@ -20,6 +21,7 @@ import {
2021
filterCollectedUrl,
2122
filterCollectedUrlQuery,
2223
} from '@sentry/core';
24+
import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server';
2325
import { classifyResponseStreaming, getClientIPAddress } from '@sentry/core/server';
2426
import type { Server, ServeOptions } from 'bun';
2527
import {
@@ -55,6 +57,26 @@ export type BunServerIntegrationOptions = {
5557
maxRequestBodySize?: MaxRequestBodySize;
5658
};
5759

60+
// Checks these options against the shared `ServerRequestOptions`. A new
61+
// shared field fails to compile here, until this integration supports
62+
// it or lists it as unsupported.
63+
type _ServerRequestOptionsCheck = AssertTrue<
64+
CheckServerRequestOptions<
65+
BunServerIntegrationOptions,
66+
Request,
67+
Response,
68+
// Not supported yet.
69+
| 'spans'
70+
| 'sessions'
71+
| 'sessionFlushingDelayMS'
72+
| 'ignoreRequestBody'
73+
| 'ignoreIncomingRequests'
74+
| 'ignoreStaticAssets'
75+
| 'onSpanCreated'
76+
| 'onSpanEnd'
77+
>
78+
>;
79+
5880
const _bunServerIntegration = ((options: BunServerIntegrationOptions = {}) => {
5981
return {
6082
name: INTEGRATION_NAME,

‎packages/cloudflare/src/integrations/httpServer.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Client, IntegrationFn, MaxRequestBodySize } from '@sentry/core';
22
import { captureBodyFromWinterCGRequest, defineIntegration, getIsolationScope } from '@sentry/core';
3+
import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server';
34

45
const INTEGRATION_NAME = 'HttpServer' as const;
56

@@ -40,6 +41,27 @@ export interface HttpServerIntegrationOptions {
4041
ignoreRequestBody?: (url: string, request: Request) => boolean;
4142
}
4243

44+
// Checks these options against the shared `ServerRequestOptions`. A new
45+
// shared field fails to compile here, until this integration supports
46+
// it or lists it as unsupported.
47+
type _ServerRequestOptionsCheck = AssertTrue<
48+
CheckServerRequestOptions<
49+
HttpServerIntegrationOptions,
50+
Request,
51+
Response,
52+
// Not supported yet.
53+
| 'spans'
54+
| 'ignoreIncomingRequests'
55+
| 'ignoreStaticAssets'
56+
| 'onSpanCreated'
57+
| 'onSpanEnd'
58+
// Not supported. A worker has no long-lived process to run the timer
59+
// that flushes session aggregates.
60+
| 'sessions'
61+
| 'sessionFlushingDelayMS'
62+
>
63+
>;
64+
4365
interface HttpServerIntegrationInstance {
4466
name: string;
4567
maxRequestBodySize: MaxRequestBodySize | undefined;
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import type { Span } from '../../types/span';
2+
import type { MaxRequestBodySize } from '../../utils/request';
3+
4+
/**
5+
* Options that every integration for incoming HTTP requests should
6+
* support, in each runtime.
7+
*
8+
* `TRequest` and `TResponse` are the request and response types that the
9+
* runtime passes to callbacks: `HttpIncomingMessage` and
10+
* `HttpServerResponse` for `node:http`, or the web `Request` and
11+
* `Response` for `Deno.serve`, `Bun.serve` and Cloudflare.
12+
*
13+
* Runtime options types do not extend this type. Each one declares its
14+
* own fields, with docs for that runtime, and uses
15+
* {@link CheckServerRequestOptions} to check them against this type.
16+
* Then a new field here fails to compile in each runtime, until the
17+
* runtime supports it or lists it as unsupported.
18+
*/
19+
export interface ServerRequestOptions<TRequest, TResponse> {
20+
/**
21+
* Whether to create spans for incoming requests.
22+
* Defaults to the client's tracing configuration.
23+
*/
24+
spans?: boolean;
25+
26+
/**
27+
* Whether to create [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for incoming
28+
* requests, to track the health and crash-free rate of releases.
29+
*
30+
* @default `true`
31+
*/
32+
sessions?: boolean;
33+
34+
/**
35+
* Number of milliseconds until sessions are flushed as a session aggregate.
36+
*
37+
* @default `60000` (60s)
38+
*/
39+
sessionFlushingDelayMS?: number;
40+
41+
/**
42+
* Controls the maximum size of incoming request bodies attached to events.
43+
*
44+
* @default 'medium'
45+
*/
46+
maxRequestBodySize?: MaxRequestBodySize;
47+
48+
/**
49+
* Do not capture the request body for incoming requests where the given callback returns `true`.
50+
*
51+
* `url` is the full URL of the request, including the query string.
52+
*/
53+
ignoreRequestBody?: (url: string, request: TRequest) => boolean;
54+
55+
/**
56+
* Do not create spans for incoming requests where the given callback returns `true`.
57+
*
58+
* `urlPath` is the path of the request, including the query string.
59+
*/
60+
ignoreIncomingRequests?: (urlPath: string, request: TRequest) => boolean;
61+
62+
/**
63+
* Whether to ignore common static asset requests (favicon.ico, robots.txt, etc.)
64+
* when creating spans.
65+
*
66+
* @default `true`
67+
*/
68+
ignoreStaticAssets?: boolean;
69+
70+
/**
71+
* A hook to change the span for an incoming request, after the span is created.
72+
*/
73+
onSpanCreated?: (span: Span, request: TRequest) => void;
74+
75+
/**
76+
* A hook to change the span for an incoming request one last time, before it ends.
77+
*/
78+
onSpanEnd?: (span: Span, request: TRequest, response: TResponse) => void;
79+
}
80+
81+
/**
82+
* `true` when `TOptions` matches {@link ServerRequestOptions}, else a
83+
* string that names the problem. Use it with a type that accepts only
84+
* `true`, so a mismatch fails to compile:
85+
*
86+
* ```ts
87+
* type _Check = AssertTrue<CheckServerRequestOptions<MyOptions, Request, Response, 'sessions'>>;
88+
* ```
89+
*
90+
* `TUnsupported` lists the fields that the runtime does not support.
91+
*
92+
* The check fails when:
93+
* - `TOptions` does not declare a field, and `TUnsupported` does not
94+
* list it.
95+
* - `TUnsupported` lists a field that `TOptions` declares.
96+
* - A callback written for the shared type does not fit the field in
97+
* `TOptions`.
98+
*/
99+
export type CheckServerRequestOptions<
100+
TOptions,
101+
TRequest,
102+
TResponse,
103+
TUnsupported extends keyof ServerRequestOptions<TRequest, TResponse> = never,
104+
> = [MissingFields<TOptions, TRequest, TResponse, TUnsupported>] extends [never]
105+
? [Extract<TUnsupported, keyof TOptions>] extends [never]
106+
? SupportedFields<
107+
ServerRequestOptions<TRequest, TResponse>,
108+
TRequest,
109+
TResponse,
110+
TUnsupported
111+
> extends SupportedFields<TOptions, TRequest, TResponse, TUnsupported>
112+
? true
113+
: 'A field type does not match ServerRequestOptions'
114+
: `Listed as unsupported, but declared: ${Extract<TUnsupported, keyof TOptions> & string}`
115+
: `Missing field: ${MissingFields<TOptions, TRequest, TResponse, TUnsupported> & string}`;
116+
117+
type MissingFields<TOptions, TRequest, TResponse, TUnsupported> = Exclude<
118+
keyof ServerRequestOptions<TRequest, TResponse>,
119+
TUnsupported | keyof TOptions
120+
>;
121+
122+
type SupportedFields<T, TRequest, TResponse, TUnsupported> = Pick<
123+
T,
124+
Extract<keyof T, Exclude<keyof ServerRequestOptions<TRequest, TResponse>, TUnsupported>>
125+
>;
126+
127+
/** Accepts only `true`. Use it with {@link CheckServerRequestOptions}. */
128+
export type AssertTrue<T extends true> = T;

‎packages/core/src/server.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ export type {
4343
HttpServerResponse,
4444
HttpModuleExport,
4545
} from './integrations/http/types';
46+
export type {
47+
AssertTrue,
48+
CheckServerRequestOptions,
49+
ServerRequestOptions,
50+
} from './integrations/http/server-request-options';
4651
export { createFetchIntegration } from './integrations/fetch';
4752
export type { FetchIntegrationOptions } from './integrations/fetch';
4853
export { classifyResponseStreaming } from './utils/responseStreaming';

‎packages/deno/src/integrations/deno-serve.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Event, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core';
22
import { debug, defineIntegration } from '@sentry/core';
3+
import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server';
34
import { DEFAULT_IGNORE_STATUS_CODES, processHttpServerTransactionEvent } from '@sentry/core/server';
45
import type { RequestHandlerWrapperOptions } from '../wrap-deno-request-handler';
56
import { wrapDenoRequestHandler } from '../wrap-deno-request-handler';
@@ -99,6 +100,11 @@ export type DenoServeIntegrationOptions = {
99100
ignoreStatusCodes?: (number | [number, number])[];
100101
};
101102

103+
// Checks these options against the shared `ServerRequestOptions`. A new
104+
// shared field fails to compile here, until this integration supports
105+
// it or lists it as unsupported.
106+
type _ServerRequestOptionsCheck = AssertTrue<CheckServerRequestOptions<DenoServeIntegrationOptions, Request, Response>>;
107+
102108
export type ServeParams =
103109
// [(Request) => Response]
104110
| [Deno.ServeHandler<Deno.NetAddr>]

‎packages/deno/src/integrations/http.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@ import { errorMonitor } from 'node:events';
33
import type { RequestOptions } from 'node:http';
44
import type { Event, Integration, IntegrationFn, Span } from '@sentry/core';
55
import { defineIntegration } from '@sentry/core';
6-
import type { HttpClientRequest, HttpIncomingMessage, HttpServerResponse } from '@sentry/core/server';
6+
import type {
7+
AssertTrue,
8+
CheckServerRequestOptions,
9+
HttpClientRequest,
10+
HttpIncomingMessage,
11+
HttpServerResponse,
12+
} from '@sentry/core/server';
713
import {
814
DEFAULT_IGNORE_STATUS_CODES,
915
getHttpClientSubscriptions,
@@ -151,6 +157,13 @@ export interface DenoHttpIntegrationOptions {
151157
onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void;
152158
}
153159

160+
// Checks these options against the shared `ServerRequestOptions`. A new
161+
// shared field fails to compile here, until this integration supports
162+
// it or lists it as unsupported.
163+
type _ServerRequestOptionsCheck = AssertTrue<
164+
CheckServerRequestOptions<DenoHttpIntegrationOptions, HttpIncomingMessage, HttpServerResponse>
165+
>;
166+
154167
const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => {
155168
const breadcrumbs = options.breadcrumbs ?? true;
156169
const tracePropagation = options.tracePropagation ?? true;

‎packages/node/src/integrations/http/httpServerIntegration.ts‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,11 @@ export interface HttpServerIntegrationOptions {
4545
* This can be useful for long running requests where the body is not needed and we want to avoid capturing it.
4646
*
4747
* @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the incoming request.
48-
* @param request Contains the {@type RequestOptions} object used to make the incoming request.
48+
* @param request The incoming request, a `node:http` `IncomingMessage`. Use `request.url`, `request.method`,
49+
* `request.headers`, etc. The `RequestOptions` part of the type is deprecated and will be removed in v12. Its
50+
* fields that `IncomingMessage` does not have (such as `path` or `hostname`) are always `undefined`.
4951
*/
50-
ignoreRequestBody?: (url: string, request: RequestOptions) => boolean;
52+
ignoreRequestBody?: (url: string, request: RequestOptions & HttpIncomingMessage) => boolean;
5153

5254
/**
5355
* Controls the maximum size of incoming HTTP request bodies attached to events.
@@ -84,13 +86,7 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) =>
8486
// core subscription helper. Explicitly opt out so the helper does not
8587
// double-create spans when the client has tracing enabled.
8688
spans: false as const,
87-
// Cast: core uses HttpIncomingMessage; node consumers pass
88-
// RequestOptions-typed callbacks.
89-
// The two are structurally compatible for the fields the callback reads
90-
// (url, method, headers).
91-
ignoreRequestBody: options.ignoreRequestBody as
92-
| ((url: string, request: HttpIncomingMessage) => boolean)
93-
| undefined,
89+
ignoreRequestBody: options.ignoreRequestBody,
9490

9591
/**
9692
* Hook called by core's `instrumentServer` to wrap the upstream

‎packages/node/src/integrations/http/index.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@ import {
88
hasSpanStreamingEnabled,
99
stripDataUrlContent,
1010
} from '@sentry/core';
11+
import type {
12+
AssertTrue,
13+
CheckServerRequestOptions,
14+
HttpIncomingMessage,
15+
HttpServerResponse,
16+
} from '@sentry/core/server';
1117
import { getRequestUrlFromClientRequest } from '@sentry/core/server';
1218
import type { NodeClient } from '../../sdk/client';
1319
import type { HttpServerIntegrationOptions } from './httpServerIntegration';
@@ -83,6 +89,19 @@ interface HttpOptions extends HttpServerIntegrationOptions, HttpServerSpansInteg
8389
outgoingRequestApplyCustomAttributes?: OutgoingHttpRequestInstrumentationOptions['outgoingRequestApplyCustomAttributes'];
8490
}
8591

92+
// Checks these options against the shared `ServerRequestOptions`. A new
93+
// shared field fails to compile here, until this integration supports
94+
// it or lists it as unsupported.
95+
type _ServerRequestOptionsCheck = AssertTrue<
96+
CheckServerRequestOptions<
97+
HttpOptions,
98+
HttpIncomingMessage,
99+
HttpServerResponse,
100+
// Not supported yet.
101+
'onSpanEnd'
102+
>
103+
>;
104+
86105
/**
87106
* The http integration instruments Node's internal http and https modules.
88107
* It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span.

0 commit comments

Comments
 (0)