Improve error handling for App Hosting compute service account (#8815)

* improve error handling for compute sa

* throw FirebaseError for unexpected error

Author: blidd-google

src/apphosting/backend.spec.ts

@@ -215,6 +215,20 @@ describe("apphosting setup functions", () => {
       expect(createServiceAccountStub).to.be.calledOnce;
       expect(addServiceAccountToRolesStub).to.not.be.called;
     });
+
+    it("should throw an unexpected error", async () => {
+      testResourceIamPermissionsStub.rejects(
+        new FirebaseError("Unexpected error", { status: 500 }),
+      );
+
+      await expect(
+        ensureAppHostingComputeServiceAccount(projectId, serviceAccount),
+      ).to.be.rejectedWith("Unexpected error");
+
+      expect(testResourceIamPermissionsStub).to.be.calledOnce;
+      expect(createServiceAccountStub).to.not.be.called;
+      expect(addServiceAccountToRolesStub).to.not.be.called;
+    });
   });
 
   describe("deleteBackendAndPoll", () => {

src/apphosting/backend.ts

@@ -266,11 +266,17 @@ export async function ensureAppHostingComputeServiceAccount(
   } catch (err: unknown) {
     if (!(err instanceof FirebaseError)) {
       throw err;
-    } else if (err.status === 403) {
+    }
+    if (err.status === 403) {
       throw new FirebaseError(
         `Failed to create backend due to missing delegation permissions for ${sa}. Make sure you have the iam.serviceAccounts.actAs permission.`,
         { original: err },
       );
+    } else if (err.status !== 404) {
+      throw new FirebaseError(
+        "Unexpected error occurred while testing for IAM service account permissions",
+        { original: err },
+      );
     }
   }
   await provisionDefaultComputeServiceAccount(projectId);
@@ -373,7 +379,7 @@ async function provisionDefaultComputeServiceAccount(projectId: string): Promise
   } catch (err: unknown) {
     if (getErrStatus(err) === 400) {
       logWarning(
-        "Your App Hosting compute service account is still being provisioned in the background; you may continue with the init flow.",
+        "Your App Hosting compute service account is still being provisioned in the background. If you encounter an error, please try again after a few moments.",
       );
     } else {
       throw err;

src/deploy/apphosting/prepare.ts

@@ -12,7 +12,6 @@ import { needProjectId } from "../../projectUtils";
 import { checkbox, confirm } from "../../prompt";
 import { logLabeledBullet, logLabeledWarning } from "../../utils";
 import { Context } from "./args";
-import { FirebaseError } from "../../error";
 
 /**
  * Prepare backend targets to deploy from source. Checks that all required APIs are enabled,
@@ -22,17 +21,7 @@ export default async function (context: Context, options: Options): Promise<void
   const projectId = needProjectId(options);
   await ensureApiEnabled(options);
   await ensureRequiredApisEnabled(projectId);
-  try {
-    await ensureAppHostingComputeServiceAccount(projectId, /* serviceAccount= */ "");
-  } catch (err) {
-    if ((err as FirebaseError).status === 400) {
-      logLabeledWarning(
-        "apphosting",
-        "Your App Hosting compute service account is still being provisioned. Please try again in a few moments.",
-      );
-    }
-    throw err;
-  }
+  await ensureAppHostingComputeServiceAccount(projectId, /* serviceAccount= */ "");
 
   context.backendConfigs = new Map<string, AppHostingSingle>();
   context.backendLocations = new Map<string, string>();