Skip to content

Commit e2c327d

Browse files
isaacsclaude
andauthored
ref(deno): Use a shared request handler wrapper for Deno.serve
Deno.serve, Bun.serve and Cloudflare each wrap a web `Request` handler with their own copy of the same steps: request data and body, the `http.server` span, streamed responses, errors and sessions. The copies drift, so a fix in one runtime does not reach the others. This adds `wrapServerRequestHandler` to `@sentry/server-utils` and moves `Deno.serve` onto it. Bun.serve and Cloudflare follow in separate PRs, so each runtime's changes can be reviewed alone. The wrapper has only what Deno needs today, and the next PRs extend it. It is exported from the shared entry, so the Node-free `@sentry/server-utils/no-diagnostic-channels` entry can use it too. The rules follow core's `node:http` server code. `OPTIONS` and `HEAD` requests get no span and no body capture, but still get request data, a session and error capture. The static asset filter applies only to `GET`. For streamed responses, the wrapper uses the stream monitor from the Bun SDK, which reads only when the client pulls and reports errors from the stream source. It replaces Deno's own copy, which read ahead of the client and ignored source errors. The existing Deno tests pass with no change. Visible changes for Deno: - An error from the source of a streamed response is now captured, and the span gets an error status. - The SDK no longer reads a streamed response ahead of the client. - Errors in `OPTIONS` and `HEAD` requests now carry the request data. - `client.address` is no longer set to the path of a Unix socket. Part of #25577 Part of JS-3975 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent 1ec6d31 commit e2c327d

10 files changed

Lines changed: 632 additions & 407 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ Work in this release was contributed by @adityareddy-dev, @jpdefo, @Jxxunnn, @ph
9696

9797
When an instrumented package loaded after `init()` (for example through a dynamic `import()`), the SDK added its integration even if the user had removed it. This applied to an `integrations` function that filtered it out, `defaultIntegrations: false`, and a custom `defaultIntegrations` list. The SDK now applies the same rules to a late-loaded integration as to the defaults at `init()`. To do this, it calls an `integrations` function again, once for each package that loads after `init()`.
9898

99+
- **ref(deno): Use the shared request handler wrapper for `Deno.serve`**
100+
101+
`Deno.serve` now uses a request handler wrapper that `Bun.serve` and Cloudflare will also use. Three small changes for Deno users:
102+
103+
- When the source of a streamed response fails, the SDK now captures the error and marks the span as failed.
104+
- The SDK no longer reads a streamed response ahead of the client.
105+
- Errors in `OPTIONS` and `HEAD` requests now carry the request data.
106+
99107
## 11.8.0
100108

101109
### Important Changes

‎packages/deno/src/utils/streaming.ts‎

Lines changed: 0 additions & 72 deletions
This file was deleted.
Lines changed: 15 additions & 208 deletions
Original file line numberDiff line numberDiff line change
@@ -1,87 +1,14 @@
1-
import {
2-
CLIENT_ADDRESS,
3-
CLIENT_PORT,
4-
NETWORK_PROTOCOL_NAME,
5-
SENTRY_OP,
6-
SENTRY_SEGMENT_NAME_SOURCE,
7-
} from '@sentry/conventions/attributes';
8-
import { HTTP_SERVER } from '@sentry/conventions/op';
9-
import type { Client, Integration } from '@sentry/core';
10-
import {
11-
captureBodyFromWinterCGRequest,
12-
captureException,
13-
continueTrace,
14-
debug,
15-
getClient,
16-
getHttpSpanDetailsFromUrlObject,
17-
hasSpansEnabled,
18-
hasSpanStreamingEnabled,
19-
httpHeadersToSpanAttributes,
20-
HTTP_SPAN_NAME_FALLBACK,
21-
parseStringToURLObject,
22-
setHttpStatus,
23-
startSpanManual,
24-
winterCGHeadersToDict,
25-
winterCGRequestToRequestData,
26-
withIsolationScope,
27-
} from '@sentry/core';
28-
import { getClientIPAddress, isStaticAssetRequest, startRequestSession } from '@sentry/core/server';
1+
import type { Integration } from '@sentry/core';
2+
import { debug, getClient, withIsolationScope } from '@sentry/core';
3+
import { wrapServerRequestHandler } from '@sentry/server-utils';
294
import type { DenoServeIntegrationOptions } from './integrations/deno-serve';
30-
import { streamResponse } from './utils/streaming';
315

326
export type RequestHandlerWrapperOptions<Addr extends Deno.Addr> = {
337
request: Request;
348
info: Deno.ServeHandlerInfo<Addr>;
359
serveOptions?: Deno.ServeOptions<Addr>;
3610
};
3711

38-
function captureServeError(e: unknown): void {
39-
captureException(e, {
40-
mechanism: {
41-
handled: false,
42-
type: 'auto.http.deno',
43-
data: { function: 'serve' },
44-
},
45-
});
46-
}
47-
48-
// A user callback that throws must not fail the request or leave the span open.
49-
function callUserCallback<T>(name: string, callback: () => T, fallback: T): T {
50-
try {
51-
return callback();
52-
} catch (e) {
53-
debug.error(`Error in the Deno.serve ${name} callback.`, e);
54-
return fallback;
55-
}
56-
}
57-
58-
function shouldCreateSpan(
59-
client: Client,
60-
integration: DenoServeIntegrationOptions | undefined,
61-
request: Request,
62-
urlPath: string,
63-
): boolean {
64-
if (!(integration?.spans ?? hasSpansEnabled(client.getOptions()))) {
65-
return false;
66-
}
67-
if (integration?.ignoreStaticAssets && request.method === 'GET' && isStaticAssetRequest(urlPath)) {
68-
return false;
69-
}
70-
const ignoreIncomingRequests = integration?.ignoreIncomingRequests;
71-
return !(
72-
ignoreIncomingRequests &&
73-
callUserCallback('ignoreIncomingRequests', () => ignoreIncomingRequests(urlPath, request), false)
74-
);
75-
}
76-
77-
const assignIfSet = <T extends Record<string, unknown>, K extends keyof T>(
78-
obj: T,
79-
key: K,
80-
value: T[K] | undefined | null,
81-
): void => {
82-
if (value !== undefined && value !== null) obj[key] = value;
83-
};
84-
8512
export const wrapDenoRequestHandler = <Addr extends Deno.Addr = Deno.Addr>(
8613
wrapperOptions: RequestHandlerWrapperOptions<Addr>,
8714
handler: () => Promise<Response> | Response,
@@ -100,140 +27,20 @@ export const wrapDenoRequestHandler = <Addr extends Deno.Addr = Deno.Addr>(
10027
}
10128
isolationScope.setClient(client);
10229

103-
const integration = client.getIntegrationByName<Integration & DenoServeIntegrationOptions>('DenoServe');
104-
const endSession = integration?.sessions
105-
? startRequestSession(client, {
106-
requestIsolationScope: isolationScope,
107-
sessionFlushingDelayMS: integration.sessionFlushingDelayMS,
108-
})
109-
: undefined;
110-
111-
if (request.method === 'OPTIONS' || request.method === 'HEAD') {
112-
try {
113-
return await handler();
114-
} catch (e) {
115-
captureServeError(e);
116-
throw e;
117-
} finally {
118-
endSession?.();
119-
}
120-
}
121-
122-
const urlObject = parseStringToURLObject(request.url);
123-
const [rawName, attributes] = getHttpSpanDetailsFromUrlObject(
124-
urlObject,
125-
'server',
126-
'auto.http.deno',
127-
request,
128-
undefined,
129-
client,
130-
);
131-
// With span streaming, span names have to be low cardinality, so we can't fall back to the URL.
132-
// A `route` source means the name already is (e.g. the `/` path), so it is kept as-is.
133-
const name =
134-
attributes[SENTRY_SEGMENT_NAME_SOURCE] === 'route' || !hasSpanStreamingEnabled(client)
135-
? rawName
136-
: request.method?.toUpperCase() || HTTP_SPAN_NAME_FALLBACK;
137-
138-
const contentLength = request.headers.get('content-length');
139-
assignIfSet(attributes, 'http.request.body.size', contentLength && parseInt(contentLength, 10));
140-
assignIfSet(attributes, 'user_agent.original', request.headers.get('user-agent'));
141-
142-
const headers = winterCGHeadersToDict(request.headers);
143-
const socketIp = (info?.remoteAddr as Deno.NetAddr)?.hostname;
144-
const dataCollection = client.getDataCollectionOptions();
145-
if (dataCollection.userInfo) {
146-
// `client.address` is the originating client, so a forwarding header wins over the socket, which
147-
// behind a proxy holds the proxy's address. The socket port is the proxy's too, so `client.port`
148-
// stays unset then.
149-
const forwardedAddress = getClientIPAddress(headers);
150-
assignIfSet(
151-
attributes,
152-
CLIENT_ADDRESS,
153-
forwardedAddress || socketIp || (info?.remoteAddr as Deno.UnixAddr)?.path,
154-
);
155-
if (!forwardedAddress) {
156-
assignIfSet(attributes, CLIENT_PORT, (info?.remoteAddr as Deno.NetAddr)?.port);
157-
}
158-
}
159-
160-
// describes the OSI application-layer protocol (http), not the scheme (might be https)
161-
attributes[NETWORK_PROTOCOL_NAME] = 'http';
162-
163-
Object.assign(attributes, httpHeadersToSpanAttributes(headers, dataCollection));
164-
attributes[SENTRY_OP] = HTTP_SERVER;
165-
isolationScope.setSDKProcessingMetadata({
166-
normalizedRequest: winterCGRequestToRequestData(request),
167-
ipAddress: socketIp,
168-
});
169-
170-
const effectiveBodySize =
171-
integration?.maxRequestBodySize ?? (dataCollection.httpBodies.includes('incomingRequest') ? 'medium' : 'none');
172-
const ignoreRequestBody = integration?.ignoreRequestBody;
173-
if (
174-
request.method !== 'GET' &&
175-
effectiveBodySize !== 'none' &&
176-
// If the filter throws, skip the body so it cannot leak data the filter was meant to hide.
177-
!(ignoreRequestBody && callUserCallback('ignoreRequestBody', () => ignoreRequestBody(request.url, request), true))
178-
) {
179-
await captureBodyFromWinterCGRequest(request, isolationScope, effectiveBodySize);
180-
}
181-
182-
const urlPath = urlObject ? `${urlObject.pathname}${urlObject.search}` : '/';
183-
const createSpan = shouldCreateSpan(client, integration, request, urlPath);
184-
185-
return continueTrace(
30+
return wrapServerRequestHandler(
18631
{
187-
sentryTrace: request.headers.get('sentry-trace') || '',
188-
baggage: request.headers.get('baggage'),
189-
},
190-
async () => {
191-
if (!createSpan) {
192-
let res: Response;
193-
try {
194-
res = await handler();
195-
isolationScope.setContext('response', {
196-
status_code: res.status,
197-
});
198-
} catch (e) {
199-
captureServeError(e);
200-
endSession?.();
201-
throw e;
202-
}
203-
return endSession ? streamResponse(undefined, res, endSession) : res;
204-
}
205-
206-
return startSpanManual({ name, attributes }, async span => {
207-
const { onSpanCreated, onSpanEnd } = integration ?? {};
208-
if (onSpanCreated) {
209-
callUserCallback('onSpanCreated', () => onSpanCreated(span, request), undefined);
210-
}
211-
let res: Response;
212-
213-
try {
214-
res = await handler();
215-
setHttpStatus(span, res.status);
216-
isolationScope.setContext('response', {
217-
status_code: res.status,
218-
});
219-
span.setAttributes(
220-
httpHeadersToSpanAttributes(Object.fromEntries(res.headers), dataCollection, 'response'),
221-
);
222-
} catch (e) {
223-
span.end();
224-
captureServeError(e);
225-
endSession?.();
226-
throw e;
227-
}
228-
229-
return streamResponse(span, res, () => {
230-
if (onSpanEnd) {
231-
callUserCallback('onSpanEnd', () => onSpanEnd(span, request, res), undefined);
232-
}
233-
endSession?.();
234-
});
235-
});
32+
request,
33+
client,
34+
isolationScope,
35+
options: client.getIntegrationByName<Integration & DenoServeIntegrationOptions>('DenoServe'),
36+
origin: 'auto.http.deno',
37+
mechanism: { handled: false, type: 'auto.http.deno', data: { function: 'serve' } },
38+
getSocketAddress: () => {
39+
const remoteAddr = info?.remoteAddr as Deno.NetAddr | undefined;
40+
return remoteAddr?.hostname ? { address: remoteAddr.hostname, port: remoteAddr.port } : undefined;
41+
},
23642
},
43+
handler,
23744
);
23845
});
23946
};

‎packages/deno/test/deno-serve.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,6 +866,46 @@ Deno.test('Deno.serve should handle streaming responses', async () => {
866866
assertEquals(transactionEvents[0]?.contexts?.trace?.op, 'http.server');
867867
});
868868

869+
Deno.test('Deno.serve should capture an error from a streamed response source', async () => {
870+
resetGlobals();
871+
const errorEvents: ErrorEvent[] = [];
872+
const transactionEvents: TransactionEvent[] = [];
873+
874+
const client = init({
875+
dsn: 'https://username@domain/123',
876+
tracesSampleRate: 1,
877+
traceLifecycle: 'static',
878+
beforeSend: (event: ErrorEvent) => {
879+
errorEvents.push(event);
880+
return null;
881+
},
882+
beforeSendTransaction: (event: TransactionEvent) => {
883+
transactionEvents.push(event);
884+
return null;
885+
},
886+
}) as DenoClient;
887+
888+
const server = await serveForTest(() => {
889+
const stream = new ReadableStream({
890+
start(controller) {
891+
controller.enqueue(new TextEncoder().encode('chunk1\n'));
892+
setTimeout(() => controller.error(new Error('stream interrupted')), 10);
893+
},
894+
});
895+
return new Response(stream, { headers: { 'content-type': 'text/event-stream' } });
896+
});
897+
898+
const response = await fetch(`${server.url}/stream`);
899+
await response.text().catch(() => undefined);
900+
await server.close();
901+
await client.flush();
902+
903+
assertEquals(errorEvents.length, 1);
904+
assertEquals(errorEvents[0]?.exception?.values?.[0]?.value, 'stream interrupted');
905+
assertEquals(errorEvents[0]?.exception?.values?.[0]?.mechanism?.type, 'auto.http.deno');
906+
assertEquals(transactionEvents[0]?.contexts?.trace?.status, 'internal_error');
907+
});
908+
869909
Deno.test('Deno.serve should work when manually capturing exceptions within handler', async () => {
870910
resetGlobals();
871911
const errorEvents: ErrorEvent[] = [];

0 commit comments

Comments
 (0)