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

consider fetching service worker scripts with no-cache by default #893

Closed
wanderview opened this issue May 9, 2016 · 37 comments
Closed

Comments

@wanderview
Copy link
Member

wanderview commented May 9, 2016

Lately I've found myself recommending to people that they use Cache-control: no-cache or max-age: 0 on their service worker script resources. It seems best practice in order for service worker updates to be detected as devs expect.

Should Update algorithm just fetch the service worker script with Request.cache='no-cache' by default? One could argue that an "update check" is semantically similar to pressing the refresh button.

Edit: Removed the erroneous statement about the firefox http cache.

@delapuente
Copy link

delapuente commented Jun 6, 2016

This could be tweak for production environments by adding an option to the registration:

navigator.serviceWorker.register('worker.js', { cache: <policy> });

@jakearchibald jakearchibald added this to the Version 1 milestone Jul 25, 2016
@jakearchibald
Copy link
Contributor

I'm pretty open to this, it does seem like a gotcha. I use & recommend max-age=0 on all service workers. An option to re-enable the current behaviour would be nice too.

The only reason that it feels "wrong" is because avoiding the cache is the exception, but it does feel like the pragmatic option in this case.

Interested to know the opinion of those doing a lot of SW development. +@naskooskov @n8schloss @bmaurer.

@muralisr
Copy link

This would be really useful for companies providing services via service workers. We are deploying SW onto clients who can host scripts, but don't have a straight-forward way to set response headers.

@n8schloss
Copy link

So, for us the current behavior is actually kind of useful. When we push out new code for the site we'll slowly ramp things up and some Facebook servers will be on the newest version while some will be on the old version. In the middle of the code push for any individual request it's just as likely that someone will hit new code or that they'll hit old code. For service workers this means that during code push we often see service workers constantly in the upgrade state. We have some mechanisms to ensure that we don't install an old version of the service worker, however we did still enter the install state more than is ideal. To solve this we actually cache the service worker script for an hour, this allows a level of stability that would otherwise be hard to achieve.

@n8schloss
Copy link

(that being said, I do agree that defaulting to no cache with an option to re-enable the current behavior is probably a better developer experience)

@inian
Copy link

inian commented Jul 26, 2016

Coming from same perspective as @muralisr (we provide service workers to our clients and we are not in direct control), we find that they usually cache the service worker script similar to other JS files on their server, which tends to have a long cache time if they are doing things correctly. They should ideally add a separate server rule making the service workers script cache for a lesser time which they typically do not end up doing.

I like @delapuente's way of declaring the cache time during the service worker registration process.

@jakearchibald
Copy link
Contributor

Pre F2F notes: do we want to make this change? We shouldn't do it without offering an API to stick with the current behaviour. Should these options be part of the registration call, or in the activate event?

@jakearchibald
Copy link
Contributor

F2F:

  • It'd be great to make max-age opt in, but need to speak to big SW users first
  • Feels like this should be a setting on the install event

@jakearchibald
Copy link
Contributor

  • We should do the above for all imported scripts & modules

@jakearchibald
Copy link
Contributor

  • Even in the opt-in cache case, we still do the 24hr limit

@wanderview
Copy link
Member Author

@jakearchibald
Copy link
Contributor

At the moment third party servers can use max-age to reduce server load, are we kinda going against them here by making this opt-in by a different origin?

@jakearchibald
Copy link
Contributor

Regarding the opt-in: the concurrent fetch feature will sit on the registration object, so maybe this should too.

@AshleyScirra
Copy link

AshleyScirra commented Sep 5, 2016

I just ran in to this. There definitely needs to be a way to force updates for fetching the SW script. I have to reiterate that as middleware/framework developers we are always developing for servers we don't control, so tweaking HTTP headers is simply not possible. Note this is also the case if you don't control the host (e.g. Dropbox public folder, Neocities, Github pages etc).

I just tried testing on a server which defaults all .js files to cache with a max-age of 50 days, presumably set up to use versioned URLs. Of course there's no sensible way to use versioned URLs on the sw.js script if the main page is cached by the SW itself, so I kept getting the stale sw.js returned from cache with no apparent workaround other than to change the server configuration, wait 24 hours or click the manual "update" button in Chrome devtools. In our case none of these are good options for our framework. I don't mind which way you choose to default this, but I think it's essential to have some way of saying "bypass cache for SW updates".

@jakearchibald
Copy link
Contributor

Resolved:

navigator.serviceWorker.register('/sw.js', {
  useCache: true
});

@delapuente
Copy link

delapuente commented Sep 20, 2016

Just realised it's pretty similar to my first comment.

@Huxpro
Copy link

Huxpro commented Oct 2, 2016

Applause for this decision!!

@wanderview
Copy link
Member Author

I'm reviewing gecko code to implement this and came up with a couple questions:

  1. I assume this does not change Step 5.3 of Register which short-circuits the registration if its a duplicate of the same script already installed. Correct?
  2. Should we expose a getter on the registration so someone can inspect the value of the useCache attribute?
  3. Should we allow the useCache value to be changed in the install or activate events?

@jakearchibald, what do you think?

@wanderview
Copy link
Member Author

I asked Jake about the short-circuit in step 5.3. He suggests the short-circuit should only happen if the useCache value is the same:

9:31 AM JakeA: so we are implementing the "use no-cache by default on updates" thing in gecko and a question arose
9:32 AM the issue defines an API like register(scriptURL, { useCache: true })
9:32 AM the spec also short circuits registration if the scriptURL and scope are the same as an existing registration
9:33 AM our question is, should that short circuit still happen even if the "useCache" value has changed?
9:33 AM or should there be another way for a service worker to change its "useCache" value?
9:33 AM JakeA: ^^^
9:34 AM I'm inclined to implement without any ability to change useCache to start, but thought I would get your opinion
9:34 AM wanderview: changing the value of useCache in .register feels like the right way to update this value
9:35 AM wanderview: whether it causes an immediate refetch of the SW (like changing the url does) is another question
9:35 AM JakeA: so the bypass check should be "if script URL, scope, and useCache value are identical, then bypass" ?
9:35 AM wanderview: yeah
9:36 AM ok
9:36 AM JakeA: it would be easiest if it just acted the same as if a script URL changed
9:36 AM wanderview: that works for me then

@jakearchibald
Copy link
Contributor

jakearchibald commented Nov 29, 2016

Also, I agree we should have registration.useCache - a getter which reflects the current value.

jungkees added a commit that referenced this issue Dec 6, 2016
This introduces service worker registration's use cache field and its
related APIs: options.useCache to register method and
registration.useCache for ServiceWorkerRegistration objects. This
changes the default cache mode of fetching SW scripts to "no-cache".
After the change, 24 hours limit and job's force bypass cache flag rules
are still enforced. register() method's options value set to
{ useCache: true } re-enables the previous default behavior provided
before this change.

Fixes #893.
jungkees added a commit that referenced this issue Dec 12, 2016
This introduces service worker registration's use cache field and its
related APIs: options.useCache to register method and
registration.useCache for ServiceWorkerRegistration objects. This
changes the default cache mode of fetching SW scripts to "no-cache".
After the change, 24 hours limit and job's force bypass cache flag rules
are still enforced. register() method's options value set to
{ useCache: true } re-enables the previous default behavior provided
before this change.

Fixes #893 #894.
@jungkees
Copy link
Collaborator

I'm not sure how they can have any expectation of a certain QPS to scripts.

I thought of a case where "hosting 3rd party libraries" is done in github or some similar environment where server-side setup is not accessible to devs. Let me know if that doesn't make sense.

I feel pretty strongly that no-cache should be the default. This is based on helping numerous developers work through this issue. I feel like its the root cause of at least half the problems people bring me. Making it default cache is not going to help here.

No opposition. Just wanted to check if we can have a good compromise considering the end-to-end picture. But if opt-in to "no-cache" doesn't help much, I'd strongly support your idea.

blickly pushed a commit to google/closure-compiler that referenced this issue Dec 14, 2016
@mfalken
Copy link
Member

mfalken commented Dec 19, 2016

@wanderview
Copy link
Member Author

FYI, we've implemented this and landed it in FF53 (includes WPT tests):

https://bugzilla.mozilla.org/show_bug.cgi?id=1290944#c65

We are defaulting to no-cache right now with the useCache option available to opt into http cache.

This will ship in our release channel in April. If there is serious concern that we should switch the default please let us know before that point.

@JustinDrake
Copy link

JustinDrake commented Feb 3, 2017

I am in a situation where I want to build an immutable JavaScript secure loader. The secure loader only loads JS files with a suitable cryptographic signature. The 24 hours cache limit prevents me from building such a secure loader that persists longer than 24 hours. (The idea is that the secure loader would persist even if the servers that delivers index.html is compromised.)

Have you considered removing the 24 hours cache limit if explicitly opted-in with the useCache option?

@asutherland
Copy link

@JustinDrake Your use-case has been raised in some other issues as well. The short story is that the use-case is at odds with the intent of ServiceWorkers and the web security model. If you can create a benevolent SW that can never be removed so it can survive when the server is compromised, then an attacker can construct a nefarious SW that can never be removed the one time the server is compromised. And browsers can't distinguish between benevolent and nefarious, just whether the TLS cert is valid.

I think many of us are interested in this specific use-case, but SW is not going to be the solution for that on its own. The most practical solution at this time is to use the increasingly cross-browser WebExtensions efforts that are already built on a packaged/release model and APIs like webRequest that lets you intercept requests to specific sites and validate/enforce their contents, etc. It's also possible spec efforts like https://w3ctag.github.io/packaging-on-the-web/ may bear fruit at some point.

@albertsylvester
Copy link

albertsylvester commented Feb 7, 2017

Dear @jakearchibald need a little help here.
I have a service worker https://www.tavest.com/service-worker-tavest-test.js
Why somehow the service-worker-file is being cached by chrome? So when I update the service worker, the update didn't take place.

I've been also set this up in my .htaccess to make the service-worker not being cached, yet not successful.

  <filesMatch "^(service-worker)\.js$">
    FileETag None
    <ifModule mod_headers.c>
       Header unset ETag
       Header set Cache-Control "max-age=0, no-cache, no-store, must-revalidate"
       Header set Pragma "no-cache"
       Header set Expires "Wed, 11 Jan 1984 05:00:00 GMT"
    </ifModule>
  </filesMatch>

Thank you

Best regards,

Albert

alexeykomov pushed a commit to alexeykomov/closure-compiler that referenced this issue Feb 8, 2017
@jakearchibald
Copy link
Contributor

@albertsylvester if this is still a problem, can you post it to stackoverflow? I (or someone else) will answer it there.

@mfalken
Copy link
Member

mfalken commented Apr 4, 2017

The F2F resolution was useCache would be an enum with the default to cache import scripts but not cache the top-level script. See issue #1104

@jungkees
Copy link
Collaborator

pieh pushed a commit to gatsbyjs/gatsby that referenced this issue Nov 15, 2018
…fy (#9680)

Related to https://www.netlify.com/blog/2018/06/28/5-pro-tips-and-plugins-for-optimizing-your-gatsby---netlify-site/#4-get-your-service-workers-um-working

Recommandation is not to cache service worker file : w3c/ServiceWorker#893

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
@kgrosvenor
Copy link

What does useCache mean exactly, to cache the service worker file or not? please explain

gpetrioli pushed a commit to gpetrioli/gatsby that referenced this issue Jan 22, 2019
…fy (gatsbyjs#9680)

Related to https://www.netlify.com/blog/2018/06/28/5-pro-tips-and-plugins-for-optimizing-your-gatsby---netlify-site/#4-get-your-service-workers-um-working

Recommandation is not to cache service worker file : w3c/ServiceWorker#893

<!--
  Q. Which branch should I use for my pull request?
  A. Use `master` branch (probably).

  Q. Which branch if my change is a bug fix for Gatsby v1?
  A. In this case, you should use the `v1` branch

  Q. Which branch if I'm still not sure?
  A. Use `master` branch. Ask in the PR if you're not sure and a Gatsby maintainer will be happy to help :)

  Note: We will only accept bug fixes for Gatsby v1. New features should be added to Gatsby v2.

  Learn more about contributing: https://www.gatsbyjs.org/docs/how-to-contribute/
-->
@f0rmat1k
Copy link

What does useCache mean exactly, to cache the service worker file or not? please explain

@kgrosvenor I suppose by default browsers now use Cache-control: no-cache. That means, browsers will try to update sw.js by asking server (etag or max-age). If you want to get old behavior, you can set { useCache: true }. With it, browser will respect your Cache-control headers. But still forcibly update sw.js if more than 24 hours have passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests