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,051	51,051	0
Ignored	1,482	1,482	0
Failed	592	592	0
Panics	0	0	0
Conformance	96.10%	96.10%	0.00%

Tested main commit: 3ee2b71a306334c2b2a0dcc20ac4b5e1c5a71558
Tested PR commit: 42c1b372ca4996a0e97f922ead20e1e55f4c151b
Compare commits: 3ee2b71...42c1b37

[codecov]

Codecov Report

❌ Patch coverage is 34.95146% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.63%. Comparing base (6ddc2b4) to head (42c1b37).
⚠️ Report is 957 commits behind head on main.

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

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5341       +/-   ##
===========================================
+ Coverage   47.24%   59.63%   +12.38%     
===========================================
  Files         476      590      +114     
  Lines       46892    63811    +16919     
===========================================
+ Hits        22154    38053    +15899     
- Misses      24738    25758     +1020     

☔ View full report in Codecov by Sentry.
📢 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.