Add json(), text() and other missing methods to JsRequest

[tomasol]

Adds request methods method, url, headers, and allows reading the request body with text, json, form_data. Host can supply the body asynchronously.

[github-actions]

Test262 conformance changes

Test result	main count	PR count	difference
Total	53,125	53,125	0
Passed	51,073	51,073	0
Ignored	1,482	1,482	0
Failed	570	570	0
Panics	0	0	0
Conformance	96.14%	96.14%	0.00%

Tested main commit: f54077467b4b01eb0fe221cfff470a8546ebe36c
Tested PR commit: 064341d96eb79c43907c86903deace85c12132ef
Compare commits: f540774...064341d

[codecov]

Codecov Report

❌ Patch coverage is 86.95652% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.90%. Comparing base (6ddc2b4) to head (064341d).
⚠️ Report is 1011 commits behind head on main.

Files with missing lines	Patch %	Lines
core/runtime/src/fetch/request.rs	86.95%	6 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5341       +/-   ##
===========================================
+ Coverage   47.24%   62.90%   +15.66%     
===========================================
  Files         476      530       +54     
  Lines       46892    59148    +12256     
===========================================
+ Hits        22154    37207    +15053     
+ Misses      24738    21941     -2797     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jedel1043]

I don't think this is the correct approach to enable fetching the body of a request. From what I investigated, body needs to return an object implementing the ReadableStream interface, which can be an async iterable, so we need to implement that first in order to implement this.

[jedel1043]

Also this will leak, because BodyState could be a future with Gc'd objects in it, making it possible to leak objects using this since the GC won't trace through the body.

[jedel1043]

The responsibility of fetching the body should be lifted to the Fetch implementor IMO, and we should just offer adaptors to convert Future<Vec<u8>> and Vec<u8> into something that implements ReadableStream.

[tomasol]

The responsibility of fetching the body should be lifted to the Fetch implementor IMO, and we should just offer adaptors to convert Future<Vec<u8>> and Vec<u8> into something that implements ReadableStream.

Fetcher update is not part of this PR and should be done afterwards. This change lays the groundwork and is already useful as is. It gives host code a way to pass an async body to JsRequest without blocking until the JS handler actually reads it.

The JS-facing body getter returning a proper ReadableStream can be added when streaming is implemented, but that would be a significant, orthogonal effort.

Also this will leak ...

The concern is technically valid at the type level, Gc<T> is 'static and could be captured in the closure, but there's no realistic use case where it would be. with_lazy_body is a host-facing API for supplying async bytes from external I/O, which by nature has nothing to do with GC-managed objects. The future produces Vec<u8>,
not JS values. I can add a doc note making this explicit, but I don't think it warrants blocking the PR.

[jedel1043]

The JS-facing body getter returning a proper ReadableStream can be added when streaming is implemented, but that would be a significant, orthogonal effort.

Then we are just implementing something that will be removed in the future, right? Seems like a waste to hack around this instead of offering a proper API for readable streams.

The concern is technically valid at the type level, Gc is 'static and could be captured in the closure, but there's no realistic use case where it would be. with_lazy_body is a host-facing API for supplying async bytes from external I/O, which by nature has nothing to do with GC-managed objects. The future produces Vec,
not JS values. I can add a doc note making this explicit, but I don't think it warrants blocking the PR.

We generally don't assume how users interact with APIs, because we have seen so many weird usages that we just go with the safest option. Leaking here is not the safest option.

[tomasol]

Adding ReadableStream support would not remove the proposed API. The BodyState enum could gain a third variant (e.g. Stream(JsReadableStream)) when streaming is implemented, keeping Ready and Pending intact for hosts that just want to supply bytes. The current API is immediately useful and not a dead end.

Ad Gc object leak:
Would making body: Gc<GcRefCell<BodyState>> (and dropping #[unsafe_ignore_trace]) be a viable path? Then the async variant would be Pending(Pin<Box<dyn Future<Output = Vec<u8>> + Trace + 'static>>) , making the contract explicit. Though Pin<P: Trace> may need a Trace impl in boa_gc .

[jedel1043]

Adding ReadableStream support would not remove the proposed API. The BodyState enum could gain a third variant (e.g. Stream(JsReadableStream)) when streaming is implemented, keeping Ready and Pending intact for hosts that just want to supply bytes. The current API is immediately useful and not a dead end.

Uh not really. Offering a ReadableStream API would remove the need for an enum for the body, since the implementor would choose which object to return as an instance of ReadableStream, including objects that support async iteration.

Would making body: Gc<GcRefCell> (and dropping #[unsafe_ignore_trace]) be a viable path? Then the async variant would be Pending(Pin<Box<dyn Future<Output = Vec> + Trace + 'static>>) , making the contract explicit. Though Pin<P: Trace> may need a Trace impl in boa_gc

That does not work, because Rust does not understand the concept of implementing Trace for async functions and blocks (you would have to Trace through the captured variables, but it is impossible to inspect captured variables inside async blocks). That's why we needed to create a lot of custom code to be able to construct traceable closures instead of using Fn directly.

[tomasol]

I did not propose passing async blocks but an implementation of Future<Output = Vec<u8>> + Trace.
This is something that will pop up anyway when ReadableStream is ready, no matter if it is going to be backed by AsyncRead, Stream , Future or any other trait.

[jedel1043]

I did not propose passing async blocks but an implementation of Future<Output = Vec> + Trace.

Are you suggesting that we force every user to manually implement Future? Because that would heavily affect the ergonomics of the API, since now users would have to deal with pinning/wakers and the whole state machine lowering ordeal.

This is something that will pop up anyway when ReadableStream is ready, no matter if it is going to be backed by AsyncRead, Stream , Future or any other trait.

Not necessarily. We have an API to convert a Future into a JsPromise, so users can just enqueue an async job that eventually resolves a promise without having to deal with storing Futures inside js objects.

[tomasol]

It seems that JsPromise::from_async_fn assumes that the input async closure does not capture Gc objects though.
I don't see how that can be considered safe (WRT GC leak) if passing a boxed future is not.

[jedel1043]

It seems that JsPromise::from_async_fn assumes that the input async closure does not capture Gc objects though.
I don't see how that can be considered safe (WRT GC leak) if passing a boxed future is not.

This is safe because the input async closure is never unrooted to store it into a JsObject; the future is resolved at the Context level, and anything stored inside Context is considered a root by the GC

[tomasol]

I've been looking at the GC internals and I believe both approaches have the same leak properties: any Gc captured inside a non-traced closure/future becomes an untraced root. This applies equally to from_async_fn, nothing prevents its closure from capturing arbitrary Gc objects.

The one difference I can see is that a future stored inside a Gc-managed object could form an uncollectable cycle if it captures a reference back to its parent. This is hard to trigger through with_lazy_body and bounded by whoever polls the future, but I may be missing something. Could you clarify the specific failure mode you're worried about?

[jedel1043]

I've been looking at the GC internals and I believe both approaches have the same leak properties: any Gc captured inside a non-traced closure/future becomes an untraced root. This applies equally to from_async_fn, nothing prevents its closure from capturing arbitrary Gc objects.

from_async_fn never leaks (if you don't explicitly leak), because when the parent closure gets dropped the inner Gc'd variables also get dropped which lowers their reference counts automatically. That doesn't happen if the future is inside a Gc'd object, because it cannot be dropped until the GC itself asserts that the Gc'd object is unreachable.

[tomasol]

Since the async-body mechanism was rejected, I am narrowing this PR to the getters as the PR title describes.