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

Add handlerDidError plugin callback #2569

Closed
jeffposnick opened this issue Jul 7, 2020 · 1 comment · Fixed by #2576
Closed

Add handlerDidError plugin callback #2569

jeffposnick opened this issue Jul 7, 2020 · 1 comment · Fixed by #2576
Labels
Discuss An open question, where input from the community would be appreciated. Feature Request workbox-strategies
Milestone

Comments

@jeffposnick
Copy link
Contributor

Library Affected:
workbox-strategies

Issue or Feature Request Description:
workbox-routing currently provides "fallback" support via the setCatchHandler() method, which is usually written as a big switch statement with a bunch of different response strategies for different types of requests all bunched together.

In addition to this approach, which is implemented on the global-router level, it would be a good developer experience to expose Strategy-level fallback support, with a handler that gets triggered whenever the "main" handler for a Strategy results in a WorkboxError.

This new setCatchHandler method on the Strategy base class would also be associated with a new TypeScript callback signature, that matched the normal handlerCallback signature, but included an additional error property, in case the developer wants access to the underlying error. (The workbox-routing setCatchHandler() should also use this new callback signature, and pass the underlying error through there as well.)

You can approximate this now by creating your own handlerCallback with a try/catch block and a call to a Strategy inside of it, but that's more awkward than it needs to be. Code that's currently:

const networkFirst = new NetworkFirst({...});

const handlerWithFallback = async (params) => {
  try {
    // Hopefully you remember the await here, or catch will never be triggered!
    return await networkFirst.handle(params);
  } catch (error) {
    return caches.match('/offline');
  }
};

Could be rewritten as:

const networkFirst = new NetworkFirst({...});

networkFirst.setCatchHandler((params) => {
  // params.error has the underlying error, if needed.
  return caches.match('/offline');
});

I think it should be pretty clean to support this with the new Strategy base class that @philipwalton put in place for v6.

@jeffposnick jeffposnick added Discuss An open question, where input from the community would be appreciated. Feature Request workbox-strategies labels Jul 7, 2020
@jeffposnick jeffposnick added this to the v6 milestone Jul 7, 2020
@jeffposnick jeffposnick changed the title Add setCatchHandler to the Strategy class Add handlerDidError plugin callback Jul 10, 2020
@jeffposnick
Copy link
Contributor Author

After discussing this with @philipwalton a bit, a cleaner approach would be to create a new handlerDidError callback that gets invoked in between handlerWillStart and handlerWillRespond, when the _handle() method throws an error.

await handler.runCallbacks('handlerWillStart', {event, request});
let response = await this._handle(request, handler);
for (const callback of handler.iterateCallbacks('handlerWillRespond')) {
response = await callback({event, request, response});
}

The signature would be roughly:

interface HandlerDidErrorCallbackParam {
  request: Request;
  event: ExtendableEvent;
  error: Error;
  state?: PluginState;
}

export interface HandlerDidErrorCallback {
  (param: HandlerDidErrorCallbackParam): Promise<Response | undefined>;
}

If the HandlerDidErrorCallback function throws or returns undefined, then the original error will be propagated and the handler will stop. If it returns a valid Response, then that will feed directly into the normal response flow, and handlerWillRespond will be invoked with the valid Response.

The example from the original proposal could be expressed as:

const networkFirst = new NetworkFirst({
  plugins: [{
    handlerDidError: () => caches.match('/offline'),
  }]
});

Using a handlerDidError callback makes it possible to create a new PrecacheFallbackPlugin in workbox-precaching that would implement handlerDidError by generating a response for a precached URL (the previous example would not work for URLs that include the __WB_REVISION=... query parameter, which many precached URLs would have). The common "fallback to a precached '/offline' error page" pattern would then be as simple as

registerRoute(
  ({request}) => request.mode === 'navigate',
  new NetworkOnly({
    plugins: [
      new PrecacheFallbackPlugin('/offline'),
    ],
  })
);

That should also be easy to translate into a runtimeCaching configuration, leading to an alternative to #2567

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discuss An open question, where input from the community would be appreciated. Feature Request workbox-strategies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant