FetchEvent.respondWith does something weird with the body of a response

[mkruisselbrink]

In the algorithm describing respondWith the following steps exist:

1. Set the potential response to response’s associated response.
2. If response’s body is non-null, run these substeps:
   1. Let dummyStream be an empty ReadableStream object.
   2. Set response’s body to a new body whose stream is dummyStream.
   3. Let reader be the result of getting a reader from dummyStream.
   4. Read all bytes from dummyStream with reader.

I think this is incorrect, although I'm not entirely sure what it is trying to do.

What it seems to say it is doing is assigning a new body to response's body (not sure how that is possible, response's body is defined as being the body of response's associated response. There doesn't seem to be a way to change the body of just the Response object while leaving the body of the underlying response alone). It then goes and reads all the bytes from this newly created empty body.

I'm guessing the intended effect is to leave the javascript Response object with a disturbed body, which happens to be an empty stream, while leaving the underlying response alone. But the fetch spec doesn't allow decoupling the body of the Response object from the body of the associated response, so what is written doesn't actually work.

[domenic]

/c @yutakahirano

[yutakahirano]

cc: @jungkees, the author of ccff1f1.

I guess the description is from the bottom of https://fetch.spec.whatwg.org/#dom-request. That works because Request doesn't expose body property, but it's not true for Response.

[jungkees]

Basically, I wanted to avoid cloning the body not to require additional memory space. I still want to find a way. Do the following steps make sense?

response: a Response object (resolved from a promise) passed to respondWith(r)

1. Let newResponse be a copy of response’s associated response, except for its body.
2. If response’s body is non-null, run these substeps:
   1. Set newResponse's body to response's body.
   2. Let dummyStream be an empty ReadableStream object.
   3. Set response’s body to a new body whose stream is dummyStream.
   4. Let reader be the result of getting a reader from dummyStream.
   5. Read all bytes from dummyStream with reader.
3. Set the potential response to newResponse.

response.body from the script surface seems okay since by the time respondWith(r) returns r's body should be disturbed already.

Or, set the newResponse's body to the result of extracting the package data out of response's body's stream will do?

[annevk]

If you want to clone a response, use the algorithms provided in Fetch. (Although those do not account for what follows at the moment, unfortunately.)

If we take a step back and look at how this maps to JavaScript, I think respondWith() is pretty much required to "structured clone" and message that clone to the environment making the fetch. What exactly "structured clone" means here is not really defined and what it means for streaming is even less understood. (And of course, user agents might not have to "structured clone" and be able to share memory more efficiently if developers avoid poking the painful bits, but we don't have to define the fast paths in the specification, just encourage them.)

[wanderview]

I'm confused. (As usual.) Why do we need to clone at all? Can't respondWith() just be spec'd to read the stream from the response per normal? It seems this should leave the stream disturbed/locked just as if javascript had read the stream.

[annevk]

How do you get the bytes read across realms (and the Response object itself), from a JavaScript perspective?

[yutakahirano]

My understanding is our plan is to use pipe. Something like this.

1. Let response2 be a response copied from response except for its body.
2. If response's body is non-null:
   1. Let {rs, ws} be an identical transform stream.
   2. Call pipeTo on response's body's stream with ws.
   3. Set response2's body to a new body with rs, response's body's transmitted bytes and response's body's total bytes.
3. Set the potential response to response2.

[wanderview]

From an implementation perspective we effectively serialize the Response into a C++ representation. That may or may not become another Response object depending on what initiated the request.

So I guess in spec language I would say you need to create a new internal response (lower case r) with a pipe for a body. Then the original body stream is .pipeTo()'d the new response's body. (Which I think matches what @yutakahirano just posted.)

[annevk]

Yeah, that sounds reasonable. (And would indeed map to some kind of structured clone operation in an all-JavaScript-world.)

[wanderview]

Yeah, that sounds reasonable. (And would indeed map to some kind of structured clone operation in an all-JavaScript-world.)

Except that "cloning" implies that the original object is still usable. This is consuming the original object. Maybe I'm splitting hairs, but I think its an important distinction and differs from what structured cloning normally does AFAIK.

[jungkees]

pipeTo says the method is still in some flux. So, I just temporarily changed the steps (6afc3d8) to #850 (comment) with leaving an issue note to use #850 (comment) when it's ready.

Also, it'll be great if you can make the pipeTo an extracted algorithm so other algorithms can directly invoke. (/cc @yutakahirano, @domenic)

[annevk]

@wanderview "structured cloning" has a concept of transfering and detaching the original object, sure, and that happens for body, but e.g., Headers is still fully functional (and therefore cloned).

[jakearchibald]

Pre F2F notes: I think this is just spec work remaining.

[jungkees]

F2F: @yutakahirano is working on #934. I think the PR will resolve this issue.

[jungkees]

I'll backport 6c2b3f1 to V1.