Skip to content

StaleWhileRevalidate never exposing network error due to minor code bug #3431

@micahjon

Description

@micahjon

First off, thanks for maintaining such an important piece of web architecture! It's always a joy to use workbox.

I'm working on adapting the StaleWhileRevalidate strategy to instead race the network against the cache (based on #2989 (comment)), with a bias toward network if available (using a timeout). Anyway, in this process, I noticed that, as written, the error variable will always be undefined:

Here's a link to the relevant lines in StaleWhileRevalidate.ts. I've included them below, omitting the logging.

const fetchAndCachePromise = handler.fetchAndCachePut(request).catch(() => {
  // Swallow this error because a 'no-response' error will be thrown in
  // main handler return flow. This will be in the `waitUntil()` flow.
});
void handler.waitUntil(fetchAndCachePromise);

let response = await handler.cacheMatch(request);

let error;
if (response) {
  // (logging)
} else {
  // (logging)
  try {
    // NOTE(philipwalton): Really annoying that we have to type cast here.
    // https://github.com/microsoft/TypeScript/issues/20006
    response = (await fetchAndCachePromise) as Response | undefined;
  } catch (err) {
    // !!!
    // !!! Wait a minute, this catch will never run since we swalled the error above, right?
    // !!!
    if (err instanceof Error) {
      error = err;
    }
  }
}

// (logging)

if (!response) {
  throw new WorkboxError('no-response', {url: request.url, error});
}
return response;

Unless I'm really having a brain fart, it seems like the catch handler will never run, and thus error will never be assigned. Should be as simple as assigning error inside of the fetchAndCachePromise's .catch() handler above. Happy to submit a PR if it'd be helpful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions