Skip to content

Service worker request body reads need to invoke callbacks #87

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

Closed
annevk opened this issue Jul 22, 2015 · 9 comments
Closed

Service worker request body reads need to invoke callbacks #87

annevk opened this issue Jul 22, 2015 · 9 comments

Comments

@annevk
Copy link
Member

annevk commented Jul 22, 2015

The HTTP-network fetch algorithm currently queues "process request body" tasks, which also need to be queued by HTTP fetch as otherwise the service worker doing event.request.text() is not defined.

Similarly, text for response updating probably needs to be included.

Both of these changes also prepare us for streams.

@annevk
Copy link
Member Author

annevk commented Jul 23, 2015

I'm having a hard time wrapping my head around this one. We have a window/worker, a service worker, and the network stack. The document includes a request body in a fetch.

The service worker can decide to read this or pass it on.

Who gets notifications for read operations?

Note that for it to get from the document to the service worker, it likely had to be transfered. Might be the case for document to the network layer too, though.

@wanderview, any ideas?

@domenic
Copy link
Member

domenic commented Jul 24, 2015

Answering your question from IRC, which I understand more than the above:

A document invokes fetch() and passes a body. A service worker gets that request and passes it to fetch(). Can both the document and the service worker observe the stream being read from for the purpose of progress events?

I'm not sure exactly what you mean by "observe the stream being read from." I would say that only the person doing the reading will observe the reads happening. Who they want to pass that information on to is up to them.

In this scenario,

  • In-document fetch() reads from the in-document body, as part of the process of "transfering the stream" over to the service worker. (I.e., constructing a second stream object over in the service worker, and filling it with the chunks read from the original one.)
  • In-SW fetch() reads from the in-SW body, as part of the process of uploading it to the network.

In both cases, the party doing the reading can choose to tell other parties, or emit progress events, as appropriate.

Maybe the todo here is for whatwg/streams#276 to be sure to include a hook for other specs to say e.g. "transfer with chunk-action X" instead of just "transfer," to allow action during the transfer.

@wanderview
Copy link
Member

Example code:

var r = new Request(url, { method: 'POST', body: myStream });
fetch(r);

// intercepted in service worker as
addEventListener('fetch', function(evt) {
  var req2 = new Request(evt.request, { body: evt.request.body.pipeThrough(transform) }); 
  evt.respondWith(fetch(req2));
});

So we have two fetch() calls here; the intercepted fetch and the sw fetch.

I would expect the intercepted fetch to report upload progress when its Request.body is read. So in this case, when the .pipeThrough() reads chunks from it.

How do we plan on reporting back upload progress? Are we adding a callback to fetch() somewhere?

@annevk
Copy link
Member Author

annevk commented Jul 24, 2015

I think progress is either a feature of streams or when you'll have to implement yourself. So perhaps the answer is that XMLHttpRequest needs to somehow layer this on top of streams. The callbacks are mostly used by XMLHttpRequest. fetch() doesn't really use them.

@wanderview
Copy link
Member

Well, currently we report progress on XMLHttpRequest when the bits are sent to the OS to go on the wire. Always doing it on the stream read is not equivalent. I think we have to break equivalence for the interception case, but for a fetch() that is not intercepted it seems we could still report when it goes to the OS.

@domenic
Copy link
Member

domenic commented Jul 24, 2015

Hmm so it sounds like XHR's progress events are really fired by the under-specified (and definitely not user-exposed) "writable" part of the system.

@wanderview
Copy link
Member

Yes. This harkens back to our writable-vs-readable discussion for Request.body. I thought we had discussed a new progress callback would be added somehow to reflect the XHR onprogress event.

@annevk
Copy link
Member Author

annevk commented Jul 24, 2015

I guess for now I'll just keep both the service worker reading the request and the network writing the request the same, and use some hand-wavy wording for both to queue "process request body" tasks. Maybe with a note that this will get more detailed in due course once we have the transfer processing model and later writable streams.

@domenic
Copy link
Member

domenic commented Jul 24, 2015

I'm hoping to have the transfer processing model for you "shortly," although given TC39 next week it could be as late as the 4th.

@annevk annevk closed this as completed in e2f0a96 Jul 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants