Fix body handling on the Request constructor

[yutakahirano]

As Request.body is exposed, we need to specify how to handle the body property of the input Request more correctly. After

let r = new Request(request, ...);

successfully returns,

• request.body should remain unchanged.
• request.bodyUsed should be true.

Fixing yutakahirano/fetch-with-streams#60.

[yutakahirano]

@domenic, can you take a look? I want to use a transform stream and pipeTo to avoid manual stream creation. Does it make sense to you?

I filed whatwg/streams#658 for this change.

Thanks!

[domenic]

This seems a little confusing to me because it mixes the conceptual level (linking to the model section for transform stream) with the concrete level (linking to the specific algorithm for pipeTo). Maybe it is OK because we are just saying "everyone knows what an identity transform stream is". But it's not 100% clear to me, e.g. what the error propagation behavior is.

So in other words, the intent seems good, but it might not be precise enough. I'm curious what @annevk thinks.

Maybe one direction would be to make things even more vague, e.g. not mention the pipeTo algorithm at all, but just kind of vaguely describe how to get rs from inputBody?

[annevk]

So my main problem is, what happens if the developer overrides pipeTo()? I guess you want to use some kind of safe pipeTo()? Maybe Streams should define a way to invoke that. If pipeTo() is safe, then invoking it with a conceptual stream is probably okay. But if pipeTo() needs to be abstracted anyway, we might as well abstract the identity transform stream too.

[domenic]

The idea would be to create an abstract op for pipeTo (whatwg/streams#658) so that calling it always calls the spec algorithm. Not sure where that falls on your safe vs. abstracted spectrum :)

[annevk]

Sorry I missed that. I think that's okay. Could Streams not also define an identity transform stream that can be passed to that algorithm? Perhaps with a note that it's only for use by specifications until someone figures out a use case to expose it to JavaScript?

[domenic]

Yeah, maybe that is the way to go. Then we can define error handling behavior more concretely.

We definitely have a use case for it, we just haven't yet specified all the underlying infrastructure (e.g. there are no algorithms for transform streams yet).

[annevk]

I suggest we file an issue against Fetch to track this. And make this a class=XXX rather than a note.

[annevk]

And point to the issue here.

[yutakahirano]

Fixed.

[annevk]

Thanks, this PR looks good to me now, but we will need tests per the new process.

[domenic]

So I am now a bit confused. Do we want streams to define these things in vague terms? Or is the vague phrasing here, linking to the vague "model" section of streams, good enough for now (until transform streams are defined)? It sounds like we don't need a pipeTo abstract operation anymore?

[annevk]

I think it's okay to be vague for now, but I don't want us to be vague forever, so I want some kind of tracking URL for making it less vague.

We also need tests for the overall change.

[yutakahirano]

web-platform-tests/wpt#4612