Developer-controlled streams for request/response

[annevk]

First https://github.com/yutakahirano/fetch-with-streams needs to be integrated. So developers can access the stream of request/response. Next is giving them control over the stream.

• What if you do fetch(url, { body }) where body is a stream which gets random stuff (not bytes) enqueued in it?
• Result: when fetch reads from body, it notices a non-BufferSource, and ... does what?
  • Returned promise rejects
  • Cancel the stream
  • On the wire, it just terminates somehow? How does HTTP work?
• Require "cors-with-forced-preflight" (you need a preflight in most cases anyway, and it's also required for progress events, which streams enable) (in particular you need it for everything that isn't a POST with one of the <form> MIME types)
• Always do content-encoding: chunked, and do uploads as chunked encoding, which we need to spec in some way

[mnot]

On the wire, it just terminates somehow? How does HTTP work?

For H1, you'd just close the connection. For H2, you'd cancel the stream.

Always do content-encoding: chunked, and do uploads as chunked encoding, which we need to spec in some way

As noted elsewhere, that's not very interoperable with existing, deployed servers; while some will accept chunked encoding, most will 411 (Length Required) if there's no Content-Length, because they don't want to infinitely buffer the request.

One way around this would be to specify that if Content-Length is set, you'll only write that many bytes to the wire, and error if it tries to write more.

[annevk]

Basic tests: web-platform-tests/wpt#4362. Fetch Standard PR: #425.

[amn]

@annevk: Is chunked a valid value for Content-Encoding HTTP header? I think not? Perhaps you meant Transfer-Encoding: chunked?

[annevk]

@amn yeah, that's what we ended up with in step 4 and 5 of https://fetch.spec.whatwg.org/#concept-http-network-fetch.

[ioquatix]

I have played around with and implemented bi-directional streaming for both HTTP/1 and HTTP/2 in async-http.

While it's not possible to demonstrate in a web browser, here is a simple example of streaming from the server to the client: https://utopia-falcon-heroku.herokuapp.com/beer/index

In addition, using the async-http API directly, it's possible to write applications that stream in both directions, and in theory there is no reason why a web-browser couldn't do it too.

For H1, you'd just close the connection. For H2, you'd cancel the stream.

In some cases you can optimise this logic to avoid closing the connection, but this is roughly what async-http ended up doing.

most will 411 (Length Required) if there's no Content-Length

async-http and by composition, falcon (web server) both accept chunked request body. In addition, falcon accepts non-content-length bodies, but you must use TCP's shutdown(WR) functionality in order to signal the end of the body. With these bits in place, it's possible to have quite decent HTTP/1 bi-directional streaming, even for (a slightly extended at the TCP layer) HTTP/1.0!

I also implemented support for WebSockets, and it's such an orthogonal concept, especially in the design of the server, that I really believe and hope that we can build something better directly on top of HTTP/2. WebSockets seems like a step in the wrong direction.

[ioquatix]

So, what is the status of bi-directional streaming in the browser? Does it work yet?

[annevk]

No.

[ricea]

Bi-di H1 is not standards compliant, not compatible with middleboxes, and doesn't align with how browsers "think about" HTTP.

Bi-di H2 is technically possible. However, I think the principle from RFC6540 that "HTTP/2 provides an optimized transport for HTTP semantics." should be adhered to.

[annevk]

@ricea it is standards compliant, what makes you say it's not? It's just that some middleboxes make it hard to deploy at scale.

[ioquatix]

What part of the H1 spec disallows bi-directional streaming?

[ricea]

@ioquatix RFC 7231 5.1.1

o A server that responds with a final status code before reading the
entire message body SHOULD indicate in that response whether it
intends to close the connection or continue reading and discarding
the request message (see Section 6.6 of [RFC7230]).

[ioquatix]

@ricea That seems to apply to the "Expect" header. Additionally, I don't see how that prevents bi-directional streaming. Can you elaborate?

[ricea]

In other words, it's perfectly fine to send a response before you've received the entire request, but it can only depend on the parts of the request received up until that point. True bi-di would mean that later parts of the response depend on later parts of the request.

Anyway, I don't want to get into standards-lawyering. Standards can be changed. Real-world compatibility is far more important.

[ioquatix]

In other words, it's perfectly fine to send a response before you've received the entire request, but it can only depend on the parts of the request received up until that point. True bi-di would mean that later parts of the response depend on later parts of the request.

I'm not sure how you derive this interpretation from that one paragraph which appears to just talk about sending connection: close.

Anyway, I don't want to get into standards-lawyering. Standards can be changed. Real-world compatibility is far more important.

I don't think anyone here is standards-lawyering, I am genuinely interested in the standard prevents bi-directional communication. I would expect something like "The server must wait until the entire request is received before sending the response" or something to that effect.

I also agree that real-world compatibility is important.

[bzbarsky]

@yutakahirano I'm trying to understand why this test claims that various stream uploads should fail... They seem to succeed in browsers as far as I can tell.

[yutakahirano]

@bzbarsky which test are you referring to? https://github.com/web-platform-tests/wpt/pull/4362/files?

[bzbarsky]

Yes. All browsers currently fail the testUploadFailure tests, because the call to promise_rejects is missing the first argument (should have test before the new TypeError()). See https://wpt.fyi/results/fetch/api/basic/request-upload.any.html?label=master&label=experimental&aligned&q=fetch%2Fapi%2Fbasic%2Frequest-upload. But if I fix the test to pass in the right args, browsers still fail, because as far as I can tell they allow those uploads.

[yutakahirano]

Thank you for fixing the tests.

The failure is caused by https://fetch.spec.whatwg.org/#concept-request-transmit-body. It rejects any chunk other than Uint8Array.

[bzbarsky]

@yutakahirano OK, but either the test is not exercising this line or browsers are not implementing it. My suspicion is on the former, since browsers also fail the "Fetch with POST with ReadableStream" test, due to getting back "" instead of "Test"; this suggests that there is no data being sent to the server, and probably that no chunks are seen at all?

[annevk]

@bzbarsky I don't think any browser supports upload streams at the moment. (There's some outstanding issues with that too, see the streams issue labels.)

[bzbarsky]

Ah, I see. I would have expected lack of support to lead to failed responses, not success responses with empty bodies, but maybe they're just buggy around that too...

[annevk]

@bzbarsky from the name it looks like it's a thing designed to echo what was posted, which I would expect to always succeed. Usually that's preferable over encoding some of the test logic on the server part as then you'd need to look at that in order to debug the failures.

[bzbarsky]

Right, but my point is that I would expect browsers to fail the fetch rather than posting nothing when passed something they don't support? That is, my issue is with the sender-side behavior here, not the server.

[annevk]

I see, that's an unfortunate side effect of USVString being one of the accepted types. There's not much to do about that aside from going against IDL's wishes.

[bzbarsky]

I thought about that, but then I would expect the body to be "[object ReadableStream]" whereas it seems to be ending up as "" somehow in browsers... And yes, just looking at the IDL in Gecko for this I don't see why that would be.

[annevk]

It looks like a bug in the tests of sorts. If I add return before new ReadableStream I get the expected result. It seems the function returns undefined. I thought that only happened if you had a semicolon at the end, but I guess it's more complicated.

[bzbarsky]

Oh, indeed! That's using an arrow function with a block on the RHS of the =>, which does need an explicit return.

[annevk]

FWIW, patch at web-platform-tests/wpt#21718.

[gterzian]

Are we certain the WPT server is setup to handle the streaming/chunked requests of fetch/api/basic/request-upload.js ?

I am currently getting a bunch of:

 0:04.54 INFO STDERR: 127.0.0.1 - - [13/Jun/2020 18:22:08] code 400, message Bad request syntax ('4')
 0:04.54 INFO STDERR: 127.0.0.1 - - [13/Jun/2020 18:22:08] "4" 400 -

with the "Fetch with POST with ReadableStream" always failing due to the body apparently being empty.

I see the failure is experienced by all browsers, where the behavior hasn't been implemented yet, and there was another problem with the test discussed above, however I am actually struggling to make the test pass in Servo, where in theory it should work.

The problem might be client side, however after extensive debugging I'm looking for other places to blame :)

[annevk]

No, nothing else creates that kind of request so there could well be issues.

[gterzian]

Ok, I've opened an issue in WPT to track this: web-platform-tests/wpt#24139

[ioquatix]

Falcon works and has been tested, if you want a server which can accept both posted chunked, content-length (half closed) and HTTP/2 whatever. I think I included a link to an example above somewhere.