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

Recovering from fetch failures #939

Open
jakearchibald opened this Issue Aug 3, 2016 · 12 comments

Comments

Projects
None yet
4 participants
@jakearchibald
Copy link
Collaborator

jakearchibald commented Aug 3, 2016

If your SW is buggy, or something changes in a particular browser that causes it to start throwing errors, the user gets nothing until you fix your SW (unless the browser provides a "retry without service worker" feature).

It'd be nice to allow the developer to recover in these cases

In many cases, a .catch() at the end of the respondWith promise lets them do that, but not in the case of a response that fetch deems unacceptable, eg an opaque response to a CORS request.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 3, 2016

fetchEvent.isResponseAcceptable(response) or similar would help here.

event.respondWith(
  something.then(response => {
    if (!event.isResponseAcceptable(response)) throw Error("Boom");
    return response;
  }).catch(() => {
    return fetch(event.request).catch(() => genericErrorPage);
  })
)
@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 3, 2016

An alternative proposed by @wanderview is some kind of fetcherror event:

self.addEventListener('fetcherror', event => {
  event.fallbackToDefaultRequest();
});

The benefits of this is the isolation of your error handling code from the thing that might be causing the error. However, you can only fallback to default behaviour, not provide some other kind of response. But do we need a whole new event? What if the request's body has already been used? Is that a problem?

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 3, 2016

I should point out that the method names I've used above are somewhat deliberately ridiculous, we can bikeshed later.

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Aug 3, 2016

The reason I like the fetcherror event approach is someone can easily opt-in to the "never show the user an error screen because my SW screwed up" behavior. They can put this logic in a separate script and just importScripts() it in all their SW scripts. They don't have to worry about integrating it with the rest of their promise chaining.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Aug 4, 2016

but not in the case of a response that fetch deems unacceptable, eg an opaque response to a CORS request

Why would that not throw just the same?

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Aug 4, 2016

If we do something like isResponseAcceptable to provide access to the checks Fetch does it should probably accept a request and a response. Or be a method on a response that takes a request. You might want to figure out that relation without a FetchEvent instance.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 4, 2016

Why would that not throw just the same?

Throw where? One idea is to make event.respondWith return a promise for success/failure, but by that point you've lost the opportunity to provide a different response.

If we do something like isResponseAcceptable to provide access to the checks Fetch does it should probably accept a request and a response

Agreed. If it doesn't need additional data from the FetchEvent, your proposal is better.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Aug 16, 2016

I was chatting with @esprehn and he mentioned an idea around a service worker event that fires if a tab shows an error page, allowing you to get information on the error, and navigate the client if you want.

His use-case was recovering from full tab crashes, but it feels similar to what we're talking about here.

@wanderview

This comment has been minimized.

Copy link
Member

wanderview commented Aug 16, 2016

I was chatting with @esprehn and he mentioned an idea around a service worker event that fires if a tab shows an error page, allowing you to get information on the error, and navigate the client if you want.

Sounds interesting and useful, but is there potential evil here with https cert error pages, etc?

@n8schloss

This comment has been minimized.

Copy link

n8schloss commented Aug 16, 2016

(please ignore my previous and now deleted comment, that's what I get for commenting without reading the full issue for context)

Imo adding an event for browser crashes would be amazing! If something like this got implemented I can promise that we'd be using it within a few days. The use case for browser crashes is different enough from an https cert error that imo it'd be fine if there was a whitelist for which events that this happens for. It'd probably be good for there to be a whitelist anyway and some standard for error names/types or something like that.

@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Apr 4, 2017

F2F:

  • Facebook like this.
  • Reporting is priority.
  • Being able to recover can be shipped later.
  • Recovery could be "reload", "redirect to this URL" - this should be async so things like caches (or even service worker registration) can be cleared before retrying.
@jakearchibald

This comment has been minimized.

Copy link
Collaborator

jakearchibald commented Apr 4, 2017

This should happen for "network failure" errors that result in a generic error page being shown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment