-
Notifications
You must be signed in to change notification settings - Fork 830
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
Improve logging when Chrome's offline check fails #2749
Comments
Apologies, but I need a bit more information to determine how to help. Are you posting this because you don't expect there to be a request automatically made for your |
I'm curious as to why it shows up as an error, yes. |
So that's why the request is being made in the first place. In terms of why the request results in an error, are you precaching your
If you were precaching |
I am indeed not precaching |
It depends! If you precache Using the new Putting it all together, and leaving out the bits not related to the navigations:
|
@jeffposnick So I tried that, full code here: import { warmStrategyCache } from "workbox-recipes"
import { skipWaiting, clientsClaim } from "workbox-core"
import { cleanupOutdatedCaches, precacheAndRoute, matchPrecache } from "workbox-precaching"
import { registerRoute, setCatchHandler } from "workbox-routing"
import { NetworkFirst, StaleWhileRevalidate, CacheFirst } from "workbox-strategies"
import { CacheableResponsePlugin } from "workbox-cacheable-response"
import { ExpirationPlugin } from "workbox-expiration"
import * as googleAnalytics from "workbox-google-analytics"
googleAnalytics.initialize()
cleanupOutdatedCaches()
precacheAndRoute(self.__WB_MANIFEST)
clientsClaim()
function cacheKeyWillBeUsed({ request }) {
const url = new URL(request.url)
url.pathname = url.pathname.replace(/\/index\.html$/, "/")
url.pathname = url.pathname.replace(/\.html$/, "/")
// Clear out all search params.
url.search = ''
return url.href
}
const navigationStrategy = new NetworkFirst({
cacheName: 'pages',
plugins: [
new CacheableResponsePlugin({
statuses: [200],
}),
cacheKeyWillBeUsed,
],
});
registerRoute(
({ request }) => request.mode === 'navigate',
navigationStrategy
);
warmStrategyCache({
urls: ['/'],
strategy: navigationStrategy,
});
// Cache CSS, JS, and Web Worker requests with a Stale While Revalidate strategy
registerRoute(
// Check to see if the request's destination is style for stylesheets, script for JavaScript, or worker for web worker
({ request }) =>
request.destination === 'style' ||
request.destination === 'script' ||
request.destination === 'worker',
// Use a Stale While Revalidate caching strategy
new StaleWhileRevalidate({
// Put all cached files in a cache named 'assets'
cacheName: 'assets',
plugins: [
// Ensure that only requests that result in a 200 status are cached
new CacheableResponsePlugin({
statuses: [200],
}),
],
}),
);
registerRoute(
/\.(?:png|jpg|jpeg|svg|gif|ico|mp4)$/,
// Use the cache if it's available.
new CacheFirst({
cacheName: "image-cache",
fetchOptions: {
credentials: "include",
},
plugins: [
new ExpirationPlugin({
// Cache only 50 images.
maxEntries: 50,
// Cache for a maximum of a day.
maxAgeSeconds: 24 * 60 * 60,
}),
],
})
)
// Cache the Google Fonts stylesheets with a stale-while-revalidate strategy.
registerRoute(
({ url }) => url.origin === "https://fonts.googleapis.com",
new StaleWhileRevalidate({
cacheName: "google-fonts-stylesheets",
})
)
// Cache the underlying font files with a cache-first strategy for 1 year.
registerRoute(
({ url }) => url.origin === "https://fonts.gstatic.com",
new CacheFirst({
cacheName: "google-fonts-webfonts",
plugins: [
new CacheableResponsePlugin({
statuses: [0, 200],
}),
new ExpirationPlugin({
maxAgeSeconds: 60 * 60 * 24 * 365,
maxEntries: 30,
}),
],
})
)
// Catch routing errors, like if the user is offline
setCatchHandler(async ({ event }) => {
// Return the precached offline page if a document is being requested
if (event.request.destination === "document") {
return matchPrecache("/offline.html")
}
return Response.error()
})
addEventListener("message", (event) => {
if (event.data && event.data.type === "SKIP_WAITING") {
skipWaiting()
}
}) But this would still cause the fetch("http://127.0.0.1:8000/?utm_source=web_app_manifest&utm_medium=web_app_manifest&utm_campaign=web_app_manifest", {
"referrer": "",
"referrerPolicy": "unsafe-url",
"body": null,
"method": "GET",
"mode": "cors",
"credentials": "omit"
}); |
Hmm... do you happen to have this deployed to a public URL at which I could check it out live? Feel free to DM |
Yes, I can! This is currently deployed on https://development.docs.coregames.com/ |
This took a while to figure out until I tried it out in a TypeScript project, at which point it identified the following mistake:
Can you try again with those missing I am still trying to figure out exactly why there might be even a one-time failure to fetch that page when Chrome automatically checks for offline capabilities, but that fix should at least (I hope) make the cache fallback work for real navigations to your |
I deployed a new version with that change but it looks like the fetch still fails :( |
I was having a really hard time understanding why I was seeing your But yes, the automatic fetch that Chrome performs that's described at https://developer.chrome.com/blog/improved-pwa-offline-detection/ is still failing. I am now wondering if it might be related to #2744, which includes a change to the way some of the asynchronous I'd like to see if, once #2744 is merged and we cut the next Workbox release, that error from the automatic offline detection is still logged. In the meantime, I don't think it should actually affect the functionality of your web app—offline capabilities when you navigate "for real" are working. |
I updated to 6.1.1 today after I saw that the PR was merged but it looks like it still errors. Updated version is at the same URL as above. |
Sorry for the delay in getting back to you. I think that everything is working as "expected," but unfortunately it is a bit confusing now that the automatic offline navigation detection fires off a navigation request for your I realize that you're seeing the request failure message logged just because that's what happens when you use a This came up in another GitHub issue recently, and I guess it's more likely to cause confusion now that the automatic offline navigation detection is fired off by Chrome. I'm going to leave this issue open to track some work that we could do to improve the logging around a failed network request when using |
start_url
We also started seeing "Failed to Fetch" ghost requests errors with Chrome 89 release starting yesterday. These are coming from the new PWA installability checks as reported by Jeff. This is being done to prove that a website that claims to be installable through reporting a link rel="manifest" is really capable of delivering content while offline. So if you page is not reporting a manifest file, you'll not get this error reproduced. I was working earlier to try to get rid of the error message in console, which is misleading, but couldn't get rid of it. Using a StaleWhileRevalidate strategy to cache the root path is getting the new PWA installability check passing, but the strategy tries to revalidate the file using the original request (which is designed to fail) and throws the error. I couldn't find documentation or an easy way to detect this type of Request and avoid trying to send it to network or use it to update cache. It would be nice to have a flag or similar on the Request object indicating it is designed to fail. |
This gets a big tough to track down to all the async code, but ultimately, when there's an workbox/packages/workbox-strategies/src/Strategy.ts Lines 221 to 242 in 2c8bfe7
@philipwalton touched this code most recently, and I'm honestly not sure what the impact would be if the error were not thrown at this point, or only thrown when
What do you think, Phil? |
In addition to the code highlighted above, there's also console noise (e.g. red error messages) from this code: workbox/packages/workbox-strategies/src/StrategyHandler.ts Lines 212 to 213 in 2c8bfe7
I think we'd have to find a way to avoid both if we want to not scare developers into thinking something is wrong (when it isn't). Experimenting locally, when I comment out both error logging/throwing, my site doesn't emit any scary console messages from the offline check.
I think we could potentially remove the re-throw. I remember not being sure about this when I was implementing it, and I chose to keep it to help make sure developers didn't have errors in their caching logic (or something like that). If a developer wants to know if their |
Thinking about this a bit more (and playing with a few options in the code), I don't think we need (or want) to remove the rethrow in the The problem currently (upon further investigation) is that the fetch error is even making its way to the workbox/packages/workbox-strategies/src/StrategyHandler.ts Lines 150 to 151 in 2c8bfe7
I remember I originally chose to do this because I wanted to mimic the fact that I think we don't actually need to pass the response promise in either the When I try removing the However, even if we do this we'll still need to address the error logged here (as mentioned above): workbox/packages/workbox-strategies/src/StrategyHandler.ts Lines 212 to 213 in 2c8bfe7
|
I hit the same error with a home-grown service-worker and came to "workbox" in the hope it would be over it already. I don't get the scary red messages in Firefox. Alternatively removing the site's manifest (!) avoids generating the messages. |
Thanks for looking into this, @philipwalton. Regarding the development build logging messages, I think that's okay, as we also log additional info about how we're falling back to the cache: workbox/packages/workbox-strategies/src/NetworkFirst.ts Lines 207 to 214 in 2c8bfe7
If folks are using the development build, I think there's enough info about what's going on that they can at least determine that the request was ultimately successful, even if we log that network failure. I'm more concerned about folks using production builds, who just see the logged exception without any other context. Regarding that exception, your suggestion about only explicitly waiting on promises that aren't used for generating a response sounds good, but also sounds like a breaking change, so that might need to wait until Workbox v7. What I'm thinking for v6 is to decorate the exception message to at least provide some context, like:
(Exact wording to be decided.) |
And we can also change that |
I made both those changes in a local copy of Workbox used on https://so-pwa.firebaseapp.com/, and this is what ultimately is logged in the console when the automatic offline check takes place: This is still not "great" in that there's some red logged, but at least there's context. (It's a little noisy because the URL is long, unfortunately.) |
Also, I wanted to let folks know that separate from the issue of how Workbox logs things, we are actively working with the Chrome DevTools team to provide more context about what's going on during the automatic offline check. Those improvements would help developers regardless of whether they're using Workbox or not. |
After chatting with @philipwalton about this, I think the consensus is that making the change outlined in #2749 (comment) falls more into the category of "bug fix for unintended behavior" than "breaking change of documented API" so we should be able to get it out sooner in the next Workbox v6 release. |
Hi @jeffposnick , my app have similar issue. https://songbook.app/en?standalone=true - while this page URL is already in cache - workbox still throws an error that it can't fetch this url. I've tried to add it to runtime cache using axios get request and also directly adding it to cache like this:
Could you help to figure out how to fix this issue? Previously I've added this url to cache using axios get request and it worked, now it's broken, I believe after google "improved offline check". |
This should be resolved in https://github.com/GoogleChrome/workbox/releases/tag/v6.1.2 |
Awesome, thank you! |
CC: @jeffposnick Hi all, after some digging and testing. I found an INTERESTING way to pass new offline check in Chrome properly. With workbox My solution will completed remove that failed network request in https://github.com/shadowwalker/next-pwa/blob/3dfda13890a00b297ad171ee9122f623b4e6f848/index.js#L246
Note the My guess is that due to some asynchronous magic, this delays the offline check request in Chrome 89+ a bit, so that cache will be ready to serve that request. So what you should do if you are not using |
Hello @shadowwalker—I'd very much suggest not attempting to avoid DevTools noise via that approach. There's overhead in re-creating the request, and I don't really understand what's going on that would cause it to lead to different logs being displayed. The Chrome team has heard the feedback from this thread and elsewhere, and rolled back the automatic offline check, starting with Chrome 90. Stay tuned for updates to the public guidance. The check may be reinstated at some point in the future, but the developer experience will be taken into account in anything that gets reimplemented. Thanks for everyone's patience regarding this in the meantime! |
@jeffposnick I am using both
|
Welcome! Please use this template for reporting bugs or requesting features. For questions about using Workbox, the best place to ask is Stack Overflow, tagged with
[workbox]
: https://stackoverflow.com/questions/ask?tags=workboxLibrary Affected: workbox
workbox-sw, workbox-build, etc.
Browser & Platform: Google Chrome 89
E.g. Google Chrome v51.0.1 for Android, or "all browsers".
Issue or Feature Request Description: Uncaught (in promise) TypeError: Failed to fetch
My
start_url
is set to/?utm_source=web_app_manifest&utm_medium=web_app_manifest&utm_campaign=web_app_manifest
When reporting bugs, please include relevant JavaScript Console logs and links to public URLs at which the issue could be reproduced.
It also errors when deployed on a https url
My config looks like this:
The text was updated successfully, but these errors were encountered: