feat(service-worker): fixes/improvements related to entering/recovering from EXISTING_CLIENTS_ONLY mode
#31865
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `latest` argument was only ever set to the value of comparing `this.latestHash` with the `appVersion` hash, which is already computed inside `versionFailed()`, so there is no reason to pass it as an argument as well. This doesn't have any impact on the current behavior of the SW.
Helper functions for making navigation requests were created in several places inside the test suite, so this commit creates a top-level such helper and uses that in all tests that need it.
… is invalidated Previously, when the latest version was invalidated (e.g. due to a hash mismatch), the SW entered a degraded `EXISTING_CLIENTS_ONLY` mode and removed _all_ clients from its client-version map (essentially stopping to serve any clients). Based on the code and surrounding comments, the intention seems to have been to only remove clients that were on the invalidated version, but keep other clients on older versions. This commit fixes it by only unassigning clients what were on the latest version and keep clients assigned to older versions.
gkalpak
changed the title
EXISTING_CLIENTS_ONLY modeEXISTING_CLIENTS_ONLY mode
gkalpak
added
area: service-worker
|
You can preview 0fbd651 at https://pr31865-0fbd651.ngbuilds.io/. |
gkalpak
added
action: review
…there is a valid update Previously, when the ServiceWorker entered a degraded mode (`EXISTING_CLIENTS_ONLY` or `SAFE_MODE`) it would remain in that mode for the rest of the lifetime of ServiceWorker instance. Note that ServiceWorkers are stopped by the browser after a certain period of inactivity and a new instance is created as soon as the ServiceWorker needs to handle an event (such as a request from the page). Those new instances would start from the `NORMAL` mode. The reason for this behavior is to err on the side of caution: If we can't be sure why the ServiceWorker entered the degraded mode, it is risky to try recovering on the same instance and might lead to unexpected behavior. However, it turns out that the `EXISTING_CLIENTS_ONLY` mode can only be a result of some error happening with the latest version (e.g. a hash mismatch in the manifest). Therefore, it is safe to recover from that mode once a new, valid update is successfully installed and to start accepting new clients. This commit ensures that the mode is set back to `NORMAL`, when (a) an update is successfully installed and (b) the current mode is `EXISTING_CLIENTS_ONLY`. Besides making the behavior more predictable (instead of relying on the browser to decide when to terminate the current ServiceWorker instance and create a new one), this change can also improve the developer experience: When people notice the error during debugging and fix it by deploying a new version (either to production or locally), it is confusing that the ServiceWorker will fetch and install the update (as seen by the requests in the Network panel in DevTools) but not serve it to clients. With this change, the update will be served to new clients as soon as it is installed. Fixes angular#31109
gkalpak
force-pushed
the
fix-sw-recover-from-degraded
branch
from
0fbd651
to
ea861b9
Compare
This was referenced Jul 26, 2019
|
You can preview ea861b9 at https://pr31865-ea861b9.ngbuilds.io/. |
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Jul 29, 2019
mhevery
approved these changes
Sep 4, 2019
gkalpak
added
action: merge
|
merge-assistance: I am a code-owner for |
mhevery
pushed a commit
that referenced
this pull request
Sep 4, 2019
… is invalidated (#31865) Previously, when the latest version was invalidated (e.g. due to a hash mismatch), the SW entered a degraded `EXISTING_CLIENTS_ONLY` mode and removed _all_ clients from its client-version map (essentially stopping to serve any clients). Based on the code and surrounding comments, the intention seems to have been to only remove clients that were on the invalidated version, but keep other clients on older versions. This commit fixes it by only unassigning clients what were on the latest version and keep clients assigned to older versions. PR Close #31865
mhevery
pushed a commit
that referenced
this pull request
Sep 4, 2019
…there is a valid update (#31865) Previously, when the ServiceWorker entered a degraded mode (`EXISTING_CLIENTS_ONLY` or `SAFE_MODE`) it would remain in that mode for the rest of the lifetime of ServiceWorker instance. Note that ServiceWorkers are stopped by the browser after a certain period of inactivity and a new instance is created as soon as the ServiceWorker needs to handle an event (such as a request from the page). Those new instances would start from the `NORMAL` mode. The reason for this behavior is to err on the side of caution: If we can't be sure why the ServiceWorker entered the degraded mode, it is risky to try recovering on the same instance and might lead to unexpected behavior. However, it turns out that the `EXISTING_CLIENTS_ONLY` mode can only be a result of some error happening with the latest version (e.g. a hash mismatch in the manifest). Therefore, it is safe to recover from that mode once a new, valid update is successfully installed and to start accepting new clients. This commit ensures that the mode is set back to `NORMAL`, when (a) an update is successfully installed and (b) the current mode is `EXISTING_CLIENTS_ONLY`. Besides making the behavior more predictable (instead of relying on the browser to decide when to terminate the current ServiceWorker instance and create a new one), this change can also improve the developer experience: When people notice the error during debugging and fix it by deploying a new version (either to production or locally), it is confusing that the ServiceWorker will fetch and install the update (as seen by the requests in the Network panel in DevTools) but not serve it to clients. With this change, the update will be served to new clients as soon as it is installed. Fixes #31109 PR Close #31865
sabeersulaiman
pushed a commit
to sabeersulaiman/angular
that referenced
this pull request
Sep 6, 2019
…()` (angular#31865) The `latest` argument was only ever set to the value of comparing `this.latestHash` with the `appVersion` hash, which is already computed inside `versionFailed()`, so there is no reason to pass it as an argument as well. This doesn't have any impact on the current behavior of the SW. PR Close angular#31865
sabeersulaiman
pushed a commit
to sabeersulaiman/angular
that referenced
this pull request
Sep 6, 2019
…sts (angular#31865) Helper functions for making navigation requests were created in several places inside the test suite, so this commit creates a top-level such helper and uses that in all tests that need it. PR Close angular#31865
sabeersulaiman
pushed a commit
to sabeersulaiman/angular
that referenced
this pull request
Sep 6, 2019
… is invalidated (angular#31865) Previously, when the latest version was invalidated (e.g. due to a hash mismatch), the SW entered a degraded `EXISTING_CLIENTS_ONLY` mode and removed _all_ clients from its client-version map (essentially stopping to serve any clients). Based on the code and surrounding comments, the intention seems to have been to only remove clients that were on the invalidated version, but keep other clients on older versions. This commit fixes it by only unassigning clients what were on the latest version and keep clients assigned to older versions. PR Close angular#31865
sabeersulaiman
pushed a commit
to sabeersulaiman/angular
that referenced
this pull request
Sep 6, 2019
…there is a valid update (angular#31865) Previously, when the ServiceWorker entered a degraded mode (`EXISTING_CLIENTS_ONLY` or `SAFE_MODE`) it would remain in that mode for the rest of the lifetime of ServiceWorker instance. Note that ServiceWorkers are stopped by the browser after a certain period of inactivity and a new instance is created as soon as the ServiceWorker needs to handle an event (such as a request from the page). Those new instances would start from the `NORMAL` mode. The reason for this behavior is to err on the side of caution: If we can't be sure why the ServiceWorker entered the degraded mode, it is risky to try recovering on the same instance and might lead to unexpected behavior. However, it turns out that the `EXISTING_CLIENTS_ONLY` mode can only be a result of some error happening with the latest version (e.g. a hash mismatch in the manifest). Therefore, it is safe to recover from that mode once a new, valid update is successfully installed and to start accepting new clients. This commit ensures that the mode is set back to `NORMAL`, when (a) an update is successfully installed and (b) the current mode is `EXISTING_CLIENTS_ONLY`. Besides making the behavior more predictable (instead of relying on the browser to decide when to terminate the current ServiceWorker instance and create a new one), this change can also improve the developer experience: When people notice the error during debugging and fix it by deploying a new version (either to production or locally), it is confusing that the ServiceWorker will fetch and install the update (as seen by the requests in the Network panel in DevTools) but not serve it to clients. With this change, the update will be served to new clients as soon as it is installed. Fixes angular#31109 PR Close angular#31865
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Sep 14, 2019
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Sep 14, 2019
14 tasks
arnehoek
pushed a commit
to arnehoek/angular
that referenced
this pull request
Sep 26, 2019
…()` (angular#31865) The `latest` argument was only ever set to the value of comparing `this.latestHash` with the `appVersion` hash, which is already computed inside `versionFailed()`, so there is no reason to pass it as an argument as well. This doesn't have any impact on the current behavior of the SW. PR Close angular#31865
arnehoek
pushed a commit
to arnehoek/angular
that referenced
this pull request
Sep 26, 2019
…sts (angular#31865) Helper functions for making navigation requests were created in several places inside the test suite, so this commit creates a top-level such helper and uses that in all tests that need it. PR Close angular#31865
arnehoek
pushed a commit
to arnehoek/angular
that referenced
this pull request
Sep 26, 2019
… is invalidated (angular#31865) Previously, when the latest version was invalidated (e.g. due to a hash mismatch), the SW entered a degraded `EXISTING_CLIENTS_ONLY` mode and removed _all_ clients from its client-version map (essentially stopping to serve any clients). Based on the code and surrounding comments, the intention seems to have been to only remove clients that were on the invalidated version, but keep other clients on older versions. This commit fixes it by only unassigning clients what were on the latest version and keep clients assigned to older versions. PR Close angular#31865
arnehoek
pushed a commit
to arnehoek/angular
that referenced
this pull request
Sep 26, 2019
…there is a valid update (angular#31865) Previously, when the ServiceWorker entered a degraded mode (`EXISTING_CLIENTS_ONLY` or `SAFE_MODE`) it would remain in that mode for the rest of the lifetime of ServiceWorker instance. Note that ServiceWorkers are stopped by the browser after a certain period of inactivity and a new instance is created as soon as the ServiceWorker needs to handle an event (such as a request from the page). Those new instances would start from the `NORMAL` mode. The reason for this behavior is to err on the side of caution: If we can't be sure why the ServiceWorker entered the degraded mode, it is risky to try recovering on the same instance and might lead to unexpected behavior. However, it turns out that the `EXISTING_CLIENTS_ONLY` mode can only be a result of some error happening with the latest version (e.g. a hash mismatch in the manifest). Therefore, it is safe to recover from that mode once a new, valid update is successfully installed and to start accepting new clients. This commit ensures that the mode is set back to `NORMAL`, when (a) an update is successfully installed and (b) the current mode is `EXISTING_CLIENTS_ONLY`. Besides making the behavior more predictable (instead of relying on the browser to decide when to terminate the current ServiceWorker instance and create a new one), this change can also improve the developer experience: When people notice the error during debugging and fix it by deploying a new version (either to production or locally), it is confusing that the ServiceWorker will fetch and install the update (as seen by the requests in the Network panel in DevTools) but not serve it to clients. With this change, the update will be served to new clients as soon as it is installed. Fixes angular#31109 PR Close angular#31865
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Sep 30, 2019
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Sep 30, 2019
H--o-l
added a commit
to H--o-l/angular
that referenced
this pull request
Sep 30, 2019
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some fixes/improvements related to entering and recovering from the
EXISTING_CLIENTS_ONLYmode. See individual commints for more details:versionFailed()EXISTING_CLIENTS_ONLYmode when there is a valid updateFixes #31109.