From eeb2691d022b7e7f82183397ced9283e0f30c291 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 20 Oct 2026 13:39:18 -0700 Subject: [PATCH] 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) --- .cursor/BUGBOT.md | 1 + .../bun/src/integrations/bunHttpServer.ts | 20 ++- packages/bun/src/integrations/bunserver.ts | 22 +++ .../cloudflare/src/integrations/httpServer.ts | 22 +++ .../http/server-request-options.ts | 128 ++++++++++++++++++ packages/core/src/server.ts | 5 + packages/deno/src/integrations/deno-serve.ts | 6 + packages/deno/src/integrations/http.ts | 15 +- .../http/httpServerIntegration.ts | 14 +- packages/node/src/integrations/http/index.ts | 19 +++ 10 files changed, 240 insertions(+), 12 deletions(-) create mode 100644 packages/core/src/integrations/http/server-request-options.ts diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index 8212b1b8489e..c0db9f483eb2 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -78,6 +78,7 @@ Keep reviews high-signal. Prefer actionable, high-confidence findings over specu - What to do instead: Use an existing reference to the scope or client. For example, this is possible in most `Integration` hooks. - 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`. - 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. +- 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. ### Code quality diff --git a/packages/bun/src/integrations/bunHttpServer.ts b/packages/bun/src/integrations/bunHttpServer.ts index f21d47e93f67..bb423dc5773d 100644 --- a/packages/bun/src/integrations/bunHttpServer.ts +++ b/packages/bun/src/integrations/bunHttpServer.ts @@ -3,7 +3,12 @@ import * as http from 'node:http'; import * as https from 'node:https'; import type { IntegrationFn, Span } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; -import type { HttpIncomingMessage, HttpServerResponse } from '@sentry/core/server'; +import type { + AssertTrue, + CheckServerRequestOptions, + HttpIncomingMessage, + HttpServerResponse, +} from '@sentry/core/server'; import { getHttpServerSubscriptions, HTTP_ON_SERVER_REQUEST } from '@sentry/core/server'; const INTEGRATION_NAME = 'BunHttpServer' as const; @@ -37,8 +42,12 @@ interface BunHttpServerOptions { /** * Do not capture the request body for incoming HTTP requests to URLs where the given callback returns `true`. + * + * The `request` parameter is the incoming `node:http` `IncomingMessage`. The `RequestOptions` part of the type is + * deprecated and will be removed in v12. Its fields that `IncomingMessage` does not have (such as `path` or + * `hostname`) are always `undefined`. */ - ignoreRequestBody?: (url: string, request: http.RequestOptions) => boolean; + ignoreRequestBody?: (url: string, request: http.RequestOptions & HttpIncomingMessage) => boolean; /** * Controls the maximum size of incoming HTTP request bodies attached to events. @@ -73,6 +82,13 @@ interface BunHttpServerOptions { onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void; } +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue< + CheckServerRequestOptions +>; + let hasPatched = false; const _bunHttpServerIntegration = ((options: BunHttpServerOptions = {}) => { diff --git a/packages/bun/src/integrations/bunserver.ts b/packages/bun/src/integrations/bunserver.ts index 1bd340703aec..46ff0fc30e25 100644 --- a/packages/bun/src/integrations/bunserver.ts +++ b/packages/bun/src/integrations/bunserver.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines */ import type { Integration, IntegrationFn, MaxRequestBodySize, SpanAttributes } from '@sentry/core'; import { captureBodyFromWinterCGRequest, @@ -20,6 +21,7 @@ import { filterCollectedUrl, filterCollectedUrlQuery, } from '@sentry/core'; +import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server'; import { classifyResponseStreaming, getClientIPAddress } from '@sentry/core/server'; import type { Server, ServeOptions } from 'bun'; import { @@ -55,6 +57,26 @@ export type BunServerIntegrationOptions = { maxRequestBodySize?: MaxRequestBodySize; }; +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue< + CheckServerRequestOptions< + BunServerIntegrationOptions, + Request, + Response, + // Not supported yet. + | 'spans' + | 'sessions' + | 'sessionFlushingDelayMS' + | 'ignoreRequestBody' + | 'ignoreIncomingRequests' + | 'ignoreStaticAssets' + | 'onSpanCreated' + | 'onSpanEnd' + > +>; + const _bunServerIntegration = ((options: BunServerIntegrationOptions = {}) => { return { name: INTEGRATION_NAME, diff --git a/packages/cloudflare/src/integrations/httpServer.ts b/packages/cloudflare/src/integrations/httpServer.ts index 97e960f916d2..7fa3cd7ce30b 100644 --- a/packages/cloudflare/src/integrations/httpServer.ts +++ b/packages/cloudflare/src/integrations/httpServer.ts @@ -1,5 +1,6 @@ import type { Client, IntegrationFn, MaxRequestBodySize } from '@sentry/core'; import { captureBodyFromWinterCGRequest, defineIntegration, getIsolationScope } from '@sentry/core'; +import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server'; const INTEGRATION_NAME = 'HttpServer' as const; @@ -40,6 +41,27 @@ export interface HttpServerIntegrationOptions { ignoreRequestBody?: (url: string, request: Request) => boolean; } +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue< + CheckServerRequestOptions< + HttpServerIntegrationOptions, + Request, + Response, + // Not supported yet. + | 'spans' + | 'ignoreIncomingRequests' + | 'ignoreStaticAssets' + | 'onSpanCreated' + | 'onSpanEnd' + // Not supported. A worker has no long-lived process to run the timer + // that flushes session aggregates. + | 'sessions' + | 'sessionFlushingDelayMS' + > +>; + interface HttpServerIntegrationInstance { name: string; maxRequestBodySize: MaxRequestBodySize | undefined; diff --git a/packages/core/src/integrations/http/server-request-options.ts b/packages/core/src/integrations/http/server-request-options.ts new file mode 100644 index 000000000000..7125978ad603 --- /dev/null +++ b/packages/core/src/integrations/http/server-request-options.ts @@ -0,0 +1,128 @@ +import type { Span } from '../../types/span'; +import type { MaxRequestBodySize } from '../../utils/request'; + +/** + * Options that every integration for incoming HTTP requests should + * support, in each runtime. + * + * `TRequest` and `TResponse` are the request and response types that the + * runtime passes to callbacks: `HttpIncomingMessage` and + * `HttpServerResponse` for `node:http`, or the web `Request` and + * `Response` for `Deno.serve`, `Bun.serve` and Cloudflare. + * + * Runtime options types do not extend this type. Each one declares its + * own fields, with docs for that runtime, and uses + * {@link CheckServerRequestOptions} to check them against this type. + * Then a new field here fails to compile in each runtime, until the + * runtime supports it or lists it as unsupported. + */ +export interface ServerRequestOptions { + /** + * Whether to create spans for incoming requests. + * Defaults to the client's tracing configuration. + */ + spans?: boolean; + + /** + * Whether to create [Sessions](https://docs.sentry.io/product/releases/health/#sessions) for incoming + * requests, to track the health and crash-free rate of releases. + * + * @default `true` + */ + sessions?: boolean; + + /** + * Number of milliseconds until sessions are flushed as a session aggregate. + * + * @default `60000` (60s) + */ + sessionFlushingDelayMS?: number; + + /** + * Controls the maximum size of incoming request bodies attached to events. + * + * @default 'medium' + */ + maxRequestBodySize?: MaxRequestBodySize; + + /** + * Do not capture the request body for incoming requests where the given callback returns `true`. + * + * `url` is the full URL of the request, including the query string. + */ + ignoreRequestBody?: (url: string, request: TRequest) => boolean; + + /** + * Do not create spans for incoming requests where the given callback returns `true`. + * + * `urlPath` is the path of the request, including the query string. + */ + ignoreIncomingRequests?: (urlPath: string, request: TRequest) => boolean; + + /** + * Whether to ignore common static asset requests (favicon.ico, robots.txt, etc.) + * when creating spans. + * + * @default `true` + */ + ignoreStaticAssets?: boolean; + + /** + * A hook to change the span for an incoming request, after the span is created. + */ + onSpanCreated?: (span: Span, request: TRequest) => void; + + /** + * A hook to change the span for an incoming request one last time, before it ends. + */ + onSpanEnd?: (span: Span, request: TRequest, response: TResponse) => void; +} + +/** + * `true` when `TOptions` matches {@link ServerRequestOptions}, else a + * string that names the problem. Use it with a type that accepts only + * `true`, so a mismatch fails to compile: + * + * ```ts + * type _Check = AssertTrue>; + * ``` + * + * `TUnsupported` lists the fields that the runtime does not support. + * + * The check fails when: + * - `TOptions` does not declare a field, and `TUnsupported` does not + * list it. + * - `TUnsupported` lists a field that `TOptions` declares. + * - A callback written for the shared type does not fit the field in + * `TOptions`. + */ +export type CheckServerRequestOptions< + TOptions, + TRequest, + TResponse, + TUnsupported extends keyof ServerRequestOptions = never, +> = [MissingFields] extends [never] + ? [Extract] extends [never] + ? SupportedFields< + ServerRequestOptions, + TRequest, + TResponse, + TUnsupported + > extends SupportedFields + ? true + : 'A field type does not match ServerRequestOptions' + : `Listed as unsupported, but declared: ${Extract & string}` + : `Missing field: ${MissingFields & string}`; + +type MissingFields = Exclude< + keyof ServerRequestOptions, + TUnsupported | keyof TOptions +>; + +type SupportedFields = Pick< + T, + Extract, TUnsupported>> +>; + +/** Accepts only `true`. Use it with {@link CheckServerRequestOptions}. */ +export type AssertTrue = T; diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts index c6d78b00f52d..626858543c77 100644 --- a/packages/core/src/server.ts +++ b/packages/core/src/server.ts @@ -43,6 +43,11 @@ export type { HttpServerResponse, HttpModuleExport, } from './integrations/http/types'; +export type { + AssertTrue, + CheckServerRequestOptions, + ServerRequestOptions, +} from './integrations/http/server-request-options'; export { createFetchIntegration } from './integrations/fetch'; export type { FetchIntegrationOptions } from './integrations/fetch'; export { classifyResponseStreaming } from './utils/responseStreaming'; diff --git a/packages/deno/src/integrations/deno-serve.ts b/packages/deno/src/integrations/deno-serve.ts index 9d20e9084848..8c8a4038e5b5 100644 --- a/packages/deno/src/integrations/deno-serve.ts +++ b/packages/deno/src/integrations/deno-serve.ts @@ -1,5 +1,6 @@ import type { Event, IntegrationFn, MaxRequestBodySize, Span } from '@sentry/core'; import { debug, defineIntegration } from '@sentry/core'; +import type { AssertTrue, CheckServerRequestOptions } from '@sentry/core/server'; import { DEFAULT_IGNORE_STATUS_CODES, processHttpServerTransactionEvent } from '@sentry/core/server'; import type { RequestHandlerWrapperOptions } from '../wrap-deno-request-handler'; import { wrapDenoRequestHandler } from '../wrap-deno-request-handler'; @@ -99,6 +100,11 @@ export type DenoServeIntegrationOptions = { ignoreStatusCodes?: (number | [number, number])[]; }; +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue>; + export type ServeParams = // [(Request) => Response] | [Deno.ServeHandler] diff --git a/packages/deno/src/integrations/http.ts b/packages/deno/src/integrations/http.ts index 34dc1ff4e5f4..ea6a4fbae6c0 100644 --- a/packages/deno/src/integrations/http.ts +++ b/packages/deno/src/integrations/http.ts @@ -3,7 +3,13 @@ import { errorMonitor } from 'node:events'; import type { RequestOptions } from 'node:http'; import type { Event, Integration, IntegrationFn, Span } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; -import type { HttpClientRequest, HttpIncomingMessage, HttpServerResponse } from '@sentry/core/server'; +import type { + AssertTrue, + CheckServerRequestOptions, + HttpClientRequest, + HttpIncomingMessage, + HttpServerResponse, +} from '@sentry/core/server'; import { DEFAULT_IGNORE_STATUS_CODES, getHttpClientSubscriptions, @@ -151,6 +157,13 @@ export interface DenoHttpIntegrationOptions { onSpanEnd?: (span: Span, request: HttpIncomingMessage, response: HttpServerResponse) => void; } +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue< + CheckServerRequestOptions +>; + const _denoHttpIntegration = ((options: DenoHttpIntegrationOptions = {}) => { const breadcrumbs = options.breadcrumbs ?? true; const tracePropagation = options.tracePropagation ?? true; diff --git a/packages/node/src/integrations/http/httpServerIntegration.ts b/packages/node/src/integrations/http/httpServerIntegration.ts index 6322cd0169bd..b3c0c939d971 100644 --- a/packages/node/src/integrations/http/httpServerIntegration.ts +++ b/packages/node/src/integrations/http/httpServerIntegration.ts @@ -45,9 +45,11 @@ export interface HttpServerIntegrationOptions { * This can be useful for long running requests where the body is not needed and we want to avoid capturing it. * * @param url Contains the entire URL, including query string (if any), protocol, host, etc. of the incoming request. - * @param request Contains the {@type RequestOptions} object used to make the incoming request. + * @param request The incoming request, a `node:http` `IncomingMessage`. Use `request.url`, `request.method`, + * `request.headers`, etc. The `RequestOptions` part of the type is deprecated and will be removed in v12. Its + * fields that `IncomingMessage` does not have (such as `path` or `hostname`) are always `undefined`. */ - ignoreRequestBody?: (url: string, request: RequestOptions) => boolean; + ignoreRequestBody?: (url: string, request: RequestOptions & HttpIncomingMessage) => boolean; /** * Controls the maximum size of incoming HTTP request bodies attached to events. @@ -84,13 +86,7 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) => // core subscription helper. Explicitly opt out so the helper does not // double-create spans when the client has tracing enabled. spans: false as const, - // Cast: core uses HttpIncomingMessage; node consumers pass - // RequestOptions-typed callbacks. - // The two are structurally compatible for the fields the callback reads - // (url, method, headers). - ignoreRequestBody: options.ignoreRequestBody as - | ((url: string, request: HttpIncomingMessage) => boolean) - | undefined, + ignoreRequestBody: options.ignoreRequestBody, /** * Hook called by core's `instrumentServer` to wrap the upstream diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index f0cfc28cdc87..7431a6a34537 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -8,6 +8,12 @@ import { hasSpanStreamingEnabled, stripDataUrlContent, } from '@sentry/core'; +import type { + AssertTrue, + CheckServerRequestOptions, + HttpIncomingMessage, + HttpServerResponse, +} from '@sentry/core/server'; import { getRequestUrlFromClientRequest } from '@sentry/core/server'; import type { NodeClient } from '../../sdk/client'; import type { HttpServerIntegrationOptions } from './httpServerIntegration'; @@ -83,6 +89,19 @@ interface HttpOptions extends HttpServerIntegrationOptions, HttpServerSpansInteg outgoingRequestApplyCustomAttributes?: OutgoingHttpRequestInstrumentationOptions['outgoingRequestApplyCustomAttributes']; } +// Checks these options against the shared `ServerRequestOptions`. A new +// shared field fails to compile here, until this integration supports +// it or lists it as unsupported. +type _ServerRequestOptionsCheck = AssertTrue< + CheckServerRequestOptions< + HttpOptions, + HttpIncomingMessage, + HttpServerResponse, + // Not supported yet. + 'onSpanEnd' + > +>; + /** * The http integration instruments Node's internal http and https modules. * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span.