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

Allow service-workers mode to be set in fetch options #492

Open
n8schloss opened this issue Feb 17, 2017 · 14 comments
Open

Allow service-workers mode to be set in fetch options #492

n8schloss opened this issue Feb 17, 2017 · 14 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest needs tests Moving the issue forward requires someone to write tests topic: api

Comments

@n8schloss
Copy link

Following up on w3c/ServiceWorker#1026 (comment) there are quite a few times where it would be useful for developers to specifically bypass service workers when issuing a fetch request. Is this something we can expose in fetch options? Perhaps something like 'use-service-workers' like @jakearchibald proposed in the linked issue?

@annevk
Copy link
Member

annevk commented Feb 18, 2017

fetch(..., { serviceWorkers: "foreign" }) with "all", "none", and "local" as other potential values makes some amount of sense to me, though it probably needs to be a slightly more complicated due to the ability to use fetch() in a service worker.

@annevk annevk added the needs implementer interest Moving the issue forward requires implementers to express interest label Feb 18, 2017
@n8schloss
Copy link
Author

@annevk, that sounds awesome! Yeah, imo within a service worker the only valid options should be "foreign" or "none" or something like that.

@mfalken
Copy link

mfalken commented Mar 3, 2017

This is a good topic for the next service worker F2F. But as an implementor, I'm feeling the developer shouldn't need to worry about whether the fetch goes through the SW for perf purposes. I'd rather make the pass-through fetch event as close to free as possible without requiring the dev to explicitly opt-out of SW. Also we should work out how this relates to the static routes proposal.

@n8schloss
Copy link
Author

@mattto, there's always the possibility of having to wait for the service worker event loop and starting and doing IPC's to talk to a service worker will never be free. Like I said in the issue linked to above "If we send 40 or so requests from the window at once, while the first few may be quick, the other events get tacked onto the end of the Service Worker event loop and end up getting delayed quite a bit before they can hit the network". There are many cases when the developer knows that something won't be available from within the service worker and in those cases it's silly to hurt performance by going through the sw when the developer already knows that it's a waste.

In terms of how this relates to static routing, this is orthogonal. Static routing is declarative and is not flexible to cases when we know that we have a new url that the service worker has no clue about.

@wanderview
Copy link
Member

wanderview commented Mar 3, 2017

It seems to me we expose "skip the service worker" as an algorithm primitive within the browser for non-perf reasons. Sometimes you functionally want things to bypass the service worker. I don't see a reason to hide this primitive from content.

In terms of perf:

"If we send 40 or so requests from the window at once, while the first few may be quick, the other events get tacked onto the end of the Service Worker event loop and end up getting delayed quite a bit before they can hit the network"

Its possible that the event loop is not actually busy, but that the round trip to the worker and back to the main thread to resume the channel is running into main thread jank. This is something that browsers can eventually optimize.

Anyway, that's my suspicion based on the testing I did over in:

w3c/ServiceWorker#756 (comment)

I guess the case that a skip-service-worker flag would really help is where you believe the response may be in a memory cache. For example, chrome's rendered side network cache or something like a stylesheet memory cache. In those cases bypassing the service worker check could dramatically speed things up. Related to that:

w3c/ServiceWorker#962

Edit: I guess I should note I think chrome does skip the service worker today if it uses a cached stylesheet. Kind of vague if this is per spec or not.

@annevk
Copy link
Member

annevk commented Mar 13, 2017

A concern of sorts that was raised on Twitter by colleagues is that this functionality weakens the ability of a service worker to be control of all resources that are loaded over the network by the application.

The application can currently bypass the service worker with WebSocket and if you control that through CSP you can no longer use fetch()...

Perhaps we should offer CSP control over whether fetches are allowed to bypass the service worker at all (or perhaps only with a nonce). That might make it slightly easier to control the network without having to inspect all scripts to see whether they use this feature.

@n8schloss
Copy link
Author

n8schloss commented Mar 13, 2017

@annevk, anything that's making a fetch request from the window client can also unregister the service worker controlling that client, so I don't think we need any extra protection there :)

@jakearchibald
Copy link
Collaborator

F2F: No objection to this setting, unsure about naming. "all" and "none" are fine. Empty string is the default.

It'd be nice to have "all" be settable in the service worker. But Chrome and Firefox say that would be very hard to implement.

V1: "none" and "".

@annevk
Copy link
Member

annevk commented Apr 3, 2017

"all" in the service worker means you go through yourself? As we already require for notification resources and some other things? Why is that hard for this, but not there?

@asutherland
Copy link

A concern is infinite loop detection and prevention. To do that, we need to be able to associate the resulting fetch(es) with the source fetch. The concern could be mitigated if the recursive fetch established an explicit link to the source/initiating fetch request at fetch() call time.

Otherwise you get into inference that depends on us reliably propagating the root cause through various async API calls. And in the face of global state, that inference can fall down. (For example, issuing a fetch in a setTimeout scheduled by a different, non-recursive fetch event. For example, a SW that tried to aggregate requests by using setTimeout to trigger a flush after waiting "long enough".) Since we expect SW's to potentially service a large number of fetch events, simpler approaches like a token bucket where we give the SW n recursive tokens every time it gets a non-recursive fetch event, a lot of wasteful recursion could happen before we stop.

@annevk
Copy link
Member

annevk commented Apr 4, 2017

Is that not going to be a problem with foreign fetch too? (Sounds like maybe we could allow this in combination with controllers/observers.)

@wanderview
Copy link
Member

wanderview commented Apr 6, 2017

Is that not going to be a problem with foreign fetch too?

It is a concern for foreign fetch as well and something we have talked about a lot at past meetings.

The other part of it is that service workers have had "can't be intercepted by itself" as an invariant for a long time. Its likely there are bits of code relying on that in places through the implementations. Even stuff like notification icons is not really initiated directly by the worker thread, so its likely not sorted out by implementing that.

I think I would adjust Jake's "would be very hard to implement" to "harder to implement than it would appear". Not impossible, but would take some work. We want to implement the ability to bypass the service worker first.

@annevk
Copy link
Member

annevk commented Apr 7, 2017

If someone can write the web-platform-tests I'm happy to do the Fetch-side of this (though I welcome contributors for that too of course).

@annevk annevk added addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests and removed needs implementer interest Moving the issue forward requires implementers to express interest labels Sep 5, 2017
@annevk annevk added topic: api needs implementer interest Moving the issue forward requires implementers to express interest labels Jan 30, 2021
@annevk
Copy link
Member

annevk commented Jan 30, 2021

Restoring needs implementer interest as I'm not sure what the situation is nearly four years later. Thoughts?

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 needs tests Moving the issue forward requires someone to write tests topic: api
Development

No branches or pull requests

6 participants