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

fix: catch sw precaching errors [DHIS2-15625] #812

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cli/src/lib/pwa/compileServiceWorker.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ function compileServiceWorker({ config, paths, mode }) {
// TODO: This could be cleaner if the production SW is built in the same
// way instead of using the CRA webpack config, so both can more easily
// share environment variables.
const env = getEnv({ name: config.title, ...getPWAEnvVars(config) })
const env = getEnv({
name: config.title,
version: config.version,
...getPWAEnvVars(config),
})

const webpackConfig = {
mode, // "production" or "development"
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
"@dhis2/cli-style": "^10.4.3",
"@dhis2/cli-utils-docsite": "^3.0.0",
"concurrently": "^6.0.0",
"patch-package": "^8.0.0",
"postinstall-postinstall": "^2.1.0",
"serve": "^12.0.0"
},
"scripts": {
Expand All @@ -37,7 +39,8 @@
"docs:build": "d2-utils-docsite build ./docs -o ./dist",
"test:adapter": "yarn workspace @dhis2/app-adapter test",
"test:cli": "yarn workspace @dhis2/cli-app-scripts test",
"test": "yarn test:adapter && yarn test:cli"
"test": "yarn test:adapter && yarn test:cli",
"postinstall": "patch-package"
},
"d2": {
"docsite": {
Expand Down
57 changes: 57 additions & 0 deletions patches/workbox-precaching+6.5.3.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
diff --git a/node_modules/workbox-precaching/PrecacheController.js b/node_modules/workbox-precaching/PrecacheController.js
index e00975e..b728b6a 100644
--- a/node_modules/workbox-precaching/PrecacheController.js
+++ b/node_modules/workbox-precaching/PrecacheController.js
@@ -150,6 +150,7 @@ class PrecacheController {
return waitUntil(event, async () => {
const installReportPlugin = new PrecacheInstallReportPlugin();
this.strategy.plugins.push(installReportPlugin);
+ const failedUrls = []
// Cache entries one at a time.
// See https://github.com/GoogleChrome/workbox/issues/2528
for (const [url, cacheKey] of this._urlsToCacheKeys) {
@@ -164,12 +165,43 @@ class PrecacheController {
params: { cacheKey },
request,
event,
- }));
+ }))
+ // DHIS2: Catch precaching errors to provide a more thorough
+ // error message
+ .catch(err => {
+ failedUrls.push(url)
+ console.error(`Error when attempting to fetch and cache URL ${url}:\n`, err)
+ });
}
const { updatedURLs, notUpdatedURLs } = installReportPlugin;
if (process.env.NODE_ENV !== 'production') {
printInstallDetails(updatedURLs, notUpdatedURLs);
}
+ // DHIS2: Log failed requests to give clear feedback, and throw
+ // error to abort installation
+ if (failedUrls.length > 0) {
+ const appVersion = process.env.REACT_APP_DHIS2_APP_VERSION
+ const appName = process.env.REACT_APP_DHIS2_APP_NAME
+ const errorMessage =
+ `The following assets were unable to be precached when ` +
+ `attempting to install ${appName} version ${appVersion}, ` +
+ `so the installation has been aborted:\n` +
+ `${failedUrls.reduce((acc, curr) => acc + ` * ${curr}\n`, '')}\n` +
+ `If another version of the app is installed in the browser, ` +
+ `you will continue to see that version. If not, you will ` +
+ `see version ${appVersion}, but caching and offline ` +
+ `features will be unavailable, and the app may not be ` +
+ `fully functional due to the missing files. You can find ` +
+ `the app version that's currently active at the bottom ` +
+ `of the user profile menu.\n\n` +
+ `[FOR TECHNICAL USERS]: If another version is installed ` +
+ `and you would like to attempt to view version ` +
+ `${appVersion} (knowing the above caveats), you may use ` +
+ `your browser's developer tools to find the service worker ` +
+ `that's currently active for this app and unregister it.`
+ throw new Error(errorMessage)
+ }
+
return { updatedURLs, notUpdatedURLs };
});
}
125 changes: 98 additions & 27 deletions pwa/src/service-worker/set-up-service-worker.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { precacheAndRoute, matchPrecache, precache } from 'workbox-precaching'
import {
precacheAndRoute,
matchPrecache,
// PrecacheController,
// PrecacheRoute,
} from 'workbox-precaching'
import { registerRoute, setDefaultHandler } from 'workbox-routing'
import {
NetworkFirst,
Expand Down Expand Up @@ -42,7 +47,7 @@ export function setUpServiceWorker() {

// Disable verbose logs
// TODO: control with env var
self.__WB_DISABLE_DEV_LOGS = true
// self.__WB_DISABLE_DEV_LOGS = true

// Globals (Note: global state resets each time SW goes idle)

Expand All @@ -60,21 +65,6 @@ export function setUpServiceWorker() {
// In development, static assets are handled by 'network first' strategy
// and will be kept up-to-date.
if (PRODUCTION_ENV) {
// Precache all of the assets generated by your build process.
// Their URLs are injected into the manifest variable below.
// This variable must be present somewhere in your service worker file,
// even if you decide not to use precaching. See https://cra.link/PWA.
// Includes all built assets and index.html
const precacheManifest = self.__WB_MANIFEST || []

// todo: also do this routing for plugin.html
// Extract index.html from the manifest to precache, then route
// in a custom way
const indexHtmlManifestEntry = precacheManifest.find(({ url }) =>
url.endsWith('index.html')
)
precache([indexHtmlManifestEntry])

// Custom strategy for handling app navigation, specifically to allow
// navigations to redirect to the login page while online if the
// user is unauthenticated. Fixes showing the app shell login dialog
Expand Down Expand Up @@ -125,13 +115,17 @@ export function setUpServiceWorker() {
// NOTE: This route must come before any precacheAndRoute calls
registerRoute(navigationRouteMatcher, navigationRouteHandler)

// Handle the rest of files in the manifest - filter out index.html,
// and all moment-locales, which bulk up the precache and slow down
// installation significantly. Handle them network-first in app shell
const restOfManifest = precacheManifest.filter((e) => {
if (e === indexHtmlManifestEntry) {
return false
}
// Precache all of the assets generated by your build process.
// Their URLs are injected into the manifest variable below.
// This variable must be present somewhere in your service worker file,
// even if you decide not to use precaching. See https://cra.link/PWA.
// Includes all built assets and index.html
const precacheManifest = self.__WB_MANIFEST || []

// This precache manifest needs to be filtered here instead of at build
// time because CRA manages the build process. The manifests below
// are managed by our own build tools, so we can filter at build time
const filteredPrecacheManifest = precacheManifest.filter((e) => {
// Files from the precache manifest generated by CRA need to be
// managed here, because we don't have access to their webpack
// config
Expand All @@ -140,11 +134,9 @@ export function setUpServiceWorker() {
)
return !entryShouldBeExcluded
})
precacheAndRoute(restOfManifest)

// Same thing for built plugin assets
const pluginPrecacheManifest = self.__WB_PLUGIN_MANIFEST || []
precacheAndRoute(pluginPrecacheManifest)

// Similar to above; manifest injection from `workbox-build`
// Precaches all assets in the shell's build folder except in `static`
Expand All @@ -154,7 +146,86 @@ export function setUpServiceWorker() {
// 'injectPrecacheManifest.js' in the CLI package.
// '[]' fallback prevents an error when switching pwa enabled to disabled
const sharedBuildManifest = self.__WB_BUILD_MANIFEST || []
precacheAndRoute(sharedBuildManifest)

// Add all these URLs to the precache list and instruct workbox to make
// a route for them
// NOTE: this includes index.html since it's on the precache list; it's
// important that the navigation route above gets registered first
precacheAndRoute([
...filteredPrecacheManifest,
...pluginPrecacheManifest,
...sharedBuildManifest,
])
} else {
// This will execute in dev environments; just included for testing purposes.
// Remove before merging
precacheAndRoute([
// Let this one work
{ url: './index.html', revision: '1' },
// The following are expected to fail
{ url: 'https://not-a-site.com/hey.jpg', revision: '1' },
{ url: 'https://not-a-site.com/bogus1.jpg', revision: '1' },
{ url: 'https://not-a-site.com/derp2.jpg', revision: '1' },
{ url: 'https://not-a-site.com/diddlibap3.jpg', revision: '1' },
{ url: 'https://not-a-site.com/squip4.jpg', revision: '1' },
])

/* An alternative to patch package, where a custom precache controller is made and implemented
class CustomPrecacheController extends PrecacheController {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is included here to show an example of another approach to catching & handling precaching errors (option 2 in the main PR comment)

// Copied and modified slightly from the original `install` method
// in order to catch errors without scrapping the entire SW
// installation
install(event) {
const fetchAndCache = async () => {
// Cache entries one at a time.
// See https://github.com/GoogleChrome/workbox/issues/2528
for (const [url, cacheKey] of this._urlsToCacheKeys) {
const integrity =
this._cacheKeysToIntegrities.get(cacheKey)
const cacheMode = this._urlsToCacheModes.get(url)

const request = new Request(url, {
integrity,
cache: cacheMode,
credentials: 'same-origin',
})

await Promise.all(
this.strategy.handleAll({
params: { cacheKey },
request,
event,
})
).catch((err) => {
console.log('Whoops there was a precaching err!')
console.error(err)
})
}
}
event.waitUntil(fetchAndCache())
}
}
const precacheController = new CustomPrecacheController()

precacheController.addToCacheList([
{ url: 'https://not-a-site.com/index.html', revision: '5' },
{ url: 'https://not-a-site.com/hey.jpg', revision: '1000' },
{ url: 'https://not-a-site.com/bogus1.jpg', revision: '1000' },
{ url: 'https://not-a-site.com/derp2.jpg', revision: '1000' },
{ url: 'https://not-a-site.com/diddlibap3.jpg', revision: '1000' },
{ url: 'https://not-a-site.com/squip4.jpg', revision: '1000' },
])

self.addEventListener('install', (event) => {
precacheController.install(event)
})

self.addEventListener('activate', (event) => {
precacheController.activate(event)
})

const precacheRoute = new PrecacheRoute(precacheController)
registerRoute(precacheRoute) */
}

// Handling pings: only use the network, and don't update the connection
Expand Down
Loading
Loading