Skip to content

Commit a41cabf

Browse files
fix(deno): Don't throw in Deno.serve wrapper when no client is bound (#24030)
Replaces the throw with a debug.warn plus pass-through to the user's handler. Cloudflare's wrapper never blocks the request when the client is missing, this brings Deno in line. One correction to the issue's mechanism from testing it: Sentry.close() doesn't actually leave getClient() undefined, it flushes and disables but the client stays bound, and a failed second init leaves the first client bound too, so neither path hit this throw. What does reproduce it is a directly constructed client, new DenoClient plus client.init() installs the Deno.serve patch without ever calling setCurrentClient, so the patch is live with no client bound and every request 500s. Either way the wrapper shouldn't be the thing that kills the request. Trade-off stated plainly: this turns a loud failure into a silent one, the warn only shows with debug enabled, and Cloudflare's equivalent doesn't warn at all there. Two tests, own file since they wipe the carrier: a plain request comes back 200, and a throwing handler still yields a 500 rather than a crash. Both assert Deno.serve was actually patched first so they can't pass vacuously. Closes #23894
1 parent d12e188 commit a41cabf

2 files changed

Lines changed: 74 additions & 1 deletion

File tree

‎packages/deno/src/wrap-deno-request-handler.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
captureBodyFromWinterCGRequest,
1212
captureException,
1313
continueTrace,
14+
debug,
1415
getClient,
1516
getHttpSpanDetailsFromUrlObject,
1617
hasSpanStreamingEnabled,
@@ -48,7 +49,12 @@ export const wrapDenoRequestHandler = <Addr extends Deno.Addr = Deno.Addr>(
4849

4950
const client = getClient();
5051
if (!client) {
51-
throw new Error('could not get Deno client. Did you run Sentry.init?');
52+
// `denoServeIntegration` patches `Deno.serve` from `Client.init()`, which a
53+
// directly-constructed client also runs — that path never calls
54+
// `setCurrentClient`, so the patch can be live with no client bound. Keep
55+
// requests flowing to the user's handler, uninstrumented.
56+
debug.warn('Cannot instrument Deno.serve request. No client defined.');
57+
return handler();
5258
}
5359
isolationScope.setClient(client);
5460

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// <reference lib="deno.ns" />
2+
3+
/**
4+
* Lives in its own file because it wipes the global carrier; Deno gives each test
5+
* file a fresh module graph, so the wipe stays contained here.
6+
*/
7+
8+
import { getMainCarrier } from '@sentry/core';
9+
import { assertEquals } from 'https://deno.land/std@0.212.0/assert/assert_equals.ts';
10+
import { assertNotEquals } from 'https://deno.land/std@0.212.0/assert/assert_not_equals.ts';
11+
import { init } from '../build/esm/index.js';
12+
13+
function resetGlobals(): void {
14+
getMainCarrier().__SENTRY__ = undefined;
15+
}
16+
17+
// Captured before any init() so the patch assertion below compares against the
18+
// genuinely unpatched function; the patch installs once per module graph.
19+
const unpatchedServe = Deno.serve;
20+
21+
function initPatchedServeWithoutClient(): void {
22+
// Install the Deno.serve patch, then unbind the client. The patch is installed by
23+
// `Client.init()`, which a directly-constructed client also runs without ever
24+
// calling `setCurrentClient` — so a live patch with no bound client is a real state.
25+
resetGlobals();
26+
init({ dsn: 'https://username@domain/123' });
27+
assertNotEquals(Deno.serve, unpatchedServe, 'Deno.serve was not patched; test would pass vacuously');
28+
resetGlobals();
29+
}
30+
31+
Deno.test('Deno.serve keeps serving when no client is bound', async () => {
32+
initPatchedServeWithoutClient();
33+
34+
const abortController = new AbortController();
35+
let onListen: ((_: unknown) => void) | undefined = undefined;
36+
const p = new Promise(resolve => (onListen = resolve));
37+
const server = Deno.serve({ port: 0, signal: abortController.signal, onListen }, () => {
38+
return new Response('Hello World');
39+
});
40+
await p;
41+
42+
const response = await fetch(`http://localhost:${server.addr.port}/test`);
43+
assertEquals(response.status, 200);
44+
assertEquals(await response.text(), 'Hello World');
45+
46+
abortController.abort();
47+
await server.finished;
48+
});
49+
50+
Deno.test('Deno.serve propagates handler errors as 500 when no client is bound', async () => {
51+
initPatchedServeWithoutClient();
52+
53+
const abortController = new AbortController();
54+
let onListen: ((_: unknown) => void) | undefined = undefined;
55+
const p = new Promise(resolve => (onListen = resolve));
56+
const server = Deno.serve({ port: 0, signal: abortController.signal, onListen }, () => {
57+
throw new Error('handler blew up');
58+
});
59+
await p;
60+
61+
const response = await fetch(`http://localhost:${server.addr.port}/boom`);
62+
assertEquals(response.status, 500);
63+
await response.body?.cancel();
64+
65+
abortController.abort();
66+
await server.finished;
67+
});

0 commit comments

Comments
 (0)