Add network timeout cancellation to core network code

Author: sam-gc

packages-exp/auth-exp/src/api/index.test.ts

@@ -17,7 +17,9 @@
 
 import { assert, expect, use } from 'chai';
 import * as chaiAsPromised from 'chai-as-promised';
+import * as sinon from 'sinon';
 import { useFakeTimers } from 'sinon';
+import * as sinonChai from 'sinon-chai';
 
 import { FirebaseError } from '@firebase/util';
 
@@ -27,15 +29,11 @@ import * as mockFetch from '../../test/helpers/mock_fetch';
 import { AuthErrorCode } from '../core/errors';
 import { ConfigInternal } from '../model/auth';
 import {
-  _getFinalTarget,
-  _performApiRequest,
-  DEFAULT_API_TIMEOUT_MS,
-  Endpoint,
-  HttpHeader,
-  HttpMethod
+    _getFinalTarget, _performApiRequest, DEFAULT_API_TIMEOUT_MS, Endpoint, HttpHeader, HttpMethod
 } from './';
 import { ServerError } from './errors';
 
+ use(sinonChai);
 use(chaiAsPromised);
 
 describe('api/_performApiRequest', () => {
@@ -227,6 +225,22 @@ describe('api/_performApiRequest', () => {
       clock.restore();
     });
 
+    it('should clear the network timeout on success', async () => {
+      const clock = useFakeTimers();
+      sinon.spy(clock, 'clearTimeout');
+      mockFetch.setUp();
+      mockEndpoint(Endpoint.SIGN_UP, {});
+      const promise = _performApiRequest(
+        auth,
+        HttpMethod.POST,
+        Endpoint.SIGN_UP,
+        request
+      );
+      await promise;
+      expect(clock.clearTimeout).to.have.been.called;
+      clock.restore();
+    });
+
     it('should handle network failure', async () => {
       mockFetch.setUpWithOverride(() => {
         return new Promise<never>((_, reject) =>

packages-exp/auth-exp/src/api/index.ts

@@ -17,11 +17,7 @@
 
 import { FirebaseError, querystring } from '@firebase/util';
 
-import {
-  AUTH_ERROR_FACTORY,
-  AuthErrorCode,
-  NamedErrorParams
-} from '../core/errors';
+import { AUTH_ERROR_FACTORY, AuthErrorCode, NamedErrorParams } from '../core/errors';
 import { fail } from '../core/util/assert';
 import { Delay } from '../core/util/delay';
 import { _emulatorUrl } from '../core/util/emulator';
@@ -120,11 +116,16 @@ export async function _performFetchWithErrorHandling<V>(
   (auth as Auth)._canInitEmulator = false;
   const errorMap = { ...SERVER_ERROR_MAP, ...customErrorMap };
   try {
+    const networkTimeout = new NetworkTimeout<Response>(auth.name);
     const response: Response = await Promise.race<Promise<Response>>([
       fetchFn(),
-      makeNetworkTimeout(auth.name)
+      networkTimeout.promise
     ]);
 
+    // If we've reached this point, the fetch succeeded and the networkTimeout
+    // didn't throw; clear the network timeout delay so that Node won't hang
+    networkTimeout.clearNetworkTimeout();
+
     const json = await response.json();
     if ('needConfirmation' in json) {
       throw makeTaggedError(auth, AuthErrorCode.NEED_CONFIRMATION, json);
@@ -201,16 +202,26 @@ export function _getFinalTarget(
   return _emulatorUrl(auth.config, base);
 }
 
-function makeNetworkTimeout<T>(appName: string): Promise<T> {
-  return new Promise((_, reject) =>
-    setTimeout(() => {
+class NetworkTimeout<T> {
+  // Node timers and browser timers are fundamentally incompatible, but we
+  // don't care about the value here
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  private timer: any|null = null;
+  readonly promise = new Promise<T>((_, reject) => {
+    this.timer = setTimeout(() => {
       return reject(
         AUTH_ERROR_FACTORY.create(AuthErrorCode.TIMEOUT, {
-          appName
+          appName: this.appName
         })
       );
-    }, DEFAULT_API_TIMEOUT_MS.get())
-  );
+    }, DEFAULT_API_TIMEOUT_MS.get());
+  });
+
+  clearNetworkTimeout():void {
+    clearTimeout(this.timer);
+  }
+
+  constructor(private readonly appName: string) {}
 }
 
 interface PotentialResponse extends IdTokenResponse {