Skip to content

Commit 8aec4d5

Browse files
authored
feat(ext/fetch): emit Network.* inspector events for fetch() (#34220)
Building on the Network CDP domain landed in #32707 and the body buffering hooks in #34201, this wires the global `fetch()` into the inspector so that DevTools and `node:inspector` clients actually observe outgoing HTTP traffic. Until now Deno exposed the `Network.*` emitters but nothing inside Deno called them, so attached frontends saw no events and `Network.getResponseBody` had no data to return. When the inspector is attached, each `fetch()` now emits `requestWillBeSent`, `responseReceived`, `dataReceived`, and either `loadingFinished` or `loadingFailed`, including the post-data and response-body bookkeeping the buffer needs. The cross-extension bridge lives on `internals.__inspectorNetwork`, installed by ext/node's inspector polyfill, so ext/fetch doesn't depend on ext/node and the non-attached path is one `isEnabled()` check. Enables `parallel/test-inspector-network-fetch.js`; node:http / node:http2 / WebSocket instrumentation will follow.
1 parent 3444038 commit 8aec4d5

10 files changed

Lines changed: 612 additions & 22 deletions

File tree

ext/fetch/26_fetch.js

Lines changed: 357 additions & 7 deletions
Large diffs are not rendered by default.

ext/node/ops/inspector.rs

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,33 @@ pub fn op_inspector_emit_protocol_event(
168168
}
169169
}
170170

171+
/// Convert a `file://` URL emitted by V8 (e.g. as a script name in a
172+
/// stack frame) into the OS filesystem path. CJS code sees `__filename`
173+
/// as such a path (with backslashes on Windows), so this lets test
174+
/// assertions like `findFrameInInitiator(__filename, initiator)` match.
175+
///
176+
/// Anything that isn't a `file://` URL is returned unchanged.
177+
///
178+
/// TODO: percent-decode the URL path. Paths with spaces or non-ASCII
179+
/// characters arrive as e.g. `/path%20with%20spaces/foo.js` and won't
180+
/// match a `__filename` containing literal spaces. Rare in CI, real in
181+
/// user code.
182+
/// TODO: handle UNC paths (`file://server/share/x`) on Windows. The
183+
/// current `strip_prefix("file://")` drops both slashes, losing the
184+
/// `\\server\share` marker.
185+
fn file_url_to_os_path(s: String) -> String {
186+
let Some(after_scheme) = s.strip_prefix("file://") else {
187+
return s;
188+
};
189+
// file:///path/to/file -> /path/to/file (Unix)
190+
// file:///C:/path -> C:\path (Windows)
191+
if sys_traits::impls::is_windows() {
192+
after_scheme.trim_start_matches('/').replace('/', "\\")
193+
} else {
194+
after_scheme.to_string()
195+
}
196+
}
197+
171198
fn capture_initiator(scope: &mut v8::PinScope<'_, '_>) -> serde_json::Value {
172199
let Some(stack_trace) = v8::StackTrace::current_stack_trace(scope, 10) else {
173200
return serde_json::json!({ "type": "other" });
@@ -195,6 +222,7 @@ fn capture_initiator(scope: &mut v8::PinScope<'_, '_>) -> serde_json::Value {
195222
let url = frame
196223
.get_script_name(scope)
197224
.map(|n| n.to_rust_string_lossy(scope))
225+
.map(file_url_to_os_path)
198226
.unwrap_or_default();
199227
let line_number = frame.get_line_number().saturating_sub(1); // CDP uses 0-based
200228
let column_number = frame.get_column().saturating_sub(1);
@@ -208,16 +236,18 @@ fn capture_initiator(scope: &mut v8::PinScope<'_, '_>) -> serde_json::Value {
208236
}));
209237
}
210238

211-
if call_frames.is_empty() {
212-
serde_json::json!({ "type": "other" })
213-
} else {
214-
serde_json::json!({
215-
"type": "script",
216-
"stackTrace": {
217-
"callFrames": call_frames,
218-
},
219-
})
220-
}
239+
// Always emit `type: "script"` even when `call_frames` is empty.
240+
// Returning `{type: "other"}` for the empty case (as the original code
241+
// did) tripped node_compat tests that assert `initiator.type ===
242+
// "script"` whenever the inspector saw a JS-originated request - which
243+
// is true here by construction, since we only reach this function from
244+
// `op_inspector_emit_protocol_event` called from JS-land emitters.
245+
serde_json::json!({
246+
"type": "script",
247+
"stack": {
248+
"callFrames": call_frames,
249+
},
250+
})
221251
}
222252

223253
struct JSInspectorSession {

ext/node/polyfills/inspector.js

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
// deno-lint-ignore-file prefer-primordials
55

66
(function () {
7-
const { core, primordials } = __bootstrap;
7+
const { core, internals, primordials } = __bootstrap;
88
const {
99
op_base64_encode_from_buffer,
1010
op_get_extras_binding_object,
@@ -305,6 +305,22 @@ const Network = {
305305
broadcastToFrontend("Network.webSocketClosed", params),
306306
};
307307

308+
// Bridge for other extensions (fetch, websocket, http) to emit Network.*
309+
// inspector events without depending on ext/node directly. Populated when
310+
// node:inspector is loaded; ext/fetch and friends look it up lazily on
311+
// `internals.__inspectorNetwork`.
312+
let networkRequestIdCounter = 0;
313+
internals.__inspectorNetwork = {
314+
isEnabled: () => op_inspector_enabled(),
315+
nextRequestId: () => `node-network-event-${++networkRequestIdCounter}`,
316+
requestWillBeSent: Network.requestWillBeSent,
317+
responseReceived: Network.responseReceived,
318+
loadingFinished: Network.loadingFinished,
319+
loadingFailed: Network.loadingFailed,
320+
dataReceived: Network.dataReceived,
321+
dataSent: Network.dataSent,
322+
};
323+
308324
const DOMStorage = {
309325
domStorageItemAdded: (params) =>
310326
broadcastToFrontend("DOMStorage.domStorageItemAdded", params),

libs/core/inspector.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ fn extend_capped(target: &mut Vec<u8>, bytes: &[u8]) {
140140
/// Charset string matching Node's `request_charset == "utf-8"` check
141141
/// in `network_agent.cc`. Anything else is treated as binary.
142142
fn is_utf8_charset(charset: Option<&str>) -> bool {
143-
charset == Some("utf-8")
143+
charset.is_some_and(|c| c.eq_ignore_ascii_case("utf-8"))
144144
}
145145

146146
fn encode_b64(bytes: &[u8]) -> String {

tests/node_compat/config.jsonc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4995,6 +4995,10 @@
49954995
"parallel/test-inspector-network-arbitrary-data.js": {},
49964996
"parallel/test-inspector-network-data-received.js": {},
49974997
"parallel/test-inspector-network-data-sent.js": {},
4998+
"parallel/test-inspector-network-fetch.js": {
4999+
"extraDenoArgs": ["--unsafely-ignore-certificate-errors"],
5000+
"reason": "Hits a local HTTPS server with a self-signed cert via fetch(). The upstream test bypasses cert checks by setting undici's global dispatcher with rejectUnauthorized:false; Deno's fetch doesn't honor that, so we disable cert validation only for this test."
5001+
},
49985002
"parallel/test-listen-fd-cluster.js": {
49995003
"linux": false,
50005004
"darwin": false,

tests/node_compat/mod.rs

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,14 @@ struct TestConfig {
9898
/// override (e.g. `node_shared_openssl=1`) which would regress other tests
9999
/// if applied globally.
100100
env: Option<HashMap<String, String>>,
101+
/// Extra `deno run`/`deno test` CLI flags to append for this test only,
102+
/// appearing after the standard `RUN_ARGS`/`TEST_ARGS` and after any
103+
/// flags derived from the test source's `// Flags:` directive. Use
104+
/// sparingly - typically for security knobs like
105+
/// `--unsafely-ignore-certificate-errors` that one specific test
106+
/// requires and that we explicitly do not want every test to inherit.
107+
#[serde(rename = "extraDenoArgs", default)]
108+
extra_deno_args: Vec<String>,
101109
}
102110

103111
/// The full config.json structure
@@ -476,9 +484,10 @@ fn uses_node_test_module(source: &str) -> bool {
476484
source.contains("'node:test'") || source.contains("\"node:test\"")
477485
}
478486

479-
fn parse_flags(source: &str) -> (Vec<String>, Vec<String>) {
487+
fn parse_flags(source: &str) -> (Vec<String>, Vec<String>, Vec<String>) {
480488
let mut v8_flags = Vec::new();
481489
let mut node_options = Vec::new();
490+
let mut deno_args = Vec::new();
482491

483492
let re = Regex::new(r"^// Flags: (.+)$").unwrap();
484493
for line in source.lines() {
@@ -535,14 +544,32 @@ fn parse_flags(source: &str) -> (Vec<String>, Vec<String>) {
535544
n if n.starts_with("--title=") => {
536545
node_options.push(flag.to_string());
537546
}
547+
// Inspector tests opt in to the inspector via `--inspect=PORT`
548+
// (commonly `--inspect=0` to pick a random port). Forward to
549+
// Deno, normalizing the bare port form to `127.0.0.1:PORT` since
550+
// Deno's CLI expects `host:port`.
551+
n if n == "--inspect" || n.starts_with("--inspect=") => {
552+
let mapped = match n.strip_prefix("--inspect=") {
553+
None | Some("") => "--inspect=127.0.0.1:0".to_string(),
554+
Some(rest) if rest.chars().all(|c| c.is_ascii_digit()) => {
555+
format!("--inspect=127.0.0.1:{}", rest)
556+
}
557+
Some(rest) => format!("--inspect={}", rest),
558+
};
559+
deno_args.push(mapped);
560+
}
561+
"--experimental-network-inspection" => {
562+
// Deno emits Network.* events when the inspector is attached;
563+
// no separate opt-in flag.
564+
}
538565
_ => {}
539566
}
540567
}
541568
break; // Only process the first Flags: line
542569
}
543570
}
544571

545-
(v8_flags, node_options)
572+
(v8_flags, node_options, deno_args)
546573
}
547574

548575
fn truncate_output(output: &str, max_len: usize) -> String {
@@ -574,7 +601,7 @@ impl TestSetup {
574601

575602
let source = full_test_path.read_to_string();
576603
let uses_node_test = uses_node_test_module(&source);
577-
let (v8_flags, node_options) = parse_flags(&source);
604+
let (v8_flags, node_options, extra_deno_args) = parse_flags(&source);
578605

579606
let mut args: Vec<String> = if uses_node_test {
580607
TEST_ARGS.iter().map(|s| s.to_string()).collect()
@@ -585,6 +612,16 @@ impl TestSetup {
585612
if !v8_flags.is_empty() {
586613
args.push(format!("--v8-flags={}", v8_flags.join(",")));
587614
}
615+
for arg in &extra_deno_args {
616+
args.push(arg.clone());
617+
}
618+
// Per-test extra args from config.jsonc, e.g. security knobs that
619+
// only one specific test needs.
620+
if let Some(extra) = test_config {
621+
for arg in &extra.extra_deno_args {
622+
args.push(arg.clone());
623+
}
624+
}
588625
if cli_args.inspect_brk {
589626
args.push("--inspect-brk".to_string());
590627
}

tests/node_compat/schema.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@
8383
"type": "object",
8484
"description": "Per-test environment variables, layered on top of the runner's defaults. Use sparingly — only for tests where one Node fixture needs a config the rest of the suite must not have.",
8585
"additionalProperties": { "type": "string" }
86+
},
87+
"extraDenoArgs": {
88+
"type": "array",
89+
"description": "Extra deno run/test CLI flags for this test only. Use sparingly — typically for security knobs like --unsafely-ignore-certificate-errors that one specific test requires.",
90+
"items": { "type": "string" }
8691
}
8792
},
8893
"additionalProperties": false
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"tests": {
3+
"fetch_events": {
4+
"args": "run --inspect=0 -A main.js",
5+
"output": "main.out"
6+
}
7+
}
8+
}
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// Covers two pieces of the fetch inspector instrumentation that the
2+
// upstream node_compat test doesn't exercise:
3+
//
4+
// 1. Redirect chain - a 302 hop should keep the same requestId and the
5+
// next `requestWillBeSent` should carry a `redirectResponse` of the
6+
// previous hop. The intermediate 30x should NOT fire its own
7+
// `responseReceived`.
8+
// 2. Streaming request body - when fetch's body is a ReadableStream,
9+
// `Network.getRequestPostData` must reject with "not finished yet"
10+
// because we never emit `dataSent({finished:true})` for that path.
11+
import inspector from "node:inspector/promises";
12+
import { strict as assert } from "node:assert";
13+
14+
const session = new inspector.Session();
15+
session.connect();
16+
await session.post("Network.enable");
17+
18+
// ---- Test 1: redirect chain ------------------------------------------------
19+
{
20+
const server = Deno.serve({ port: 0, onListen: () => {} }, (req) => {
21+
const url = new URL(req.url);
22+
if (url.pathname === "/start") {
23+
return new Response(null, {
24+
status: 302,
25+
headers: { location: "/landing" },
26+
});
27+
}
28+
return new Response("ok", { headers: { "content-type": "text/plain" } });
29+
});
30+
31+
const events = [];
32+
const onEvent = ({ method, params }) => events.push({ method, params });
33+
session.on("Network.requestWillBeSent", onEvent);
34+
session.on("Network.responseReceived", onEvent);
35+
session.on("Network.loadingFinished", onEvent);
36+
37+
const startUrl = `http://127.0.0.1:${server.addr.port}/start`;
38+
const finalUrl = `http://127.0.0.1:${server.addr.port}/landing`;
39+
const resp = await fetch(startUrl);
40+
await resp.text();
41+
// Let the background drain emit loadingFinished.
42+
await new Promise((r) => setTimeout(r, 50));
43+
44+
session.off("Network.requestWillBeSent", onEvent);
45+
session.off("Network.responseReceived", onEvent);
46+
session.off("Network.loadingFinished", onEvent);
47+
await server.shutdown();
48+
49+
const willBeSent = events.filter((e) =>
50+
e.method === "Network.requestWillBeSent"
51+
);
52+
const received = events.filter((e) =>
53+
e.method === "Network.responseReceived"
54+
);
55+
const finished = events.filter((e) => e.method === "Network.loadingFinished");
56+
57+
assert.equal(willBeSent.length, 2, "expected two requestWillBeSent events");
58+
assert.equal(
59+
willBeSent[0].params.requestId,
60+
willBeSent[1].params.requestId,
61+
"redirect hop should reuse the original requestId",
62+
);
63+
assert.equal(willBeSent[0].params.request.url, startUrl);
64+
assert.equal(willBeSent[0].params.redirectResponse, undefined);
65+
assert.equal(willBeSent[1].params.request.url, finalUrl);
66+
assert.equal(willBeSent[1].params.redirectResponse.status, 302);
67+
assert.equal(willBeSent[1].params.redirectResponse.url, startUrl);
68+
69+
// The intermediate 302 must not produce its own responseReceived; only
70+
// the final 200 should.
71+
assert.equal(received.length, 1, "exactly one responseReceived");
72+
assert.equal(received[0].params.response.status, 200);
73+
assert.equal(received[0].params.response.url, finalUrl);
74+
75+
assert.equal(finished.length, 1, "exactly one loadingFinished");
76+
assert.equal(
77+
finished[0].params.requestId,
78+
willBeSent[0].params.requestId,
79+
"loadingFinished should share the chain's requestId",
80+
);
81+
console.log("PASS: redirect chain emits with shared requestId");
82+
}
83+
84+
// ---- Test 2: streaming request body ---------------------------------------
85+
{
86+
const server = Deno.serve(
87+
{ port: 0, onListen: () => {} },
88+
async (req) => new Response(await req.text()),
89+
);
90+
91+
let requestId;
92+
const gotRequestWillBeSent = new Promise((resolve) => {
93+
session.once("Network.requestWillBeSent", ({ params }) => {
94+
requestId = params.requestId;
95+
resolve();
96+
});
97+
});
98+
99+
const body = new ReadableStream({
100+
start(c) {
101+
c.enqueue(new TextEncoder().encode("streamed-body"));
102+
c.close();
103+
},
104+
});
105+
const resp = await fetch(`http://127.0.0.1:${server.addr.port}/`, {
106+
method: "POST",
107+
body,
108+
});
109+
await resp.text();
110+
await gotRequestWillBeSent;
111+
112+
// For streaming bodies we deliberately don't flip is_request_finished -
113+
// there's no chunked dataSent path wired yet, so getRequestPostData
114+
// must reject rather than return garbage.
115+
let rejected = false;
116+
try {
117+
await session.post("Network.getRequestPostData", { requestId });
118+
} catch (err) {
119+
rejected = true;
120+
assert.match(
121+
String(err.message ?? err),
122+
/not finished yet/i,
123+
"should reject with 'not finished yet'",
124+
);
125+
}
126+
assert.equal(
127+
rejected,
128+
true,
129+
"getRequestPostData should reject for streaming bodies",
130+
);
131+
132+
await server.shutdown();
133+
console.log("PASS: streaming-body request stays un-finished");
134+
}
135+
136+
session.disconnect();
137+
console.log("ALL PASSED");
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
[WILDCARD]PASS: redirect chain emits with shared requestId
2+
PASS: streaming-body request stays un-finished
3+
ALL PASSED

0 commit comments

Comments
 (0)