Skip to content

fix: #769 — wire node:http / node:https client request through codegen dispatch#771

Merged
TheHypnoo merged 5 commits into
mainfrom
fix/769-http-request-dispatch
May 14, 2026
Merged

fix: #769 — wire node:http / node:https client request through codegen dispatch#771
TheHypnoo merged 5 commits into
mainfrom
fix/769-http-request-dispatch

Conversation

@TheHypnoo
Copy link
Copy Markdown
Contributor

@TheHypnoo TheHypnoo commented May 14, 2026

Summary

Closes #769 (split off from #767 case 3).

http.request(...) and http.get(...) (and the https.* variants) always returned undefined from compiled TypeScript, so the standard const req = http.request(url, cb); req.on('error', ...); req.end() pattern crashed at .on with "Cannot read properties of undefined". The runtime implementations existed in perry-ext-http and the FFI symbols were declared, but no NativeModSig entries existed in the codegen dispatch table — calls fell through to the unknown-method path and got back TAG_UNDEFINED.

What changed

  • Codegen dispatch (crates/perry-codegen/src/lower_call.rs): add NativeModSig entries for http.request, http.get, https.request, https.get and the ClientRequest instance methods on, end, write, setHeader, setTimeout.
  • HIR class tagging (crates/perry-hir/src/lower.rs + crates/perry-hir/src/lower_decl.rs): tag const req = http.request(...) | http.get(...) | https.* bindings as ("http", "ClientRequest") so req.on/.end/... dispatch through the new class-filtered entries. Mirrors the existing net.createConnection / tls.connect tagging. Important: the closure-body path in lower_decl.rs had no equivalent of the top-level arm in lower.rs, so request() inside a callback (the common shape: server.listen(..., () => { ... request(...) ... })) was unreachable.
  • perry-ext-http (crates/perry-ext-http/src/lib.rs):
    • accept either a URL string or an options object in request_common — the previous code only handled the options-object overload, and the reporter's repro uses http.request("http://...", cb);
    • rewrite dispatch_request to spawn the reqwest future as a detached task on the multi-thread runtime instead of Handle::current().block_on. Now that the dispatch actually reaches this code, block_on panicked with "Cannot start a runtime from within a runtime" because spawn_blocking_with_reactor runs the closure inside runtime().spawn(async {...}). Mirrors the spawn_socket_runner pattern in perry-ext-net, including the LTO black_box workaround for tokio's CONTEXT statics;
    • export js_http_is_incoming_message(handle) so perry-stdlib can gate property dispatch on registry membership.
  • IncomingMessage property dispatch (crates/perry-stdlib/src/common/dispatch.rs): add a statusCode / statusMessage / headers arm in js_handle_property_dispatch. Without this, (res) => res.statusCode inside the response callback returned undefined.
  • Client-side pump (crates/perry-stdlib/Cargo.toml + crates/perry-stdlib/src/common/async_bridge.rs + crates/perry/src/commands/compile/optimized_libs.rs): add an external-http-client-pump feature, drain js_http_process_pending from the main-thread pump, and keep the event loop alive via js_http_has_pending while a request is in-flight. Activated by the well-known flip whenever node:http / node:https route to perry-ext-http. Mirrors the existing external-http-server-pump plumbing.

Test

New fixture test-files/test_node_http_client_request.ts exercises all three forms (request(url, cb), request(options, cb), get(url, cb)) against an in-process createServer and verifies res.statusCode reflects the response. End-to-end verified against http://example.com/:

statusCode: 200
statusMessage: OK
typeof headers: object
content-type header: text/html

The reporter's exact req.on/req.end repro from #769 also runs without crashing.

cargo test --release -p perry-hir -p perry-codegen -p perry-ext-http --lib — all green. Regression-checked fetch/axios (still work) and net.connect(options, cb) (still fails — tracked separately in #770).

Test plan

  • test-files/test_node_http_client_request.ts runs locally on macOS arm64
  • res.statusCode / res.statusMessage / res.headers return real values against an external server
  • fetch / axios continue to return 200
  • cargo test --release -p perry-hir -p perry-codegen -p perry-ext-http --lib passes
  • CI parity / cargo-test / compile-smoke green

TheHypnoo added a commit that referenced this pull request May 14, 2026
The cargo-test in #771's CI flagged that the new
`ClientRequest.setTimeout` NativeModSig entry has no manifest
counterpart. Existing class-filtered http methods (on/end/write/
setHeader) collapse to the manifest entries already covering
ServerResponse / IncomingMessage / HttpServer — only setTimeout was
genuinely new on the http module.
TheHypnoo added a commit that referenced this pull request May 14, 2026
CI parity on #771 surfaced \`test_node_http_client_request.ts\` as an
output mismatch — the three concurrent requests fired their \"status\"
prints in tokio-scheduling-dependent order, which diverged from Node's
behavior run-to-run.

Chain the requests instead (req2 inside req1's response callback, req3
inside req2's) so the line order is deterministic and matches
\`node --experimental-strip-types\` byte-for-byte. Also dropped the
per-path \`statusCode\` setter from the in-process server — it was
unused (Perry and Node both default to 200) and only confused the
expected output.

Verified locally: Perry and Node both produce:
  server listening
  req1 status: 200
  req2 status: 200
  req3 status: 200
  done
TheHypnoo added 5 commits May 14, 2026 20:06
…rough codegen dispatch

`http.request(...)` and `http.get(...)` (and the `https.*` variants) always
returned `undefined` from compiled TypeScript, even though the runtime
implementations existed in `perry-ext-http`. The FFI symbols were declared
but no `NativeModSig` dispatch entries existed, so the call site fell
through to the unknown-method path and got back `TAG_UNDEFINED`.
Reported in #767 (case 3) and tracked separately in #769.

Changes:

- `perry-codegen/src/lower_call.rs`: add dispatch entries for
  `http.request`, `http.get`, `https.request`, `https.get` (module-level)
  and ClientRequest instance methods (`on`, `end`, `write`, `setHeader`,
  `setTimeout`).
- `perry-hir/src/lower.rs` + `perry-hir/src/lower_decl.rs`: tag
  `const req = http.request(...) | http.get(...) | https.*` bindings as
  `("http", "ClientRequest")` native instances so `req.on/.end/...`
  dispatch through the class-filtered entries above. Mirrors the
  existing `net.createConnection` / `tls.connect` tagging; the
  closure-body path in `lower_decl.rs` had no equivalent of the
  top-level arm in `lower.rs`, so `request()` inside a callback was
  unreachable.
- `perry-ext-http/src/lib.rs`:
  - accept either a URL string or an options object in `request_common`
    (was options-only), matching Node's `http.request(url, cb)` overload
    which the reporter's repro used;
  - rewrite `dispatch_request` to spawn the reqwest future as a
    detached task on the same multi-thread runtime instead of
    `Handle::current().block_on` (which panicked with "Cannot start a
    runtime from within a runtime" now that the dispatch reaches this
    code). Mirrors `perry-ext-net::spawn_socket_runner`, including the
    LTO `black_box` workaround for tokio's CONTEXT statics.
- `perry-stdlib/Cargo.toml` + `perry-stdlib/src/common/async_bridge.rs`:
  add `external-http-client-pump` feature so perry-stdlib's main-thread
  pump drains perry-ext-http's response/error queue and keeps the event
  loop alive while a request is in-flight. Mirrors the existing
  `external-http-server-pump` plumbing.
- `perry/src/commands/compile/optimized_libs.rs`: activate
  `external-http-client-pump` when the well-known flip routes
  `node:http` / `node:https` to perry-ext-http.

Test: `test-files/test_node_http_client_request.ts` exercises all three
forms (`request(url, cb)`, `request(options, cb)`, `get(url, cb)`)
against an in-process `createServer`, verifying the response callback
fires and the server actually receives the requests.

Scope: response-side property access (`res.statusCode`, `res.headers`)
still needs the handle-property dispatch surface and is left as a
follow-up — the repro from #769 only needs `req.on('error', ...)`
and `req.end()` to stop crashing, which this PR delivers.
… statusMessage / headers)

Follow-up on the same PR — the previous commit made `http.request(...)`
return a usable ClientRequest handle, but `(res) => res.statusCode`
inside the response callback still returned `undefined` because
IncomingMessage handles had no property dispatch.

- `perry-ext-http`: export `js_http_is_incoming_message(handle)` so
  callers can gate property dispatch on registry membership (avoids
  colliding with unrelated handles whose ids happen to overlap).
- `perry-stdlib/common/dispatch.rs`: add a `statusCode` / `statusMessage`
  / `headers` arm in `js_handle_property_dispatch`, gated on
  `external-http-client-pump`. Calls the existing
  `js_http_status_code` / `js_http_status_message` /
  `js_http_response_headers` externs from perry-ext-http.
- `test-files/test_node_http_client_request.ts`: assert
  `res.statusCode` actually reflects the response.

Verified against an external endpoint (`http://example.com/` →
`statusCode: 200`, `statusMessage: "OK"`, `headers["content-type"]:
"text/html"`).
The cargo-test in #771's CI flagged that the new
`ClientRequest.setTimeout` NativeModSig entry has no manifest
counterpart. Existing class-filtered http methods (on/end/write/
setHeader) collapse to the manifest entries already covering
ServerResponse / IncomingMessage / HttpServer — only setTimeout was
genuinely new on the http module.
CI parity on #771 surfaced \`test_node_http_client_request.ts\` as an
output mismatch — the three concurrent requests fired their \"status\"
prints in tokio-scheduling-dependent order, which diverged from Node's
behavior run-to-run.

Chain the requests instead (req2 inside req1's response callback, req3
inside req2's) so the line order is deterministic and matches
\`node --experimental-strip-types\` byte-for-byte. Also dropped the
per-path \`statusCode\` setter from the in-process server — it was
unused (Perry and Node both default to 200) and only confused the
expected output.

Verified locally: Perry and Node both produce:
  server listening
  req1 status: 200
  req2 status: 200
  req3 status: 200
  done
@TheHypnoo TheHypnoo force-pushed the fix/769-http-request-dispatch branch from 78937af to a595d55 Compare May 14, 2026 18:10
@TheHypnoo TheHypnoo merged commit c3b0b73 into main May 14, 2026
9 checks passed
@TheHypnoo TheHypnoo deleted the fix/769-http-request-dispatch branch May 14, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node:http request() returns undefined — missing codegen dispatch entry

1 participant