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

Remove error logging caused by offline check #2777

Merged
merged 1 commit into from
Mar 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 88 additions & 92 deletions packages/workbox-strategies/src/StrategyHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,85 +147,83 @@ class StrategyHandler {
* @param {Request|string} input The URL or request to fetch.
* @return {Promise<Response>}
*/
fetch(input: RequestInfo): Promise<Response> {
return this.waitUntil((async () => {
const {event} = this;
let request: Request = toRequest(input);

if (request.mode === 'navigate' &&
event instanceof FetchEvent &&
event.preloadResponse) {
const possiblePreloadResponse = await event.preloadResponse;
if (possiblePreloadResponse) {
if (process.env.NODE_ENV !== 'production') {
logger.log(`Using a preloaded navigation response for ` +
`'${getFriendlyURL(request.url)}'`);
}
return possiblePreloadResponse;
async fetch(input: RequestInfo): Promise<Response> {
const {event} = this;
let request: Request = toRequest(input);

if (request.mode === 'navigate' &&
event instanceof FetchEvent &&
event.preloadResponse) {
const possiblePreloadResponse = await event.preloadResponse;
if (possiblePreloadResponse) {
if (process.env.NODE_ENV !== 'production') {
logger.log(`Using a preloaded navigation response for ` +
`'${getFriendlyURL(request.url)}'`);
}
return possiblePreloadResponse;
}
}

// If there is a fetchDidFail plugin, we need to save a clone of the
// original request before it's either modified by a requestWillFetch
// plugin or before the original request's body is consumed via fetch().
const originalRequest = this.hasCallback('fetchDidFail') ?
request.clone() : null;
// If there is a fetchDidFail plugin, we need to save a clone of the
// original request before it's either modified by a requestWillFetch
// plugin or before the original request's body is consumed via fetch().
const originalRequest = this.hasCallback('fetchDidFail') ?
request.clone() : null;

try {
for (const cb of this.iterateCallbacks('requestWillFetch')) {
request = await cb({request: request.clone(), event});
}
} catch (err) {
throw new WorkboxError('plugin-error-request-will-fetch', {
thrownError: err,
});
try {
for (const cb of this.iterateCallbacks('requestWillFetch')) {
request = await cb({request: request.clone(), event});
}
} catch (err) {
throw new WorkboxError('plugin-error-request-will-fetch', {
thrownError: err,
});
}

// The request can be altered by plugins with `requestWillFetch` making
// the original request (most likely from a `fetch` event) different
// from the Request we make. Pass both to `fetchDidFail` to aid debugging.
const pluginFilteredRequest: Request = request.clone();
// The request can be altered by plugins with `requestWillFetch` making
// the original request (most likely from a `fetch` event) different
// from the Request we make. Pass both to `fetchDidFail` to aid debugging.
const pluginFilteredRequest: Request = request.clone();

try {
let fetchResponse: Response;
try {
let fetchResponse: Response;

// See https://github.com/GoogleChrome/workbox/issues/1796
fetchResponse = await fetch(request, request.mode === 'navigate' ?
undefined : this._strategy.fetchOptions);
// See https://github.com/GoogleChrome/workbox/issues/1796
fetchResponse = await fetch(request, request.mode === 'navigate' ?
undefined : this._strategy.fetchOptions);

if (process.env.NODE_ENV !== 'production') {
logger.debug(`Network request for ` +
`'${getFriendlyURL(request.url)}' returned a response with ` +
`status '${fetchResponse.status}'.`);
}
if (process.env.NODE_ENV !== 'production') {
logger.debug(`Network request for ` +
`'${getFriendlyURL(request.url)}' returned a response with ` +
`status '${fetchResponse.status}'.`);
}

for (const callback of this.iterateCallbacks('fetchDidSucceed')) {
fetchResponse = await callback({
event,
request: pluginFilteredRequest,
response: fetchResponse,
});
}
return fetchResponse;
} catch (error) {
if (process.env.NODE_ENV !== 'production') {
logger.error(`Network request for `+
`'${getFriendlyURL(request.url)}' threw an error.`, error);
}
for (const callback of this.iterateCallbacks('fetchDidSucceed')) {
fetchResponse = await callback({
event,
request: pluginFilteredRequest,
response: fetchResponse,
});
}
return fetchResponse;
} catch (error) {
if (process.env.NODE_ENV !== 'production') {
logger.log(`Network request for `+
`'${getFriendlyURL(request.url)}' threw an error.`, error);
}

// `originalRequest` will only exist if a `fetchDidFail` callback
// is being used (see above).
if (originalRequest) {
await this.runCallbacks('fetchDidFail', {
error,
event,
originalRequest: originalRequest.clone(),
request: pluginFilteredRequest.clone(),
});
}
throw error;
// `originalRequest` will only exist if a `fetchDidFail` callback
// is being used (see above).
if (originalRequest) {
await this.runCallbacks('fetchDidFail', {
error,
event,
originalRequest: originalRequest.clone(),
request: pluginFilteredRequest.clone(),
});
}
})());
throw error;
}
}

/**
Expand Down Expand Up @@ -259,36 +257,34 @@ class StrategyHandler {
* @param {Request|string} key The Request or URL to use as the cache key.
* @return {Promise<Response|undefined>} A matching response, if found.
*/
cacheMatch(key: RequestInfo): Promise<Response | undefined> {
return this.waitUntil((async () => {
const request: Request = toRequest(key);
let cachedResponse: Response | undefined;
const {cacheName, matchOptions} = this._strategy;
async cacheMatch(key: RequestInfo): Promise<Response | undefined> {
const request: Request = toRequest(key);
let cachedResponse: Response | undefined;
const {cacheName, matchOptions} = this._strategy;

const effectiveRequest = await this.getCacheKey(request, 'read');
const multiMatchOptions = {...matchOptions, ...{cacheName}};
const effectiveRequest = await this.getCacheKey(request, 'read');
const multiMatchOptions = {...matchOptions, ...{cacheName}};

cachedResponse = await caches.match(effectiveRequest, multiMatchOptions);
cachedResponse = await caches.match(effectiveRequest, multiMatchOptions);

if (process.env.NODE_ENV !== 'production') {
if (cachedResponse) {
logger.debug(`Found a cached response in '${cacheName}'.`);
} else {
logger.debug(`No cached response found in '${cacheName}'.`);
}
if (process.env.NODE_ENV !== 'production') {
if (cachedResponse) {
logger.debug(`Found a cached response in '${cacheName}'.`);
} else {
logger.debug(`No cached response found in '${cacheName}'.`);
}
}

for (const callback of this.iterateCallbacks('cachedResponseWillBeUsed')) {
cachedResponse = (await callback({
cacheName,
matchOptions,
cachedResponse,
request: effectiveRequest,
event: this.event,
})) || undefined;
}
return cachedResponse;
})());
for (const callback of this.iterateCallbacks('cachedResponseWillBeUsed')) {
cachedResponse = (await callback({
cacheName,
matchOptions,
cachedResponse,
request: effectiveRequest,
event: this.event,
})) || undefined;
}
return cachedResponse;
}

/**
Expand Down
15 changes: 2 additions & 13 deletions test/workbox-strategies/sw/test-NetworkFirst.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -196,21 +196,10 @@ describe(`NetworkFirst`, function() {
return networkResponse;
});

// To ensure an attempt to respond from cache is made.
const cacheReadSpy = sandbox.spy();
sandbox.stub(caches, 'match').resolves(undefined);

const networkFirst = new NetworkFirst({
networkTimeoutSeconds,
plugins: [
{
cacheKeyWillBeUsed: ({request, mode}) => {
if (mode === 'read') {
cacheReadSpy(request);
}
return request;
},
},
],
});
const handlePromise = networkFirst.handle({
request,
Expand All @@ -220,7 +209,7 @@ describe(`NetworkFirst`, function() {
const handlerResponse = await handlePromise;

expect(handlerResponse).to.equal(networkResponse);
expect(cacheReadSpy.firstCall.args[0]).to.equal(request);
expect(caches.match.firstCall.args[0]).to.equal(request);
});

it(`should throw when NetworkFirst() is called with an invalid networkTimeoutSeconds parameter`, function() {
Expand Down
2 changes: 1 addition & 1 deletion test/workbox-strategies/sw/test-StrategyHandler.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ describe(`StrategyHandler`, function() {
sandbox.spy(handler, 'cachePut');
sandbox.spy(handler, 'waitUntil');

handler.fetchAndCachePut('/url');
await handler.fetchAndCachePut('/url');

await handler.doneWaiting();
expect(handler.waitUntil.calledWith(
Expand Down