fix(runtime): subclassing native Request/Response exposes working body methods#4756
Merged
Conversation
…y methods (v0.5.1130)
A Web-Fetch Request/Response in Perry is a native registry handle (a small id
NaN-boxed as a pointer) whose methods are resolved by native handle dispatch,
not via a JS prototype chain. `class X extends Request {}` + `new X(...)`
produced an ordinary heap JS object with no link to a native handle, so
`sub.text` was `undefined`, calling it threw, and `sub instanceof Request` was
`false`. This breaks `@hono/node-server` (`class Request extends GlobalRequest`).
Fix: `super(...)` on a Request/Response parent now allocates the underlying
native handle and stashes its id on `this` under `__perry_fetch_handle__`
(mirrors the Web-Streams `__perry_stream_handle__` mechanism). Wired across all
construction paths:
- direct `class extends Request/Response`: codegen super arm + lower_new
no-ctor hook -> js_request/response_subclass_init;
- aliased parent (`extends GlobalRequest`, where GlobalRequest = global.Request):
runtime-value super() routes through js_fetch_or_value_super, which identifies
the parent constructor and attaches;
- dynamic class-expression / ClassRef construction: a fetch-parent side table
(recorded at js_register_class_parent_dynamic) drives a construction-time
attach in js_new_function_construct.
At access time, body methods (call + value read), inherited property getters,
and `instanceof` on a subclass instance are forwarded to the native handle.
Non-subclassed Request/Response and other native-builtin subclasses are
unaffected. Regression test: tests/test_request_subclass_body.sh.
50fd692 to
ef270ae
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Subclassing the native global
Request(orResponse) produced instances that were missing the body-reading methods (text/json/arrayBuffer/blob/formData/bytes), missing the inherited property getters, andsub instanceof Requestwasfalse:This breaks
@hono/node-server, which doesclass Request extends GlobalRequest { … }, so everyc.req.text()/c.req.json()on a POST/PUT route threw, turning webhooks and form posts into 500s.Root cause
A Web-Fetch
Request/Responsein Perry is not a heap JS object — it's a native registry handle (a small id NaN-boxed as a pointer) whose methods are resolved by native handle dispatch, not via a JS prototype chain.class X extends Request {}+new X(...)produced an ordinary heap JS object with no link to any native handle: property/method lookup walked the JS prototype chain, found nothing (the body methods live in native dispatch, not onRequest.prototype), and returnedundefined.Fix
super(...)on a Request/Response parent now allocates the underlying native handle and stashes its id onthisunder the hidden field__perry_fetch_handle__— mirroring the Web-Streams__perry_stream_handle__mechanism (#562). Wired across construction paths:class extends Request/Response: codegenExpr::SuperCallarm + the no-constructorlower_newhook →js_request_subclass_init/js_response_subclass_init(object/global_this.rs);extends GlobalRequestwhereGlobalRequest = global.Request): the runtime-valuesuper()routes through a newjs_fetch_or_value_super, which identifies the parent constructor viaidentify_global_builtin_constructorand attaches (falling back to the ordinaryjs_native_call_valuefor every other runtime-value parent);js_register_class_parent_dynamicdrives a construction-time attach injs_new_function_construct.At access time:
native_call_method.rs): inherited body methods on a subclass receiver are forwarded to the native handle dispatcher (only after all user-defined dispatch — own fields, vtable, prototype walk — has missed, so a subclass override still wins);field_get_set.rs): a missed read forwards to the handle, sourl/method/headers/body/bodyUsed/… and body-method-as-value reads resolve;instanceof(instanceof.rs): the Request/Response/Headers/Blob arm unwraps the stashed handle and runs the existing fetch kind-probe.Non-subclassed
Request/Responseand other native-builtin subclasses are unaffected — the new paths only trigger when a heap object carries__perry_fetch_handle__.Verification
typeof sub.text→"function",sub instanceof Request→true,await sub.text()→"hello".tests/test_request_subclass_body.sh(style of existingtests/test_*.sh, prints PASS/FAIL): subclasses nativeRequestas both a no-constructor class and an explicit-constructor class, asserts.text()/.json()round-trip a body andinstanceof Request. PASS.class … extends GlobalRequestwith an explicit constructor (the@hono/node-servershape) verified to expose working body methods when constructed via the static path.Known remaining gaps for the full
@hono/node-serverend-to-end (separate from this dispatch bug)While validating against a real Hono app I found two distinct, pre-existing gaps that also stand between this fix and a green webhook, and are larger than the documented dispatch bug:
Requestdoes not read aReadableStreambody. The body coercion stringifies the stream handle:new Request(url, { body: stream }).text()returns the handle id (e.g."1048576") instead of the bytes.@hono/node-serveralways wraps the incoming body asReadable.toWeb(incoming)(a pull-based stream), so even with dispatch fixed,c.req.text()returns garbage. Fixing this needs the request-body coercion to drain a (pull-based) web stream.var Request = class extends GlobalRequest {…}, built lazily insidegetRequestCache()) does not yet wire the fetch parent in every path. This PR adds the dynamic-construction side table toward it, but the node-server shape still needs follow-up.These are tracked as follow-ups; this PR fixes the documented root cause (subclass instances missing body methods) with a passing regression test.