fix(functions): surface unhandled exceptions during functions deployment

CHANGELOG.md

@@ -1,3 +1,4 @@
+- Fixed an issue where unhandled exceptions during Cloud Functions deployment could be silently ignored and exit with code 0 (#6989).
 - Added `crashlytics:heapdumps:{enable,disable,status}` commands to manage Crashlytics Android heap dump collection. (#11225)
 - Removed unused dependencies `proxy-agent`, `universal-analytics`, and `@apphosting/common`.
 - Fixed an issue where throttled task queues stopped processing remaining tasks when a task exhausted its retries or timed out (#6989, #11040).

src/deploy/functions/release/fabricator.spec.ts

@@ -453,7 +453,7 @@
     it("handles topics that already exist", async () => {
       pubsub.createTopic.callsFake(() => {
         const err = new Error("Already exists");
         (err as any).status = 409;
         return Promise.reject(err);
       });
       gcfv2.createFunction.resolves({ name: "op", done: false });
@@ -528,7 +528,7 @@
       eventarc.createChannel.callsFake(({ name }) => {
         expect(name).to.equal("channel");
         const err = new Error("Already exists");
         (err as any).status = 409;
         return Promise.reject(err);
       });
       gcfv2.createFunction.resolves({ name: "op", done: false });
@@ -594,7 +594,7 @@
       eventarc.getChannel.resolves(undefined);
       eventarc.createChannel.callsFake(() => {
         const err = new Error("🤷‍♂️");
         (err as any).status = 400;
         return Promise.reject(err);
       });
 
@@ -1933,7 +1933,7 @@
       expect(deleteEndpoint).to.not.have.been.called;
 
       // Resolve the create operation
       resolveCreate!();
 
       await applyPlanPromise;
 
@@ -1976,11 +1976,128 @@
       expect(scrapers).to.have.lengthOf(2);
       expect(scrapers[0]).to.not.equal(scrapers[1]);
     });
+
+    it("throws FirebaseError and skips deletes when applyUpserts rejects unexpectedly", async () => {
+      const ep1 = endpoint({ httpsTrigger: {} }, { id: "createFn", region: "us-central1" });
+      const ep2 = endpoint({ httpsTrigger: {} }, { id: "deleteFn", region: "us-west1" });
+      const plan: planner.DeploymentPlan = {
+        default: {
+          plannedBackend: backend.of(ep1),
+          regionalChangesets: {
+            "us-central1": {
+              endpointsToCreate: [ep1],
+              endpointsToUpdate: [],
+              endpointsToDelete: [],
+              endpointsToSkip: [],
+            },
+            "us-west1": {
+              endpointsToCreate: [],
+              endpointsToUpdate: [],
+              endpointsToDelete: [ep2],
+              endpointsToSkip: [],
+            },
+          },
+        },
+      };
+
+      const upsertCause = new Error("Unexpected upsert crash");
+      const applyUpsertsStub = sinon.stub(fab, "applyUpserts");
+      applyUpsertsStub.onFirstCall().rejects(upsertCause);
+      applyUpsertsStub.onSecondCall().resolves([]);
+      const applyDeletesSpy = sinon.spy(fab, "applyDeletes");
+
+      const err = (await expect(fab.applyPlan(plan)).to.be.rejectedWith(
+        FirebaseError,
+        "Fabricator.applyUpserts encountered an unhandled exception: Unexpected upsert crash",
+      )) as FirebaseError;
+      expect(err.original).to.equal(upsertCause);
+      expect(applyDeletesSpy).to.not.have.been.called;
+    });
+
+    it("throws FirebaseError with children and skips role removals when applyDeletes rejects across regions", async () => {
+      const ep1 = endpoint({ httpsTrigger: {} }, { id: "deleteFnCentral", region: "us-central1" });
+      const ep2 = endpoint({ httpsTrigger: {} }, { id: "deleteFnWest", region: "us-west1" });
+      const plan: planner.DeploymentPlan = {
+        default: {
+          plannedBackend: backend.empty(),
+          regionalChangesets: {
+            "us-central1": {
+              endpointsToCreate: [],
+              endpointsToUpdate: [],
+              endpointsToDelete: [ep1],
+              endpointsToSkip: [],
+            },
+            "us-west1": {
+              endpointsToCreate: [],
+              endpointsToUpdate: [],
+              endpointsToDelete: [ep2],
+              endpointsToSkip: [],
+            },
+          },
+        },
+      };
+
+      const applyDeletesStub = sinon.stub(fab, "applyDeletes");
+      applyDeletesStub.onFirstCall().rejects(new Error("Unexpected delete crash central"));
+      applyDeletesStub.onSecondCall().rejects(new Error("Unexpected delete crash west"));
+      const removeOldRolesSpy = sinon.spy(fab, "removeOldRoles");
+
+      const err = (await expect(fab.applyPlan(plan)).to.be.rejectedWith(
+        FirebaseError,
+        "Fabricator.applyDeletes encountered unhandled exceptions",
+      )) as FirebaseError;
+      expect(err.children).to.have.lengthOf(2);
+      expect((err.children[0] as Error).message).to.equal("Unexpected delete crash central");
+      expect((err.children[1] as Error).message).to.equal("Unexpected delete crash west");
+      expect(removeOldRolesSpy).to.not.have.been.called;
+    });
+
+    it("isolates logOpStart errors to the failing endpoint and handles unknown runtimes", async () => {
+      const epUnknownRuntime = endpoint(
+        { httpsTrigger: {} },
+        {
+          id: "unknownRuntimeFn",
+          region: "us-central1",
+          runtime: "custom-future-runtime" as backend.Endpoint["runtime"],
+        },
+      );
+      const epFailingLog = endpoint(
+        { httpsTrigger: {} },
+        { id: "failingLogFn", region: "us-central1" },
+      );
+      const changes: planner.Changeset = {
+        endpointsToCreate: [epUnknownRuntime, epFailingLog],
+        endpointsToUpdate: [],
+        endpointsToDelete: [],
+        endpointsToSkip: [],
+      };
+
+      sinon.stub(fab, "createEndpoint").resolves();
+      const origLogOpStart = fab.logOpStart.bind(fab);
+      sinon.stub(fab, "logOpStart").callsFake((op: string, ep: backend.Endpoint) => {
+        if (ep.id === "failingLogFn") {
+          throw new Error("logOpStart failure");
+        }
+        origLogOpStart(op, ep);
+      });
+
+      const results = await fab.applyUpserts(
+        changes,
+        new scraper.SourceTokenScraper(),
+        new scraper.SourceTokenScraper(),
+      );
+
+      expect(results).to.have.lengthOf(2);
+      expect(results[0].endpoint.id).to.equal("unknownRuntimeFn");
+      expect(results[0].error).to.be.undefined;
+      expect(results[1].endpoint.id).to.equal("failingLogFn");
+      expect(results[1].error?.message).to.equal("logOpStart failure");
+    });
   });
 
   describe("createRunFunction", () => {
     it("creates a Cloud Run service with correct configuration", async () => {
       runv2.createService.resolves({ uri: "https://service", name: "service" } as any);
       run.setInvokerCreate.resolves();
 
       const ep = endpoint(
@@ -2021,7 +2138,7 @@
     });
 
     it("always sets callable triggers to public on creation", async () => {
       runv2.createService.resolves({ uri: "https://service", name: "service" } as any);
       run.setInvokerCreate.resolves();
 
       const ep = endpoint(

src/deploy/functions/release/fabricator.ts

@@ -8,7 +8,7 @@ import {
   parseErrorCode,
 } from "./executor";
 import * as ensure from "../ensure";
-import { FirebaseError } from "../../../error";
+import { FirebaseError, getError, getErrStack } from "../../../error";
 
 import { SourceTokenScraper } from "./sourceTokenScraper";
 import { Timer } from "./timer";
@@ -275,20 +275,37 @@ export class Fabricator {
     const createAndUpdateResultsArray = await Promise.allSettled(createAndUpdatePromises);
 
     // Process results of Phase 1
+    const upsertErrors: Error[] = [];
     summary.results = createAndUpdateResultsArray.reduce<reporter.DeployResult[]>((acc, r) => {
       if (r.status === "fulfilled") {
         return [...acc, ...r.value];
       }
       // Handle rejection
+      upsertErrors.push(getError(r.reason));
       logger.debug(
         "Fabricator.applyUpserts returned an unhandled exception.",
-        JSON.stringify(r.reason, null, 2),
+        getErrStack(r.reason),
       );
       return acc;
     }, []);
 
     await this.cleanupUnusedServiceAccounts(plan, summary.results);
 
+    // If applyUpserts rejected outside wrapOperation, fail immediately instead of
+    // proceeding to function deletions with an incomplete result set and exiting 0.
+    if (upsertErrors.length) {
+      const opts =
+        upsertErrors.length === 1 ? { original: upsertErrors[0] } : { children: upsertErrors };
+      const msg =
+        upsertErrors.length === 1
+          ? `Fabricator.applyUpserts encountered an unhandled exception: ${upsertErrors[0].message}`
+          : "Fabricator.applyUpserts encountered unhandled exceptions";
+      throw new FirebaseError(msg, {
+        ...opts,
+        exit: 2,
+      });
+    }
+
     const hasFailures = summary.results.some((r) => r.error);
 
     if (hasFailures) {
@@ -312,19 +329,36 @@ export class Fabricator {
       allChangesets.map((changes) => this.applyDeletes(changes)),
     );
 
+    const deleteErrors: Error[] = [];
     const deleteResults = deleteResultsArray.reduce<reporter.DeployResult[]>((acc, r) => {
       if (r.status === "fulfilled") {
         return [...acc, ...r.value];
       }
+      deleteErrors.push(getError(r.reason));
       logger.debug(
         "Fabricator.applyDeletes returned an unhandled exception. This should never happen",
-        JSON.stringify(r.reason, null, 2),
+        getErrStack(r.reason),
       );
       return acc;
     }, []);
 
     summary.results.push(...deleteResults);
 
+    // If applyDeletes rejected outside wrapOperation, fail instead of returning
+    // an incomplete summary that allows the deploy to succeed with exit code 0.
+    if (deleteErrors.length) {
+      const opts =
+        deleteErrors.length === 1 ? { original: deleteErrors[0] } : { children: deleteErrors };
+      const msg =
+        deleteErrors.length === 1
+          ? `Fabricator.applyDeletes encountered an unhandled exception: ${deleteErrors[0].message}`
+          : "Fabricator.applyDeletes encountered unhandled exceptions";
+      throw new FirebaseError(msg, {
+        ...opts,
+        exit: 2,
+      });
+    }
+
     // Similarly to grantNewRoles, we execute role removals sequentially to avoid concurrent
     // IAM policy update conflicts (409 Conflict / etag mismatch) on GCP.
     for (const [codebase, codebasePlan] of Object.entries(plan)) {
@@ -370,11 +404,11 @@ export class Fabricator {
     const ops: Array<Promise<reporter.DeployResult>> = [];
 
     for (const endpoint of changes.endpointsToCreate) {
-      this.logOpStart("creating", endpoint);
       ops.push(
-        this.wrapOperation("create", endpoint, () =>
-          this.createEndpoint(endpoint, scraperV1, scraperV2),
-        ),
+        this.wrapOperation("create", endpoint, () => {
+          this.logOpStart("creating", endpoint);
+          return this.createEndpoint(endpoint, scraperV1, scraperV2);
+        }),
       );
     }
 
@@ -383,11 +417,11 @@ export class Fabricator {
     }
 
     for (const update of changes.endpointsToUpdate) {
-      this.logOpStart("updating", update.endpoint);
       ops.push(
-        this.wrapOperation("update", update.endpoint, () =>
-          this.updateEndpoint(update, scraperV1, scraperV2),
-        ),
+        this.wrapOperation("update", update.endpoint, () => {
+          this.logOpStart("updating", update.endpoint);
+          return this.updateEndpoint(update, scraperV1, scraperV2);
+        }),
       );
     }
 
@@ -398,8 +432,12 @@ export class Fabricator {
     const ops: Array<Promise<reporter.DeployResult>> = [];
 
     for (const endpoint of changes.endpointsToDelete) {
-      this.logOpStart("deleting", endpoint);
-      ops.push(this.wrapOperation("delete", endpoint, () => this.deleteEndpoint(endpoint)));
+      ops.push(
+        this.wrapOperation("delete", endpoint, () => {
+          this.logOpStart("deleting", endpoint);
+          return this.deleteEndpoint(endpoint);
+        }),
+      );
     }
 
     return Promise.all(ops);
@@ -1177,7 +1215,9 @@ export class Fabricator {
   }
 
   logOpStart(op: string, endpoint: backend.Endpoint): void {
-    const runtime = endpoint.runtime ? RUNTIMES[endpoint.runtime].friendly : "unknown";
+    const runtime = endpoint.runtime
+      ? RUNTIMES[endpoint.runtime]?.friendly ?? endpoint.runtime
+      : "unknown";
     const platform = getHumanFriendlyPlatformName(endpoint.platform);
     const label = helper.getFunctionLabel(endpoint);
     utils.logLabeledBullet(