Allow a ReadableByteStream in `fetch({body: <body>})` #577

ConradIrwin · 2017-08-11T18:18:55Z

I'd like progress events on uploads. I think this should work according to the current spec, but it doesn't seem to work in practice, as I'm not sure how to tell the fetch "how much stream" it should read.

  function upload (url, blob, onUploadProgress) {
    let progress = 0
    let readableByteStream = new ReadableStream({
      type: 'byte'
      pull: function (controller) {
        onUploadProgress({todo: blob.size, done: progress})
        let nextProgress = progress + controller.desiredSize
        controller.enqueue(@blob.slice(progress, nextProgress))
        progress = nextProgress
      }
    })

    return fetch(url, {
      mode: 'cors'
      method: 'POST'
      headers: {'Content-Type': blob.type}
      body: readableByteStream
    })
  }

annevk · 2017-08-12T10:14:12Z

The reason it doesn't work in practice is because it isn't implemented yet.

ConradIrwin · 2017-08-14T17:31:37Z

Nice! But this will be how it works?

Looking forward to it — had to roll out an XmlHttpRequest to handle uploads last week :(.

wanderview · 2017-08-14T18:08:55Z

You should be able to use fixed sized bodies (string, blob, etc) for upload streams today. So if you did an XHR thing I think you should be able to do the same with fetch().

Note, there is a limitation on what servers will support streamed uploaded. It requires chunked encoding which pretty much limits it to h2. (You can't negotiate chunked encoding in h1.1 until after request has been sent, which is too late.)

annevk · 2017-08-14T18:13:27Z

@wanderview that shouldn't be too late really (and H/2 doesn't do chunked encoding). You can just send the request and if the server affirms you can start sending the request body. Though it also seems fine that if ReadableStream is passed you just assume support since it won't work anyway otherwise.

wanderview · 2017-08-14T18:18:20Z

I guess I will refer you to @mcmanus. I believe this is what he told me.

ConradIrwin · 2017-08-14T18:21:07Z

How does the XmlHttpRequest upload progress event work? I naively assumed it was just periodically pinging me with the number of bytes that had been ACKed by the upstream server. (I'm uploading to Google Cloud Storage, and it seemed to work automatically).

I was hoping the same would happen for fetch() where I could see at a low level what bytes were written to the network, and when.

wanderview · 2017-08-14T18:24:48Z

XHR progress is based on browser writing bytes to OS socket. Not based on server acks.

Fetch API needs a progress interface added to get equivalent behavior. The stream reading wont be quite the same since buffering may take place.

ConradIrwin · 2017-08-14T20:12:55Z

It depends how much buffering I guess. It looks like the XHR equivalent is pretty low fidelity (and if it's just bytes written, not bytes acked, also subject to queueing and over-promising).

For what it's worth, we have an internal fetch() wrapper that takes an onProgress callback, and falls back to XHR if it's specified. It seemed nicer than trying to figure out an evented way of doing it given that fetch() doesn't have an instance that you can attach events to.

fetch(url, {
  body: blob,
  method: 'PUT',
  onProgress: ({status, loaded, total}) => {
    // status is one of "loading" (before the returned promise resolves or rejects), "success" (after the returned promise resolves), "error" (after the returned promise rejects)
    // loaded is the bytes transferred as reported by XmlHttpRequest's ProgressEvent
    // total is the total bytes as reported by XmlHttpRequest.
  }
})

I also considered having fetch() return a thenable event emitter instead of a promise; but because we're using a mix of promise libraries already, that seemed like a recipe for confusion.

Would love to see an official solution to this problem, either by specifying how much buffering is allowed in the ReadableStream case, or just adding a callback.

annevk · 2017-08-15T06:48:22Z

@wanderview I don't see anything about that in https://tools.ietf.org/html/rfc7230#section-4 and haven't heard this concern until now. Why did nobody raise an issue? @mcmanus?

mcmanus · 2017-08-15T13:27:21Z

due to the lack of threading here I can't figure out the question I'm being asked. can you rephrase?

wanderview · 2017-08-15T13:43:54Z

due to the lack of threading here I can't figure out the question I'm being asked. can you rephrase?

Patrick, the question is can we support chunked encoding uploads without a known fixed size? I thought you had told me this was only possible in h2.

@wanderview I don't see anything about that in https://tools.ietf.org/html/rfc7230#section-4 and haven't heard this concern until now. Why did nobody raise an issue?

I am pretty sure we've had this conversation before and I did bring it up earlier both with you and @domenic. I can't find the issue now, though. It may be buried in some tangential streams spec issue.

wanderview · 2017-08-15T13:47:15Z

Found it. It was in the fetch-with-streams repo. See yutakahirano/fetch-with-streams#30 (comment), for example.

annevk · 2017-08-15T13:49:00Z

@wanderview that became yutakahirano/fetch-with-streams#57 and the conclusion there was that this wasn't a problem.

mcmanus · 2017-08-15T13:55:00Z

Patrick, the question is can we support chunked encoding uploads without a known fixed size? I thought you had told me this was only possible in h2.

h2 only supports streamed bodies (true of both directions) - it doesn't call them chunked, but that's effectively what they are.

h1 only reliably supports chunk downstream on the Internet, though 1.1 specifies it in both directions so you could try it as long as you're willing to tolerate the errors which will come in a variety of forms (early, late, hangs, etc..).. this is really an e2e problem, so you'll still get some errors even if you apriori know the server deals with chunked uploads, or that you are speaking h2 (because those properties might not be true e2e).

wanderview · 2017-08-15T13:55:23Z

Yea, we basically punted the problem to the web developer. It may break with particular h1.x servers, but we're not going to protect you. The underlying problem hasn't really gone away. (I'm still not happy with that resolution.)

I think my advice above that you might want to plan to only use this with h2 is still reasonable.

jakearchibald · 2017-08-15T15:05:25Z

#95 (comment) – discussing how to enable developers to set Content-Length of streamed bodies so chunked encoding isn't needed.

annevk · 2017-09-04T14:02:25Z

Closing this due to lack of a problem with Fetch.

annevk closed this as completed Sep 4, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow a ReadableByteStream in `fetch({body: <body>})` #577

Allow a ReadableByteStream in `fetch({body: <body>})` #577

ConradIrwin commented Aug 11, 2017

annevk commented Aug 12, 2017

ConradIrwin commented Aug 14, 2017

wanderview commented Aug 14, 2017

annevk commented Aug 14, 2017

wanderview commented Aug 14, 2017

ConradIrwin commented Aug 14, 2017

wanderview commented Aug 14, 2017

ConradIrwin commented Aug 14, 2017

annevk commented Aug 15, 2017

mcmanus commented Aug 15, 2017

wanderview commented Aug 15, 2017

wanderview commented Aug 15, 2017

annevk commented Aug 15, 2017

mcmanus commented Aug 15, 2017 •

edited by jakearchibald

Loading

wanderview commented Aug 15, 2017

jakearchibald commented Aug 15, 2017

annevk commented Sep 4, 2017

Allow a ReadableByteStream in fetch({body: <body>}) #577

Allow a ReadableByteStream in fetch({body: <body>}) #577

Comments

ConradIrwin commented Aug 11, 2017

annevk commented Aug 12, 2017

ConradIrwin commented Aug 14, 2017

wanderview commented Aug 14, 2017

annevk commented Aug 14, 2017

wanderview commented Aug 14, 2017

ConradIrwin commented Aug 14, 2017

wanderview commented Aug 14, 2017

ConradIrwin commented Aug 14, 2017

annevk commented Aug 15, 2017

mcmanus commented Aug 15, 2017

wanderview commented Aug 15, 2017

wanderview commented Aug 15, 2017

annevk commented Aug 15, 2017

mcmanus commented Aug 15, 2017 • edited by jakearchibald Loading

wanderview commented Aug 15, 2017

jakearchibald commented Aug 15, 2017

annevk commented Sep 4, 2017

Allow a ReadableByteStream in `fetch({body: <body>})` #577

Allow a ReadableByteStream in `fetch({body: <body>})` #577

mcmanus commented Aug 15, 2017 •

edited by jakearchibald

Loading