Merge remote-tracking branch 'origin/main' into mtewani/partial-errors

maneesht · maneesht · commit f08d28d5864d · 2025-03-19T14:45:06.000-07:00
diff --git a/.changeset/fluffy-otters-whisper.md b/.changeset/fluffy-otters-whisper.md
@@ -0,0 +1,5 @@
+---
+'@firebase/app-check': patch
+---
+
+Improve error handling in AppCheck. The publicly-exported `getToken()` will now throw `internalError` strings it was previously ignoring.
diff --git a/.github/workflows/release-pr.yml b/.github/workflows/release-pr.yml
@@ -24,6 +24,9 @@ jobs:
   release:
     name: Create Release PR
     runs-on: ubuntu-latest
+    permissions:
+      pull-requests: write
+      contents: write
     if: ${{ !startsWith(github.event.head_commit.message, 'Version Packages (#') }}
     steps:
       - name: Checkout Repo
diff --git a/packages/app-check/src/api.ts b/packages/app-check/src/api.ts
@@ -209,6 +209,9 @@ export async function getToken(
   if (result.error) {
     throw result.error;
   }
+  if (result.internalError) {
+    throw result.internalError;
+  }
   return { token: result.token };
 }
 
diff --git a/packages/app-check/src/errors.ts b/packages/app-check/src/errors.ts
@@ -27,6 +27,7 @@ export const enum AppCheckError {
   STORAGE_GET = 'storage-get',
   STORAGE_WRITE = 'storage-set',
   RECAPTCHA_ERROR = 'recaptcha-error',
+  INITIAL_THROTTLE = 'initial-throttle',
   THROTTLED = 'throttled'
 }
 
@@ -54,7 +55,8 @@ const ERRORS: ErrorMap<AppCheckError> = {
   [AppCheckError.STORAGE_WRITE]:
     'Error thrown when writing to storage. Original error: {$originalErrorMessage}.',
   [AppCheckError.RECAPTCHA_ERROR]: 'ReCAPTCHA error.',
-  [AppCheckError.THROTTLED]: `Requests throttled due to {$httpStatus} error. Attempts allowed again after {$time}`
+  [AppCheckError.INITIAL_THROTTLE]: `{$httpStatus} error. Attempts allowed again after {$time}`,
+  [AppCheckError.THROTTLED]: `Requests throttled due to previous {$httpStatus} error. Attempts allowed again after {$time}`
 };
 
 interface ErrorParams {
@@ -66,6 +68,7 @@ interface ErrorParams {
   [AppCheckError.STORAGE_OPEN]: { originalErrorMessage?: string };
   [AppCheckError.STORAGE_GET]: { originalErrorMessage?: string };
   [AppCheckError.STORAGE_WRITE]: { originalErrorMessage?: string };
+  [AppCheckError.INITIAL_THROTTLE]: { time: string; httpStatus: number };
   [AppCheckError.THROTTLED]: { time: string; httpStatus: number };
 }
 
diff --git a/packages/app-check/src/internal-api.test.ts b/packages/app-check/src/internal-api.test.ts
@@ -163,7 +163,7 @@ describe('internal api', () => {
       const error = new Error('oops, something went wrong');
       stub(client, 'exchangeToken').returns(Promise.reject(error));
 
-      const token = await getToken(appCheck as AppCheckService);
+      const token = await getToken(appCheck as AppCheckService, false, true);
 
       expect(reCAPTCHASpy).to.be.called;
       expect(token).to.deep.equal({
@@ -186,7 +186,7 @@ describe('internal api', () => {
       const error = new Error('oops, something went wrong');
       stub(client, 'exchangeToken').returns(Promise.reject(error));
 
-      const token = await getToken(appCheck as AppCheckService);
+      const token = await getToken(appCheck as AppCheckService, false, true);
 
       expect(token).to.deep.equal({
         token: formatDummyToken(defaultTokenErrorData),
@@ -208,7 +208,7 @@ describe('internal api', () => {
       const reCAPTCHASpy = stubGetRecaptchaToken('', false);
       const exchangeTokenStub = stub(client, 'exchangeToken');
 
-      const token = await getToken(appCheck as AppCheckService);
+      const token = await getToken(appCheck as AppCheckService, false, true);
 
       expect(reCAPTCHASpy).to.be.called;
       expect(exchangeTokenStub).to.not.be.called;
@@ -290,7 +290,6 @@ describe('internal api', () => {
     });
 
     it('calls 3P error handler if there is an error getting a token', async () => {
-      stub(console, 'error');
       const appCheck = initializeAppCheck(app, {
         provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
         isTokenAutoRefreshEnabled: true
@@ -314,7 +313,6 @@ describe('internal api', () => {
     });
 
     it('ignores listeners that throw', async () => {
-      stub(console, 'error');
       const appCheck = initializeAppCheck(app, {
         provider: new ReCaptchaV3Provider(FAKE_SITE_KEY),
         isTokenAutoRefreshEnabled: true
diff --git a/packages/app-check/src/internal-api.ts b/packages/app-check/src/internal-api.ts
@@ -60,7 +60,8 @@ export function formatDummyToken(
  */
 export async function getToken(
   appCheck: AppCheckService,
-  forceRefresh = false
+  forceRefresh = false,
+  shouldLogErrors = false
 ): Promise<AppCheckTokenResult> {
   const app = appCheck.app;
   ensureActivated(app);
@@ -136,11 +137,14 @@ export async function getToken(
       state.token = tokenFromDebugExchange;
       return { token: tokenFromDebugExchange.token };
     } catch (e) {
-      if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) {
+      if (
+        (e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}` ||
+        (e as FirebaseError).code ===
+          `appCheck/${AppCheckError.INITIAL_THROTTLE}`
+      ) {
         // Warn if throttled, but do not treat it as an error.
         logger.warn((e as FirebaseError).message);
-      } else {
-        // `getToken()` should never throw, but logging error text to console will aid debugging.
+      } else if (shouldLogErrors) {
         logger.error(e);
       }
       // Return dummy token and error
@@ -167,11 +171,13 @@ export async function getToken(
     }
     token = await getStateReference(app).exchangeTokenPromise;
   } catch (e) {
-    if ((e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}`) {
+    if (
+      (e as FirebaseError).code === `appCheck/${AppCheckError.THROTTLED}` ||
+      (e as FirebaseError).code === `appCheck/${AppCheckError.INITIAL_THROTTLE}`
+    ) {
       // Warn if throttled, but do not treat it as an error.
       logger.warn((e as FirebaseError).message);
-    } else {
-      // `getToken()` should never throw, but logging error text to console will aid debugging.
+    } else if (shouldLogErrors) {
       logger.error(e);
     }
     // Always save error to be added to dummy token.
diff --git a/packages/app-check/src/providers.ts b/packages/app-check/src/providers.ts
@@ -92,7 +92,7 @@ export class ReCaptchaV3Provider implements AppCheckProvider {
           Number((e as FirebaseError).customData?.httpStatus),
           this._throttleData
         );
-        throw ERROR_FACTORY.create(AppCheckError.THROTTLED, {
+        throw ERROR_FACTORY.create(AppCheckError.INITIAL_THROTTLE, {
           time: getDurationString(
             this._throttleData.allowRequestsAfter - Date.now()
           ),
@@ -185,7 +185,7 @@ export class ReCaptchaEnterpriseProvider implements AppCheckProvider {
           Number((e as FirebaseError).customData?.httpStatus),
           this._throttleData
         );
-        throw ERROR_FACTORY.create(AppCheckError.THROTTLED, {
+        throw ERROR_FACTORY.create(AppCheckError.INITIAL_THROTTLE, {
           time: getDurationString(
             this._throttleData.allowRequestsAfter - Date.now()
           ),
diff --git a/packages/vertexai/test-utils/convert-mocks.ts b/packages/vertexai/test-utils/convert-mocks.ts
@@ -27,7 +27,7 @@ const { join } = require('path');
 
 const mockResponseDir = join(
   __dirname,
-  'vertexai-sdk-test-data/mock-responses'
+  'vertexai-sdk-test-data/mock-responses/vertexai'
 );
 
 async function main(): Promise<void> {
diff --git a/scripts/update_vertexai_responses.sh b/scripts/update_vertexai_responses.sh
@@ -17,7 +17,7 @@
 # This script replaces mock response files for Vertex AI unit tests with a fresh
 # clone of the shared repository of Vertex AI test data.
 
-RESPONSES_VERSION='v6.*' # The major version of mock responses to use
+RESPONSES_VERSION='v7.*' # The major version of mock responses to use
 REPO_NAME="vertexai-sdk-test-data"
 REPO_LINK="https://github.com/FirebaseExtended/$REPO_NAME.git"
 

Original file line number	Diff line number	Diff line change
`@@ -209,6 +209,9 @@ export async function getToken(`
`209`	`209`	`if (result.error) {`
`210`	`210`	`throw result.error;`
`211`	`211`	`}`
	`212`	`+ if (result.internalError) {`
	`213`	`+ throw result.internalError;`
	`214`	`+ }`
`212`	`215`	`return { token: result.token };`
`213`	`216`	`}`
`214`	`217`