fix(service-worker): handle offline mode and ignore invalid `only-if-cached` requests #22883

gkalpak · 2018-03-20T13:25:15Z

PR Checklist

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
~~Docs have been added / updated (for bug fixes / features)~~

PR Type

[x] Bugfix

What is the current behavior?

This PR fixes two issues:

When trying to fetch ngsw.json (e.g. during checkForUpdate()) while either the client or the server are offline, the ServiceWorker would enter a degrade mode, where only existing clients would be served. This essentially means that the ServiceWorker doesn't work offline.
Under some circumstances (possibly related to opening Chrome DevTools), requests are made with cache: 'only-if-cached' and mode: 'no-cors'. These request will eventually fail, because only-if-cached is only allowed to be used with mode: 'same-origin'.
This is likely a bug in Chrome DevTools. See also Failed to load resource: the server responded with a status of 504 (Gateway Timeout) #22362 (comment).

Issue Number: #21636, #22362

What is the new behavior?

Offline errors (status: 504) are treated differently than other fetch errors and the SW doesn't enter degraded mode. The ServiceWorker will remain in the current mode until connectivity to the server is restored.
Invalid only-if-cached requests (i.e. with mode !== 'same-origin') are not handled by the SW.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Fixes #21636 and #22362.

mary-poppins · 2018-03-20T13:35:10Z

You can preview aa7b71c at https://pr22883-aa7b71c.ngbuilds.io/.

alxhub · 2018-03-21T19:24:32Z

packages/service-worker/worker/src/driver.ts

+    // (See also https://github.com/angular/angular/issues/22362.)
+    // TODO(gkalpak): Remove once no longer necessary (i.e. fixed in Chrome DevTools).
+    if ((req.cache as string) === 'only-if-cached' && req.mode !== 'same-origin') {
+      return;


Let's console.error the first time this happens so it's not a silent failure.

Why a failure? It is just that the SW won't handle this request, so it will pass through (and I assume the browser will do the right thing).
(Interestingly, no error is logged when the SW doesn't handle such requests.)

I think if someone is looking in DevTools, expecting to see their request answered (from ServiceWorker), that having a message locally there saying "hey, the SW didn't handle this request because DevTools sent it in an invalid way" is beneficial.

I'm okay if we only log the first time it happens.

As discussed "offline", this is not a request made by the app/user, but a background request made by DevTools (apparently). So, we'll debugger.log() the first time we see it.

I have pushed another commit. PTAL

mary-poppins · 2018-03-23T16:32:18Z

You can preview 0535086 at https://pr22883-0535086.ngbuilds.io/.

Previously, when trying to fetch `ngsw.json` (e.g. during `checkForUpdate()`) while either the client or the server were offline, the ServiceWorker would enter a degrade mode, where only existing clients would be served. This essentially meant that the ServiceWorker didn't work offline. This commit fixes it by differentiating offline errors and not entering degraded mode. The ServiceWorker will remain in the current mode until connectivity to the server is restored. Fixes angular#21636

Under some circumstances (possibly related to opening Chrome DevTools), requests are made with `cache: 'only-if-cached'` and `mode: 'no-cors'`. These request will eventually fail, because `only-if-cached` is only allowed to be used with `mode: 'same-origin'`. This is likely a bug in Chrome DevTools. This commit avoids errors related to such requests by not handling them. Fixes angular#22362

mary-poppins · 2018-03-23T21:24:31Z

You can preview 5fae6e7 at https://pr22883-5fae6e7.ngbuilds.io/.

mary-poppins · 2018-03-26T11:06:27Z

You can preview 0235d68 at https://pr22883-0235d68.ngbuilds.io/.

mary-poppins · 2018-03-26T11:12:49Z

You can preview 76109cb at https://pr22883-76109cb.ngbuilds.io/.

Aarchana · 2018-03-27T18:00:58Z

Hi, when can we expect this to be merged so that we can start playing with the service worker offline mode capability:)

gkalpak · 2018-03-27T19:04:09Z

It is marked for merge, so I guess soon 😃

Previously, when trying to fetch `ngsw.json` (e.g. during `checkForUpdate()`) while either the client or the server were offline, the ServiceWorker would enter a degrade mode, where only existing clients would be served. This essentially meant that the ServiceWorker didn't work offline. This commit fixes it by differentiating offline errors and not entering degraded mode. The ServiceWorker will remain in the current mode until connectivity to the server is restored. Fixes #21636 PR Close #22883

Under some circumstances (possibly related to opening Chrome DevTools), requests are made with `cache: 'only-if-cached'` and `mode: 'no-cors'`. These request will eventually fail, because `only-if-cached` is only allowed to be used with `mode: 'same-origin'`. This is likely a bug in Chrome DevTools. This commit avoids errors related to such requests by not handling them. Fixes #22362 PR Close #22883

PR Close #22883

Previously, when trying to fetch `ngsw.json` (e.g. during `checkForUpdate()`) while either the client or the server were offline, the ServiceWorker would enter a degrade mode, where only existing clients would be served. This essentially meant that the ServiceWorker didn't work offline. This commit fixes it by differentiating offline errors and not entering degraded mode. The ServiceWorker will remain in the current mode until connectivity to the server is restored. Fixes #21636 PR Close #22883

Under some circumstances (possibly related to opening Chrome DevTools), requests are made with `cache: 'only-if-cached'` and `mode: 'no-cors'`. These request will eventually fail, because `only-if-cached` is only allowed to be used with `mode: 'same-origin'`. This is likely a bug in Chrome DevTools. This commit avoids errors related to such requests by not handling them. Fixes #22362 PR Close #22883

Aarchana · 2018-03-30T21:36:10Z

Hi i created a new angular cli project with the service worker enabled , still i see when app is made offline the 504 error appears. Can anyone suggest on this please:)

gkalpak · 2018-04-02T11:04:42Z

The fix is included in @angular/service-worker@6.0.0-rc.1. Make sure you are using that version for all packages.

_{If you don't want to use a version that is not yet marked as stable, you need to wait a bit for the final 6.0 release (currently scheduled for this week).}

Aarchana · 2018-04-05T15:40:29Z

@gkalpak ,
Thanks a lot.

tobyStaff · 2018-05-04T11:24:12Z

can anyone tell me when #22883 is likely to go into a release?
A quick search of the 5.2.10 -> 6 tells me that it hasn't been included yet.

probert94 · 2018-05-04T12:18:34Z

As much as I remember, it is already in version 5.2.10.
But you should take a look at the changelog

gkalpak · 2018-05-05T11:14:31Z

@Springrbua is right. It is in 5.2.10 and 6.x.

ALGDB · 2018-05-08T13:55:54Z

I have the same problem. I've updated to v6.x

"dependencies": {
    "@angular/animations": "6.0.0",
    "@angular/cdk": "^6.0.0",
    "@angular/common": "6.0.0",
    "@angular/compiler": "6.0.0",
    "@angular/core": "6.0.0",
    "@angular/flex-layout": "^6.0.0-beta.15",
    "@angular/forms": "6.0.0",
    "@angular/http": "6.0.0",
    "@angular/material": "6.0.1",
    "@angular/platform-browser": "6.0.0",
    "@angular/platform-browser-dynamic": "6.0.0",
    "@angular/router": "6.0.0",
    "@angular/service-worker": "6.0.0",

that's the error on Chrome 66.0.3359.139

e {headers: t, status: 504, statusText: "Gateway Timeout", url: "http://localhost:3000/api/v1/externalHumPre", ok: false, …}
error:null
headers:t {normalizedNames: Map(0), lazyUpdate: null, headers: Map(0)}
message:"Http failure response for http://localhost:3000/api/v1/externalHumPre: 504 Gateway Timeout"
name:"HttpErrorResponse"
ok:false
status:504
statusText:"Gateway Timeout"
url:"http://localhost:3000/api/v1/externalHumPre"

gkalpak · 2018-05-09T09:23:57Z

Could be a different problem. Please open a new issue and make sure you provide a minimal reproduction.

angular-automatic-lock-bot · 2019-09-13T14:52:35Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Mar 20, 2018

gkalpak requested a review from alxhub March 20, 2018 13:25

gkalpak added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release area: service-worker Issues related to the @angular/service-worker package labels Mar 20, 2018

This was referenced Mar 20, 2018

[ServiceWorker] localhost took too long to respond. HTTP ERROR 504 #21636

Closed

[Service Worker] Can't update sw if in SAFE_MODE #22087

Closed

feat(aio): Convert AIO to use the new Service Worker 5.0.0. #19795

Closed

alxhub approved these changes Mar 21, 2018

View reviewed changes

gkalpak force-pushed the fix-sw-offline branch from aa7b71c to 0535086 Compare March 23, 2018 16:19

gkalpak added 3 commits March 23, 2018 23:14

test(service-worker): minor test fixes and refactorings

68d78a9

gkalpak force-pushed the fix-sw-offline branch from 0535086 to 5fae6e7 Compare March 23, 2018 21:15

gkalpak mentioned this pull request Mar 25, 2018

Service Worker does not work for offline applications #22976

Closed

gkalpak force-pushed the fix-sw-offline branch from 5fae6e7 to 0235d68 Compare March 26, 2018 10:54

fixup! fix(service-worker): ignore invalid only-if-cached requests

76109cb

gkalpak force-pushed the fix-sw-offline branch from 0235d68 to 76109cb Compare March 26, 2018 11:03

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 27, 2018

alxhub closed this in 12665a7 Mar 28, 2018

alxhub pushed a commit that referenced this pull request Mar 28, 2018

test(service-worker): minor test fixes and refactorings (#22883)

d0f575b

PR Close #22883

gkalpak deleted the fix-sw-offline branch March 28, 2018 18:34

jnrndn mentioned this pull request Mar 29, 2018

PWA is not working in offline mode ngcolombia/ngcolombia-2018#24

Closed

gkalpak mentioned this pull request Apr 3, 2018

Angular service worker stop working after going offline #23135

Closed

gkalpak mentioned this pull request Apr 24, 2018

Service worker updates do not work on latest chrome (Chrome 66.0.3359.117 ) #23526

Closed

1 task

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(service-worker): handle offline mode and ignore invalid `only-if-cached` requests #22883

fix(service-worker): handle offline mode and ignore invalid `only-if-cached` requests #22883

gkalpak commented Mar 20, 2018

mary-poppins commented Mar 20, 2018

alxhub Mar 21, 2018

gkalpak Mar 21, 2018

alxhub Mar 23, 2018 •

edited

Loading

gkalpak Mar 23, 2018

mary-poppins commented Mar 23, 2018

mary-poppins commented Mar 23, 2018

mary-poppins commented Mar 26, 2018

mary-poppins commented Mar 26, 2018

Aarchana commented Mar 27, 2018 •

edited

Loading

gkalpak commented Mar 27, 2018

Aarchana commented Mar 30, 2018

gkalpak commented Apr 2, 2018

Aarchana commented Apr 5, 2018

tobyStaff commented May 4, 2018

probert94 commented May 4, 2018

gkalpak commented May 5, 2018 •

edited

Loading

ALGDB commented May 8, 2018

gkalpak commented May 9, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

fix(service-worker): handle offline mode and ignore invalid only-if-cached requests #22883

fix(service-worker): handle offline mode and ignore invalid only-if-cached requests #22883

Conversation

gkalpak commented Mar 20, 2018

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

mary-poppins commented Mar 20, 2018

alxhub Mar 21, 2018

Choose a reason for hiding this comment

gkalpak Mar 21, 2018

Choose a reason for hiding this comment

alxhub Mar 23, 2018 • edited Loading

Choose a reason for hiding this comment

gkalpak Mar 23, 2018

Choose a reason for hiding this comment

mary-poppins commented Mar 23, 2018

mary-poppins commented Mar 23, 2018

mary-poppins commented Mar 26, 2018

mary-poppins commented Mar 26, 2018

Aarchana commented Mar 27, 2018 • edited Loading

gkalpak commented Mar 27, 2018

Aarchana commented Mar 30, 2018

gkalpak commented Apr 2, 2018

Aarchana commented Apr 5, 2018

tobyStaff commented May 4, 2018

probert94 commented May 4, 2018

gkalpak commented May 5, 2018 • edited Loading

ALGDB commented May 8, 2018

gkalpak commented May 9, 2018

angular-automatic-lock-bot bot commented Sep 13, 2019

fix(service-worker): handle offline mode and ignore invalid `only-if-cached` requests #22883

fix(service-worker): handle offline mode and ignore invalid `only-if-cached` requests #22883

alxhub Mar 23, 2018 •

edited

Loading

Aarchana commented Mar 27, 2018 •

edited

Loading

gkalpak commented May 5, 2018 •

edited

Loading