Address PR comments

Author: hsubox76

packages/app-check/src/api.test.ts

@@ -380,7 +380,7 @@ describe('api', () => {
         isTokenAutoRefreshEnabled: false
       });
 
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getState(app).tokenObservers.length).to.equal(0);
 
       const fakeRecaptchaToken = 'fake-recaptcha-token';
       stub(reCAPTCHA, 'getToken').returns(Promise.resolve(fakeRecaptchaToken));
@@ -395,13 +395,13 @@ describe('api', () => {
 
       await internalApi.getToken(appCheck as AppCheckService);
 
-      expect(getState(app).tokenObservers.length).to.equal(2);
+      expect(getState(app).tokenObservers.length).to.equal(1);
 
       expect(errorFn1).to.be.calledOnce;
       expect(errorFn1.args[0][0].name).to.include('exchange error');
 
       unsubscribe1();
-      expect(getState(app).tokenObservers.length).to.equal(1);
+      expect(getState(app).tokenObservers.length).to.equal(0);
     });
   });
 });

packages/app-check/src/api.ts

@@ -98,12 +98,17 @@ export function initializeAppCheck(
 
   const appCheck = provider.initialize({ options });
   _activate(app, options.provider, options.isTokenAutoRefreshEnabled);
-  // Adding a listener will start the refresher and fetch a token if needed.
-  // This gets a token ready and prevents a delay when an internal library
-  // requests the token.
-  // Listener function does not need to do anything, its base functionality
-  // of calling getToken() already fetches token and writes it to memory/storage.
-  addTokenListener(appCheck, ListenerType.INTERNAL, () => {});
+  // If isTokenAutoRefreshEnabled is false, do not send any requests to the
+  // exchange endpoint without an explicit call from the user either directly
+  // or through another Firebase library (storage, functions, etc.)
+  if (getState(app).isTokenAutoRefreshEnabled) {
+    // Adding a listener will start the refresher and fetch a token if needed.
+    // This gets a token ready and prevents a delay when an internal library
+    // requests the token.
+    // Listener function does not need to do anything, its base functionality
+    // of calling getToken() already fetches token and writes it to memory/storage.
+    addTokenListener(appCheck, ListenerType.INTERNAL, () => {});
+  }
 
   return appCheck;
 }

packages/app-check/src/internal-api.test.ts

@@ -405,6 +405,11 @@ describe('internal api', () => {
 
       const token = await getToken(appCheck as AppCheckService);
 
+      // ReCaptchaV3Provider's _throttleData is private so checking
+      // the resulting error message to be sure it has roughly the
+      // correct throttle time. This also tests the time formatter.
+      // Check both the error itself and that it makes it through to
+      // console.warn
       expect(token.error?.message).to.include('503');
       expect(token.error?.message).to.include('00m');
       expect(warnStub.args[0][0]).to.include('503');
@@ -427,6 +432,12 @@ describe('internal api', () => {
 
       const token = await getToken(appCheck as AppCheckService);
 
+
+      // ReCaptchaV3Provider's _throttleData is private so checking
+      // the resulting error message to be sure it has roughly the
+      // correct throttle time. This also tests the time formatter.
+      // Check both the error itself and that it makes it through to
+      // console.warn
       expect(token.error?.message).to.include('403');
       expect(token.error?.message).to.include('1d');
       expect(warnStub.args[0][0]).to.include('403');
@@ -466,6 +477,8 @@ describe('internal api', () => {
         listener
       );
 
+      expect(getState(app).tokenRefresher?.isRunning()).to.be.undefined;
+
       // addTokenListener() waits for the result of cachedTokenPromise
       // before starting the refresher
       await getState(app).cachedTokenPromise;

packages/app-check/src/providers.test.ts

@@ -87,20 +87,22 @@ describe('ReCaptchaV3Provider', () => {
     await expect(provider.getToken()).to.be.rejectedWith('503');
     expect(exchangeTokenStub).not.to.be.called;
     exchangeTokenStub.resetHistory();
+    // Times below are max range of each random exponential wait,
+    // the possible range is 2^(backoff_count) plus or minus 50%
     // Wait for 1.5 seconds to pass, should call exchange endpoint again
     // (and be rejected again)
     clock.tick(1500);
     await expect(provider.getToken()).to.be.rejectedWith('503');
     expect(exchangeTokenStub).to.be.called;
     exchangeTokenStub.resetHistory();
-    // Wait for 10 seconds to pass, should call exchange endpoint again
+    // Wait for 3 seconds to pass, should call exchange endpoint again
     // (and be rejected again)
-    clock.tick(10000);
+    clock.tick(3000);
     await expect(provider.getToken()).to.be.rejectedWith('503');
     expect(exchangeTokenStub).to.be.called;
-    // Wait for 10 seconds to pass, should call exchange endpoint again
+    // Wait for 6 seconds to pass, should call exchange endpoint again
     // (and succeed)
-    clock.tick(10000);
+    clock.tick(6000);
     exchangeTokenStub.restore();
     exchangeTokenStub = stub(client, 'exchangeToken').resolves({
       token: 'fake-exchange-token',

packages/app-check/src/providers.ts

@@ -93,6 +93,7 @@ export class ReCaptchaV3Provider implements AppCheckProvider {
           Number((e as FirebaseError).customData?.httpStatus),
           this._throttleData
         );
+        console.log('next interval', this._throttleData.allowRequestsAfter - Date.now());
         throw ERROR_FACTORY.create(AppCheckError.THROTTLED, {
           time: getDurationString(
             this._throttleData.allowRequestsAfter - Date.now()
@@ -247,6 +248,8 @@ export class CustomProvider implements AppCheckProvider {
  * Set throttle data to block requests until after a certain time
  * depending on the failed request's status code.
  * @param httpStatus - Status code of failed request.
+ * @param throttleData - `ThrottleData` object containing previous throttle
+ * data state.
  * @returns Data about current throttle state and expiration time.
  */
 function setBackoff(