GoogleChrome / GoogleChrome/workbox

`StaleWhileRevalidate` never exposing network error due to minor code bug

Open
#3,431 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
13k
Forks
880
Avg merge
2h 44m
Merged PRs (30d)
8

Description

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 https://github.com/GoogleChrome/workbox/issues/2989#issue-1067271388), 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](https://github.com/GoogleChrome/workbox/blob/v7/packages/workbox-strategies/src/StaleWhileRevalidate.ts#L83-L131). I've included them below, omitting the logging.
```ts
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.