workbox icon indicating copy to clipboard operation
workbox copied to clipboard

Strategy unhandled rejection

Open joshkel opened this issue 1 year ago • 1 comments

Fixes https://github.com/GoogleChrome/workbox/issues/3171

StrategyHandler.doneWaiting throws on the first rejected promise that it encounters. This means that subsequent rejected promises may result in unhandled rejection errors.

Promise.allSettled is a natural solution. It does not appear to be supported on all browsers that workbox uses. (However, I'm having trouble finding a clear statement of what browsers workbox supports, so I could be wrong.) I tried installing promise.allsettled from NPM but had trouble getting the default import from that module to work correctly with Rollup, and I noticed that the workbox service worker libraries tend to avoid external dependencies in general, so I created a local version instead. I put it under workbox-core's _private directory, following the example of other utility code such as Deferred. If I should handle this differently, please let me know.

This is a rebase of #3172 against the v7 branch.

joshkel avatar Apr 27 '24 18:04 joshkel

Fully approve of the intent of the PR. However, I don't think Promise.allSettled() needs polyfilling looking at browser support. Looking at devices that didn't make the Safari 13 cut, this are iPhone 5S, iPhone 6 and 6 Plus, the original iPad Air, the iPad mini 2 and iPad mini 3, and the 6th-generation iPod Touch. For Chromebooks, we'd be locking out Chromebooks stuck on Chrome <76. @tropicadri, what do you think?

tomayac avatar Apr 30 '24 08:04 tomayac