networkTimeoutSeconds in NetworkOnly (#2620)

* networkTimeoutSeconds in NetworkOnly

* Linting

* Review feedback

Author: jeffposnick

packages/workbox-strategies/src/NetworkOnly.ts

@@ -8,14 +8,19 @@
 
 import {assert} from 'workbox-core/_private/assert.js';
 import {logger} from 'workbox-core/_private/logger.js';
+import {timeout} from 'workbox-core/_private/timeout.js';
 import {WorkboxError} from 'workbox-core/_private/WorkboxError.js';
 
-import {Strategy} from './Strategy.js';
+import {Strategy, StrategyOptions} from './Strategy.js';
 import {StrategyHandler} from './StrategyHandler.js';
 import {messages} from './utils/messages.js';
 import './_version.js';
 
 
+interface NetworkOnlyOptions extends Omit<StrategyOptions, 'cacheName' | 'matchOptions'> {
+  networkTimeoutSeconds?: number;
+}
+
 /**
  * An implementation of a
  * [network-only]{@link https://developers.google.com/web/fundamentals/instant-and-offline/offline-cookbook/#network-only}
@@ -30,6 +35,24 @@ import './_version.js';
  * @memberof module:workbox-strategies
  */
 class NetworkOnly extends Strategy {
+  private readonly _networkTimeoutSeconds: number;
+
+  /**
+   * @param {Object} [options]
+   * @param {Array<Object>} [options.plugins] [Plugins]{@link https://developers.google.com/web/tools/workbox/guides/using-plugins}
+   * to use in conjunction with this caching strategy.
+   * @param {Object} [options.fetchOptions] Values passed along to the
+   * [`init`](https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/fetch#Parameters)
+   * of all fetch() requests made by this strategy.
+   * @param {number} [options.networkTimeoutSeconds] If set, any network requests
+   * that fail to respond within the timeout will result in a network error.
+   */
+  constructor(options: NetworkOnlyOptions = {}) {
+    super(options);
+
+    this._networkTimeoutSeconds = options.networkTimeoutSeconds || 0;
+  }
+
   /**
    * @private
    * @param {Request|string} request A request to run this strategy for.
@@ -42,15 +65,27 @@ class NetworkOnly extends Strategy {
       assert!.isInstance(request, Request, {
         moduleName: 'workbox-strategies',
         className: this.constructor.name,
-        funcName: 'handle',
+        funcName: '_handle',
         paramName: 'request',
       });
     }
 
-    let error;
-    let response;
+    let error: Error | undefined = undefined;
+    let response: Response | undefined;
+
     try {
-      response = await handler.fetch(request);
+      const promises: Promise<Response|undefined>[] = [handler.fetch(request)];
+
+      if (this._networkTimeoutSeconds) {
+        const timeoutPromise = timeout(this._networkTimeoutSeconds * 1000) as Promise<undefined>;
+        promises.push(timeoutPromise);
+      }
+
+      response = await Promise.race(promises);
+      if (!response) {
+        throw new Error(`Timed out the network response after ` +
+            `${this._networkTimeoutSeconds} seconds.`);
+      }
     } catch (err) {
       error = err;
     }
@@ -74,4 +109,4 @@ class NetworkOnly extends Strategy {
   }
 }
 
-export {NetworkOnly};
+export {NetworkOnly, NetworkOnlyOptions};

test/workbox-strategies/sw/test-NetworkOnly.mjs

@@ -10,6 +10,7 @@ import {cacheNames} from 'workbox-core/_private/cacheNames.mjs';
 import {NetworkOnly} from 'workbox-strategies/NetworkOnly.mjs';
 import {spyOnEvent} from '../../../infra/testing/helpers/extendable-event-utils.mjs';
 import {generateUniqueResponse} from '../../../infra/testing/helpers/generateUniqueResponse.mjs';
+import {sleep} from '../../../infra/testing/helpers/sleep.mjs';
 
 
 describe(`NetworkOnly`, function() {
@@ -138,5 +139,33 @@ describe(`NetworkOnly`, function() {
       expect(fetchStub.calledOnce).to.be.true;
       expect(fetchStub.calledWith(request, fetchOptions)).to.be.true;
     });
+
+    it(`should throw if the network request times out`, async function() {
+      const request = new Request('http://example.io/test/');
+      const event = new FetchEvent('fetch', {request});
+      spyOnEvent(event);
+
+      // Use a short timeout to not slow down the test.
+      // Note Sinon fake timers do not work with `await timeout()` used
+      // in the current `StrategyHandler` implementation.
+      const networkTimeoutSeconds = 0.5;
+      const sleepLongerThanNetworkTimeout =
+          sleep(2 * networkTimeoutSeconds * 1000);
+
+      sandbox.stub(self, 'fetch').callsFake(async () => {
+        await sleepLongerThanNetworkTimeout;
+        return new Response('Unexpected Response');
+      });
+
+      const networkOnly = new NetworkOnly({networkTimeoutSeconds});
+
+      await expectError(
+          () => networkOnly.handle({event, request}),
+          'no-response',
+          (err) => {
+            expect(err.details.error.message).to.include(networkTimeoutSeconds);
+          },
+      );
+    });
   });
 });