GoogleChromeLabs / sw-appcache-behavior

A service worker implementation of the behavior defined in a page's AppCache manifest.
Apache License 2.0
54 stars 15 forks source link

I get a "network error response" when using this on a live website #20

Closed thebigh2014 closed 4 years ago

thebigh2014 commented 4 years ago

I am attempting to use this to migrate from my web app's existing AppCache but I am encountering the following error:

Uncaught (in promise) DOMException: The FetchEvent for "SITE_ROOT" resulted in a network error response: an object that was not a Response was passed to respondWith().

and this points to the line:

event.respondWith(appcachePolyfill.handle(event));

It happens any time after the service worker has successfully cached the site's contents. Any help would be much appreciated.

Note: I am using this (polyfill) library because backwards compatibility with AppCache is still needed for my web app. I will only consider other libraries that allow me to continue to use my existing AppCache manifest.

thebigh2014 commented 4 years ago

So I've ~hacked~ come up with the following temporary solution by replacing the line of code above with:

    let responsePromise = appcachePolyfill.handle(event);

    responsePromise.then(response =>
    {
        try
        {
            if (response)
            {
                event.respondWith(response);
                return;
            }
        }
        catch (err) {}
    });

This is a pretty ugly way of getting past undefined responses (which cause the app not to load and display a built-in browser error page). It also suppresses DOMExceptions when the event handler is already finished. I don't consider this a final solution so I look forward to what others might have to say.

jeffposnick commented 4 years ago

Is SITE_ROOT actually what's included in your error message, or is that you substituting out the real URL when sharing the message on GitHub?

thebigh2014 commented 4 years ago

Sorry, yes, that's me substituting out the real URL.

jeffposnick commented 4 years ago

Thanks for clarifying.

The most likely part of the codebase that I could see resulting in handle() returning undefined is https://github.com/GoogleChromeLabs/sw-appcache-behavior/blob/f7dc78d4a26a8b83640e418a8936130eacb0d69f/packages/appcache-polyfill-sw/index.ts#L89-L96

if that cache.match() results in a miss. Do you have a CACHE section of your manifest? Would you be able to share your full manifest, and ideally a link to a version of your web app that presents this behavior? It would probably be the quickest way for me to investigate.

thebigh2014 commented 4 years ago

I don't have that section so I'll see if adding it changes anything. If not, then I'll see about posting the manifest and one of the staging URLs.

thebigh2014 commented 4 years ago

The current manifest is here. I added a CACHE section but it didn't result in any difference in the behavior.

I should also note that it previously had the following section at the end of the file, though its removal had no effect either:

NETWORK:
*
jeffposnick commented 4 years ago

Thanks for sharing that manifest. One avenue worth exploring is that you have URLs like ./index.html in your manifest, but you mentioned that you saw a failure when requesting the URL SITE_ROOT. Was the actual URL for SITE_ROOT something like https://example.com/? I.e. the origin and /, without /index.html?

If that's the case, it might point to an underlying need for this library to treat a request that ends in / as if it were actually a request for the URL with index.html at the end, at least when doing cache lookups. (This is some that's come up in the Workbox project.)

If at all possible, it would be ideal if you could pass along a URL that I could visit which reproduces the problem. You could send it directly via a Twitter DM to @jeffposnick, if you'd rather not share it publicly.

In the meantime, I think defensively checking whether the handle() promise results in a valid response, along with some fallback logic if it doesn't, is a legitimate workaround. I'd do that with something like:

async function getResponse(event) {
  try {
    const response = await appcachePolyfill.handle(event);
    if (response) {
      return response;
    }
    throw new Error('Invalid response from polyfill.');
  } catch (error) {
    // Fall back to the network if there isn't a valid response from the polyfill.
    return fetch(event.request);
  }
}

self.addEventListener('fetch', (event) => {
  event.respondWith(getResponse(event));
});
thebigh2014 commented 4 years ago

The manifest I provided is at the URL you can visit.

jeffposnick commented 4 years ago

Sorry, do you have a deployment that exhibits the original issue you were describing? The site that currently hosts your manifest that you linked to has your additional code from https://github.com/GoogleChromeLabs/sw-appcache-behavior/issues/20#issuecomment-705379581 in it.

(That additional code you're using won't work reliably, as event.respondWith() should be called synchronously, during the fetch handler's execution. Your additional code calls event.respondWith() inside of a .then() block, which might execute after the fetch handler has already missed its chance to respond. The sample code in https://github.com/GoogleChromeLabs/sw-appcache-behavior/issues/20#issuecomment-706380856 which calls event.respondWith() during the fetch handler's synchronous execution is an alternative.)

thebigh2014 commented 4 years ago

That is the only site I'm working with as making use of the library is a new feature for the web app. I can revert the site back to the original code as provided in the README of this repo.

thebigh2014 commented 4 years ago

So it now works while online (using your suggested code above https://github.com/GoogleChromeLabs/sw-appcache-behavior/issues/20#issuecomment-706380856) but when I switch to offline I get the following warning & error (along with the browser's offline error page):

Warning: The FetchEvent for "SITE_ROOT/" resulted in a network error response: the promise was rejected. Error: Uncaught (in promise) TypeError: Failed to fetch

I also see in the Network tab of DevTools that neither the SITE_ROOT (type: document) nor the sw.js (type: fetch) are able to load, with errors net::ERR_FAILED and net::ERR_INTERNET_DISCONNECTED respectively.

If that's the case, it might point to an underlying need for this library to treat a request that ends in / as if it were actually a request for the URL with index.html at the end, at least when doing cache lookups. (This is some that's come up in the Workbox project.)

This has me thinking that what you mentioned above is likely what now needs to be done.

thebigh2014 commented 4 years ago

I should also mention that if this library doesn't work out, I do plan on looking into Workbox. Though I am still hopeful that this library can do what it is designed to do. And thanks again for your correspondence in helping me find a solution to the issues I've encountered with it.

jeffposnick commented 4 years ago

I've taken a closer look at how the current staging version of your site behaves, with the changes from https://github.com/GoogleChromeLabs/sw-appcache-behavior/issues/20#issuecomment-706380856 in place.

One thing that I noticed is that neither your /index.html (nor your /) actually end up being added to the Cache Storage API, even though ./index.html is listed in your AppCache manifest. I realized that this is because your web server includes Cache-Control: max-age=0, no-cache, no-store, must-revalidate on that HTML response, and the AppCache logic explicitly checks for this and excludes those entries from being cached:

https://github.com/GoogleChromeLabs/sw-appcache-behavior/blob/f7dc78d4a26a8b83640e418a8936130eacb0d69f/packages/appcache-polyfill-window/index.ts#L74-L77

I think that this behavior matches what's in the AppCache specification. Are you sure that that HTML was being cached previously, under a "real" AppCache implementation?

(If you do decide to go the Workbox route, I'm happy to assist with any questions that come up with that.)

thebigh2014 commented 4 years ago

So I wanted to let you know where I am at now and what the service worker code looks like now. I removed the caching headers so the app now works offline and everything is caching correctly. I amended the sw.js file to be as follows:

importScripts('./lib/appcache-polyfill-sw/build/index.umd.js');

async function getResponse(event)
{
    const requestClone = event.request.clone();

    try
    {
        const response = await appcachePolyfill.handle(event);

        if (response)
        {
            return response;
        }

        throw new Error('Invalid response from polyfill.');
    }
    catch (error)
    {
        try
        {
            // Fall back to the network if there isn't a valid response from the polyfill.
            const response = await fetch(requestClone);

            if (response)
            {
                return response;
            }

            // Don't return anything if there is no response.
        }
        catch (error)
        {
            // Silently fail here.
        }
    }
}

self.addEventListener('fetch', (event) =>
{
    event.respondWith(getResponse(event));
});

As you can see, I had to make a clone of the event's request in order to be able to use it in the outer catch block (otherwise got an error that it couldn't be re-used). I decided to not do anything if there is no response in that catch block nor if the inner catch block is reached (since there's nothing really to do).