[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch body streams are not full duplex #1254

Open
lucacasonato opened this issue Jun 16, 2021 · 38 comments
Open

Fetch body streams are not full duplex #1254

lucacasonato opened this issue Jun 16, 2021 · 38 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: streams

Comments

@lucacasonato
Copy link
Member

With request body streams and response body streams one would think it is now possible to use fetch for full duplex communication over HTTP, like in the example below:

const intervalStream = new ReadableStream({
  start(c) {
    let count = 0;
    const timer = setInterval(() => {
      console.log("sent!");
      c.enqueue("Hello\n");
      if (count === 5) {
        clearInterval(timer);
        c.close();
      }
      count++;
    }, 1000);
  },
}).pipeThrough(new TextEncoderStream());

const resp = await fetch("https://full-duplex-server.deno.dev/", {
  method: "POST",
  body: intervalStream,
});

console.log("foo");

const reader = resp.body.pipeThrough(new TextDecoderStream()).getReader();

while (true) {
  const { value, done } = await reader.read();
  if (done) break;
  console.log(value);
}

console.log("done!");

In this example the server just pipes the incoming request body stream into the response body steam with no modification.

If full duplex communication was working, you would see "sent!" and "Hello\n" being logged in close proximity to each other, live. This is however not the case, because the promise returned from fetch does not resolve until the entire request body stream has been sent and closed.

The solution here would be to resolve the Promise<Response> returned from fetch before the request body stream has completed. To do this, body upload streaming has to happen in parallel. The main issue with the spec in this area is that the promise returned from fetch is expected to reject if a chunk of the request body stream is not a Uint8Array (behaviour is tested in https://staging.wpt.fyi/results/fetch/api/basic/request-upload.any.html).

Are there any plans to resolve this shortcoming before request body streaming is sthbilized in browsers? HTTP/2 and QUIC are very capable duplex stream protocols (HTTP/1 is theoretically also capable) and it would be a shame if that power was not exposed to the web platform.

@lucacasonato
Copy link
Member Author

To be clear, I am not explicitly vouching for duplex streams to make it into the initial request body uploading implementation in browsers, I just want to make sure that it would be possible to add down the line. Deno implements fetch w/ request body streaming, and our implementation is capable of full duplex too, but it has to deviate from the spec to allow that. The backing HTTP implementations for both Node.js and Servo are also capable of full duplex body streams.

@annevk
Copy link
Member
annevk commented Jun 16, 2021

See #229 for some history. Since then, I've learned that this would require a lot of work in browsers and it's not planned to be supported initially.

However, I do think we should allow for it in theory, so if you could elaborate on what prohibits it that would help.

@lucacasonato
Copy link
Member Author
lucacasonato commented Jun 16, 2021

so if you could elaborate on what prohibits it that would help

The promise returned from fetch does not resolve until the entire request.body ReadableStream has been read and uploaded. This is fundamentally incompatible with duplex streaming, as you can not get a handle to the response.body until you close request.body.

These tests demonstrate that issue: https://github.com/web-platform-tests/wpt/blob/master/fetch/api/basic/request-upload.any.js. It is expected that the promise returned from fetch rejects if a chunk that is pulled off the request.body is not a Uint8Array. Instead this error should be communicated via cancelling the request.body ReadableStream the given error.

@annevk
Copy link
Member
annevk commented Jun 16, 2021

Does the specification also have such issues?

Now that you mention that example though I realize it is rather tricky if not impossible to do well here and be forward compatible. I suspect that applications will have the same kind of dependencies and browsers changing this will break them.

@sleevi
Copy link
sleevi commented Jun 16, 2021

Yeah, there seems some overlap with #95 / #538 in terms of upload/request processing.

@annevk
Copy link
Member
annevk commented Jun 16, 2021

cc @yoichio @yutakahirano

@lucacasonato
Copy link
Member Author

Does the specification also have such issues?

As far as I can tell, yes. The request body is sent synchronously before receiving response headers (not in parallel). See the "To transmit request’s body body, run these steps:" section in HTTP-network fetch step 8.5. Essentially what needs to happen is, that this needs to be changed to happen in parallel with receiving the response headers.

Now that you mention that example though I realize it is rather tricky if not impossible to do well here and be forward compatible.

To elaborate on this: many (likely most) web servers are programmed to only send back response headers after the request body has been received and processed (from now on referred to as category one servers). Because the promise returned from fetch would only resolve once the response headers are received. For the user the behaviour is that "fetch completes after the request body is fully sent". This behaviour would not change once full duplex streaming is implemented in browsers because the response headers are not received by the browser until the request body has been sent (because only then are they sent by the server).

For servers that send back response headers before the request body has been received (from now on referred to as category two servers), the promise returned from fetch would still only resolve once the request body is fully sent (because as you say, browsers do not support full duplex HTTP right now). Once browsers implement full duplex HTTP the promise returned from fetch would resolve after the response headers are received, regardless of the state of the request body. This is technically a minor change in behaviour, but one that could be marked as a spec compliance bug that Chrome, FF, and Safari have. The Deno or Servo implementations could implement full duplex right away.

Because the Python WPT test server probably falls into category one, the WPT I mentioned above is technically correct, but only under the assumption that the server sends headers after the response body is received. For cases where you need to handle both category one and category two servers you should handle the "cancellation" of the ReadableStream. This would be the only way to get notified of request body errors for category two servers.


TLDR, to solve this following would need to be resolved in the spec:

  • Request body sending in HTTP-network fetch should happen in parallel to waiting for response body headers.

@lucacasonato
Copy link
Member Author
lucacasonato commented Jun 16, 2021

One question this brings up: if request body sending on HTTP-network fetch happens in parallel, how and where are errors during request body sending propagated to the user?

@annevk
Copy link
Member
annevk commented Jun 16, 2021

In the case where the caller is not using upload streams? I suppose they would need FetchObserver (#607).

@lucacasonato
Copy link
Member Author
lucacasonato commented Jun 16, 2021

Yeah. For users that are using upload streams it can be surfaced as a cancellation of the stream.

Maybe the error could be surfaced as a rejection of the promise returned by fetch if the response headers have not yet been received, or alternatively as an error on the response body stream. Sort of like how aborting fetch works, but with a custom error message (the error that request body upload errored with), instead of an AbortError.

This would introduce the constraint that request body uploading must not outlive response body download. Good chance that is already a HTTP spec constraint anyway, but not sure.

@ricea
Copy link
Collaborator
ricea commented Jun 17, 2021

My view:

  • HTTP is a half-duplex request/response protocol and should remain so
  • WebSocket or WebTransport should be used where full-duplex is required. In particular, they have mitigations against request smuggling.
  • My team would be responsible for implementing this in Chromium, and even if we decided we wanted to do it, we don't have the resources to do the necessary rearchitecting of the network stack. Even just upload streaming has proven more challenging than expected.
  • The Fetch Standard should continue to reflect what browsers actually implement.

@annevk
Copy link
Member
annevk commented Jun 17, 2021

@ricea the assertion that HTTP is half-duplex seems in contradiction with it allowing the client to consume the response before it has transmitted the request body. I tend to agree with regards to the cost/benefit tradeoff and what Fetch should specify (although I would like for things to be cooler).

@lucacasonato
Copy link
Member Author

@ricea Some comments:

HTTP is not half duplex. It allows for simultaneous request body and response body streaming at a per request level. This is supported in both HTTP/1.1. and HTTP/2. Here are some discussions around the topic from 8 years ago: https://datatracker.ietf.org/doc/html/draft-zhu-http-fullduplex#section-2.1. Note that this is full duplex on a per request level, not full duplex on a connection level (the latter is only supported in HTTP/2, and browsers make ample use of it). Go, Node.js, and many other runtimes support full-duplex HTTP body streams.

Regarding request smuggling: full-duplex is not any more vulnerable to this than any other request body streaming (#966). Whatever the ultimate restrictions are that result from the #966 discussion, they would apply to full-duplex request body streaming too.

Regarding the effort required for implementation: it is very understandable that there are no resources to implement this now. My concern is that with the current spec wording there is also no way to implement it in the future, even if opinion on this matter changes. We are effectively locked into half-duplex for fetch forever. I think the specification should leave room for specifying this behaviour in the future - who knows what happens 5 years from now :-)

Also some non-browser engines implement the Fetch specification now, or are planning to in the future (Deno, Node.js, Cloudflare Workers). I think it is worth discussing if the specification should cater to these audiences too - I think APIs being the same across browsers and servers is a great thing that nearly no other language can provide. Also the Servo network stack is already capable of full-duplex HTTP.

@domenic
Copy link
Member
domenic commented Jun 17, 2021

Regarding the effort required for implementation: it is very understandable that there are no resources to implement this now. My concern is that with the current spec wording there is also no way to implement it in the future, even if opinion on this matter changes. We are effectively locked into half-duplex for fetch forever. I think the specification should leave room for specifying this behaviour in the future - who knows what happens 5 years from now :-)

That's not really how specifications work (at least in the WHATWG). Implementations aren't prohibited from implementing things because the spec doesn't support them. Instead, implementations go ahead and implement ahead of or in parallel with spec work, and then bring their findings back to the spec for discussion. If there's multiple implementers interested in shipping something, then we change the spec, around the same time as it ships.

So I don't think you should be worried about the fact that the spec reflects implementations. That's working as intended. The spec can change, if resources to implement this free up.

@ioquatix
Copy link

Having implemented full-duplex HTTP clients and servers for Ruby, I fully support @lucacasonato's position and think we should guide the fetch spec towards being full duplex.

If browsers can't support it, that's understandable, but if someone wants to implement a browser (or client) that does, it should not be prevented from being compatible with the spec here IMHO.

@benbucksch
Copy link
benbucksch commented Jun 13, 2022

[Problem:] the promise returned from fetch does not resolve until the entire request body stream has been sent and closed.
The solution here would be to resolve the Promise returned from fetch before the request body stream has completed

Agreed. That would also be the expected behavior for upload-only streams. I want to know whether the connection is properly established, the promise to resolve, and then start streaming. I.e.:

I would expect the promise to resolve in the exact moment when the upload stream can start.

@benbucksch
Copy link

For download streams, the connection and streaming body are 2 separate steps, which can be awaited separately. The same would make sense for upload.

@annevk
Copy link
Member
annevk commented Jun 13, 2022

So what API changes would we make here and how are they observable in half-duplex implementations? Would we end up with hanging promises? (I don't think the upload stream racing with the response headers would be a good forward-looking design. You don't want to depend on that with regards to forwarding the error.)

Is that all we'd need to change? Is that a change we can make? I suspect you can already run into this scenario with blobs and I/O errors, for instance.

I think it's worth exploring this a bit, but if it turns out the existing API has half-duplex baked in already than @yutakahirano's suggestion of a fullDuplex opt-in seems like the way to go. And we could make that throw today so it'll be forward-compatible a couple years from now.

@annevk
Copy link
Member
annevk commented Jun 13, 2022

To be clear, I don't think we need to resolve the promise returned from fetch() any sooner. Resolving it once we have the response headers in is fine. That's the signal. If you care about the request you monitor its stream.

@benbucksch
Copy link

[not] upload stream racing with the response headers

Yes, indeed, I would expect the response headers to have returned before the fetch() promise returns. Once the fetch() promise returns, I would immediately start sending stream data. That's a simple and logical API.

@ioquatix
Copy link

To be clear, I don't think we need to resolve the promise returned from fetch() any sooner. Resolving it once we have the response headers in is fine. That's the signal. If you care about the request you monitor its stream.

This is also how I've implemented it in Ruby and it works well. You can in theory implement streaming headers, but I'm not sure there is an actual good use case for it.

@annevk
Copy link
Member
annevk commented Jul 1, 2022

Now that #1457 has merged this becomes a feature request.

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Jul 1, 2022
@lucacasonato
Copy link
Member Author

FYI, Chrome only shipping half duplex has caused definitely user confusion. Users are expecting full duplex (without knowing that is what it is called), and as such even when specifying duplex: "half" (without knowing why, Chrome just says you have to add it) they do not understand that they are only getting half duplex.

For example, this Deno user expected full duplex streaming and was disappointed / confused as to why only half duplex is working: https://www.reddit.com/r/Deno/comments/150wet7/why_doesnt_servetls_stream_data_from_a/

@yutakahirano Is Chrome open to reconsider its stance on implementing full duplex if we do the spec work & WPT tests?

@lucacasonato
Copy link
Member Author
lucacasonato commented Jul 18, 2023

I'd also be interested to hear from WebKit (@annevk) and Gecko (@valenting) on this issue

@ioquatix
Copy link

Chrome only shipping half duplex has caused definitely user confusion

I'm honestly not surprised by this, and I would have expected full duplex by default, and I support making this full duplex by default if possible.

@ricea
Copy link
Collaborator
ricea commented Jul 21, 2023

@yutakahirano Is Chrome open to reconsider its stance on implementing full duplex if we do the spec work & WPT tests?

Yutaka no longer works on Chrome. My response is unchanged from #1254 (comment).

@ioquatix
Copy link
ioquatix commented Jul 21, 2023

@ricea HTTP/2 specifically advertises bi-directional streaming as a feature, and HTTP/1 makes no such assertions about being half-duplex AFAIK. Can you clarify your assertion "HTTP is a half-duplex request/response protocol and should remain so" - ideally with references to relevant RFCs if possible.

@ricea
Copy link
Collaborator
ricea commented Jul 21, 2023

The response is not sent until the request (including body) has been completely received. That's what I mean by "half-duplex".

Smart people have been known to disagree about what the HTTP RFCs say on this point, so I am going to stay out of that rabbit hole.

@ioquatix
Copy link

Do you mind pointing us at the statements regarding "half-duplex" in the RFCs that "Smart people have been known to disagree about"?

@annevk
Copy link
Member
annevk commented Jul 22, 2023

@ioquatix I don't think that inquiry is a productive use of everyone's time. Fact is that in all browser engines full duplex would be a substantial undertaking and alternatives such as WebSockets and soon WebTransport are available. The benefits and demand for full duplex HTTP would have to be quantified quite a bit to convince anyone I suspect.

@ioquatix
Copy link

What level of quantification is required? Because there are quite a few people on this thread who are already convinced it's a requirement.

If you ask for quantification, but that bar impossibly high or ambiguous to the point where it cannot be satisfied, that's not a productive use of anyone's time either.

If the point is "we standardise what the browsers do" and "my view (chrome) is that HTTP is half-duplex", then we are basically at an impasse as far as I can tell.

I have no problem championing a feature that I think is incredibly useful and important, but if the answer is simply "we don't want to do it for reasons", then let's not go down the path of trying to "quantify quite a bit to convince anyone".

On the other hand, if this is a genuine call to quantify whether such a feature is required, let's set the bar appropriately and then make a binding decision when appropriate evidence is collected.

@piranna
Copy link
piranna commented Jul 22, 2023

When HTTP was being used just for request-response, It made sense of being half-duplex: made a (small) request, finish to fully receive It, process, and send response back. But now we can send streams, not just "fixed size" requests, so not only for performance reasons make sense to start processing the request without fully receiving It, but also send back the response, but also from an users and developer PoV makes sense too. Servers are already working like that for performance reasons (for example Node.js), missing part to standarize It are browsers. I know there can be problems if an issue appears in the middle of receiving the requests and the response started to be send, that's the kind of details the spec needs to solve, but for the "happy path", technically IS totally feasable to implement.

@annevk
Copy link
Member
annevk commented Jul 24, 2023

@ioquatix I don't speak for Chrome. I sometimes speak for WebKit (and in particular Apple's port), and used to sometimes speak for Gecko, but I'm also the editor of the Fetch Standard and responsible for this repository.

https://whatwg.org/working-mode#changes has the requirements for changing a WHATWG Standard. You will need implementer support. In the past, web developer demand for certain features as well as end user benefit has been able to sway implementers. No guarantees for the future, but it seems likely that still holds.

So how would you go about quantifying something? Showing lots of questions on Stack Overflow, existence of JS libraries that paper around the problem using WebSockets or WebTransport, demonstrating deficiencies in workarounds, etc. This can take a long time, doesn't always work out, and even when there is reasonable demand it might still not be enough to persuade all relevant parties. But it has generally worked which is why we keep issues requesting new features open.

@piranna

This comment was marked as duplicate.

@ioquatix
Copy link
ioquatix commented Jul 26, 2023

Okay, that's a fair enough level of quantification and I think we can aim for that. Is it okay if I start collecting references to those things on this thread or should I document it elsewhere?

Evidence of User Difficulty

Evidence that HTTP is Full Duplex

From Roy Fielding himself:
image

Evidence of Implementations

  • coming soon.

@annevk
Copy link
Member
annevk commented Jul 26, 2023

Here is fine, but maybe continue updating that comment so everyone watching the repository doesn't get a stream of small updates. (I also don't think we need evidence HTTP is full duplex. Apart from Adam I haven't seen anyone contest that.)

@helmturner
Copy link
helmturner commented Oct 31, 2023

@ioquatix I'd like to add to that. I recently started contributing to tRPC, and the implementation of streaming functionality (in particularly high demand thanks to ChatGPT) is being partially blocked by this.

The use case for us was the ability to batch requests that include streamed responses. In order to cancel a single stream without cancelling the entire batch we need to get a message from server to client mid-response, potentially when that server is a stateless 'serverless' function. We already eagerly write minimal Headers, and the intent would have been to use Trailers where necessary.

Without this, users are forced to pick between either batching OR streaming, or they have to implement some pretty painstaking request orchestration.

Re: 'Implementer support'... huh? As was already mentioned many times, server runtimes are implementing this already. In fact, it wasn't until our group read the spec, built a POC in node, AND did extra in-browser testing that we found out this was a no-go.

Finally, I want to add to the long list of obvious use-cases: simple push-messaging in React Native apps, without being forced to use Messaging APN providers.

gRPC and Firebase are perfect examples of how useful this sort of thing would be (though, also probably a factor in why this isn't supported by the Chrome team...)

@benlesh
Copy link
benlesh commented Feb 23, 2024

Commenting to illustrate need:

I have a direct need for this as well. Working on a private code base, but the use case is we have a lot of streaming that we're currently multiplexing over a WebSocket. Most of these streams are simply "1 request, N responses", however 2 or 3 of them are bidirectional. Moving to http/2 for "1 request, N responses" case would be a boon for us, because we'd be able to let http/2 do the multiplexing for us, less JavaScript, less code running on the server and the client (because there's no need to wrap something in an "envelope" and match it up on both sides). The lack of bidirectional communication (full duplex) in fetch is really the only thing holding me up from moving everything over to http/2.

There's always WebTransport of course, but I feel like http/3 is an even taller order, I don't think it's supported in Node yet, and it's unlikely that the support is great in other languages our teams deal with.

pdonias added a commit to vatesfr/xen-orchestra that referenced this issue Apr 29, 2024
If the body of a request is a stream, we could start receiving the response
before the request has been fully sent (full duplex).
However, Node does not support full duplex. Since other JS runtimes do support
full duplex, Node now requires to pass "duplex: 'half'" for compatibility
concerns with those JS runtimes.

Spec: https://fetch.spec.whatwg.org/#ref-for-dom-requestinit-duplex
WHATWG discussion: whatwg/fetch#1254
Node discussion: nodejs/node#46221
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest topic: streams
Development

No branches or pull requests

10 participants