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

Prevent onSync infinite loops #1943

Merged
merged 1 commit into from
Mar 7, 2019
Merged

Prevent onSync infinite loops #1943

merged 1 commit into from
Mar 7, 2019

Conversation

philipwalton
Copy link
Member

R: @jeffposnick

Fixes #1937

This PR updates the background sync replay logic in the following ways to help prevent infinite loops when adding new requests to the queue during a sync event replay:

  • Adding a request to the queue only registers a sync tag if a sync event is not currently in progress.
  • At the end of a successful sync event replay (i.e. the waitUntil promise did not reject) a sync tag is registered if new requests have been added to the queue.
  • At the end of a failed sync event replay (i.e. the waitUntil promise rejected) no sync event is registered because the browser will automatically retry later (unless the event.lastChance property is true, in which case a new sync tag is registered

The PR also updates some of the workbox-google-analytics behavior to handle these new changes.

@philipwalton philipwalton requested a review from jeffposnick March 7, 2019 07:08
@workbox-pr-bot
Copy link
Collaborator

PR-Bot Size Plugin

Changed File Sizes

File Before After Change GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js 3.21 KB 3.38 KB +5% 1.46 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js 1.89 KB 1.89 KB -0% 897 B

New Files

No new files have been added.

All File Sizes

View Table
File Before After Change GZipped
packages/workbox-background-sync/build/workbox-background-sync.prod.js 3.21 KB 3.38 KB +5% 1.46 KB
packages/workbox-broadcast-update/build/workbox-broadcast-update.prod.js 1.86 KB 1.86 KB 0% 933 B
packages/workbox-build/build/_types.js 41 B 41 B 0% 61 B
packages/workbox-build/build/index.js 3.64 KB 3.64 KB 0% 1.36 KB
packages/workbox-cacheable-response/build/workbox-cacheable-response.prod.js 579 B 579 B 0% 344 B
packages/workbox-cli/build/app.js 5.58 KB 5.58 KB 0% 1.98 KB
packages/workbox-cli/build/bin.js 1.16 KB 1.16 KB 0% 580 B
packages/workbox-core/build/workbox-core.prod.js 5.34 KB 5.34 KB 0% 2.30 KB
packages/workbox-expiration/build/workbox-expiration.prod.js 2.83 KB 2.83 KB 0% 1.24 KB
packages/workbox-google-analytics/build/workbox-offline-ga.prod.js 1.89 KB 1.89 KB -0% 897 B
packages/workbox-navigation-preload/build/workbox-navigation-preload.prod.js 652 B 652 B 0% 317 B
packages/workbox-precaching/build/workbox-precaching.prod.js 4.25 KB 4.25 KB 0% 1.71 KB
packages/workbox-range-requests/build/workbox-range-requests.prod.js 1.51 KB 1.51 KB 0% 758 B
packages/workbox-routing/build/workbox-routing.prod.js 3.38 KB 3.38 KB 0% 1.47 KB
packages/workbox-strategies/build/workbox-strategies.prod.js 4.86 KB 4.86 KB 0% 1.19 KB
packages/workbox-streams/build/workbox-streams.prod.js 1.38 KB 1.38 KB 0% 677 B
packages/workbox-sw/build/workbox-sw.js 1.33 KB 1.33 KB 0% 741 B
packages/workbox-webpack-plugin/build/generate-sw.js 5.29 KB 5.29 KB 0% 1.84 KB
packages/workbox-webpack-plugin/build/index.js 349 B 349 B 0% 255 B
packages/workbox-webpack-plugin/build/inject-manifest.js 7.22 KB 7.22 KB 0% 2.48 KB
packages/workbox-window/build/workbox-window.dev.umd.js 30.52 KB 30.52 KB 0% 8.07 KB
packages/workbox-window/build/workbox-window.prod.umd.js 4.66 KB 4.66 KB 0% 1.81 KB

Workbox Aggregate Size Plugin

8.77KB gzip'ed (58% of limit)
22.04KB uncompressed

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 82.736% when pulling 73f8006 on bg-sync-loop into 5e2c4a1 on master.

Copy link
Contributor

@jeffposnick jeffposnick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for wading through the edge cases!

@jeffposnick jeffposnick merged commit 5e04a62 into master Mar 7, 2019
@jeffposnick jeffposnick deleted the bg-sync-loop branch March 7, 2019 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants