feat: sync scheduled function timeout with attempt deadline (#9544)

Rollback #9464

Firebase CLI will now synchronizes Cloud Scheduler's attemptDeadline with the function's timeoutSeconds for v2 scheduled functions, capped at 1800 seconds.

This is a change from original attempt where we tried to expose a new configuration for scheduled function that allowed users to directly set the attemptDeadline. We don't know why they would ever drift, so we'll simplify make sure the Scheduler's timeout is in sync with functions timeout.

Note that Cloud Scheduler has an attempt deadline range of [15s, 1800s], and defaults to 180s. We floor at 180s to be safe, even if the function timeout is shorter.

This is because GCF/Cloud Run will already terminate the function at its configured timeout, so Cloud Scheduler won't actually wait the full 180s unless GCF itself fails to respond. Setting it shorter than 180s might cause premature retries due to network latency.

Author: taeold

CHANGELOG.md

@@ -1 +1,2 @@
 - Fixed issue where MCP server didn't detect if iOS app uses Crashlytics in projects that use `project.pbxproj` (#9515)
+- Add logic to synchronize v2 scheduled function timeout with Cloud Schduler's attempt deadline (#9544)

src/deploy/functions/backend.ts

@@ -26,7 +26,6 @@ export interface ScheduleTrigger {
   schedule?: string;
   timeZone?: string | null;
   retryConfig?: ScheduleRetryConfig | null;
-  attemptDeadlineSeconds?: number | null;
 }
 
 /** Something that has a ScheduleTrigger */
@@ -187,16 +186,6 @@ export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettin
   return egress === "PRIVATE_RANGES_ONLY" || egress === "ALL_TRAFFIC";
 }
 
-export const MIN_ATTEMPT_DEADLINE_SECONDS = 15;
-export const MAX_ATTEMPT_DEADLINE_SECONDS = 1800; // 30 mins
-
-/**
- * Is a given number a valid attempt deadline?
- */
-export function isValidAttemptDeadline(seconds: number): boolean {
-  return seconds >= MIN_ATTEMPT_DEADLINE_SECONDS && seconds <= MAX_ATTEMPT_DEADLINE_SECONDS;
-}
-
 /** Returns a human-readable name with MB or GB suffix for a MemoryOption (MB). */
 export function memoryOptionDisplayName(option: MemoryOptions): string {
   return {

src/deploy/functions/build.spec.ts

@@ -224,44 +224,6 @@ describe("toBackend", () => {
       expect(endpointDef.func.serviceAccount).to.equal("service-account-1@");
     }
   });
-
-  it("throws if attemptDeadlineSeconds is out of range", () => {
-    const desiredBuild: build.Build = build.of({
-      func: {
-        platform: "gcfv2",
-        region: ["us-central1"],
-        project: "project",
-        runtime: "nodejs16",
-        entryPoint: "func",
-        scheduleTrigger: {
-          schedule: "every 1 minutes",
-          attemptDeadlineSeconds: 10, // Invalid: < 15
-        },
-      },
-    });
-    expect(() => build.toBackend(desiredBuild, {})).to.throw(
-      FirebaseError,
-      /attemptDeadlineSeconds must be between 15 and 1800 seconds/,
-    );
-
-    const desiredBuild2: build.Build = build.of({
-      func: {
-        platform: "gcfv2",
-        region: ["us-central1"],
-        project: "project",
-        runtime: "nodejs16",
-        entryPoint: "func",
-        scheduleTrigger: {
-          schedule: "every 1 minutes",
-          attemptDeadlineSeconds: 1801, // Invalid: > 1800
-        },
-      },
-    });
-    expect(() => build.toBackend(desiredBuild2, {})).to.throw(
-      FirebaseError,
-      /attemptDeadlineSeconds must be between 15 and 1800 seconds/,
-    );
-  });
 });
 
 describe("envWithType", () => {

src/deploy/functions/build.ts

@@ -147,7 +147,6 @@ export interface ScheduleTrigger {
   schedule: string | Expression<string>;
   timeZone?: Field<string>;
   retryConfig?: ScheduleRetryConfig | null;
-  attemptDeadlineSeconds?: Field<number>;
 }
 
 export type HttpsTriggered = { httpsTrigger: HttpsTrigger };
@@ -604,18 +603,6 @@ function discoverTrigger(endpoint: Endpoint, region: string, r: Resolver): backe
     } else if (endpoint.scheduleTrigger.retryConfig === null) {
       bkSchedule.retryConfig = null;
     }
-    if (typeof endpoint.scheduleTrigger.attemptDeadlineSeconds !== "undefined") {
-      const attemptDeadlineSeconds = r.resolveInt(endpoint.scheduleTrigger.attemptDeadlineSeconds);
-      if (
-        attemptDeadlineSeconds !== null &&
-        !backend.isValidAttemptDeadline(attemptDeadlineSeconds)
-      ) {
-        throw new FirebaseError(
-          `attemptDeadlineSeconds must be between ${backend.MIN_ATTEMPT_DEADLINE_SECONDS} and ${backend.MAX_ATTEMPT_DEADLINE_SECONDS} seconds (inclusive).`,
-        );
-      }
-      bkSchedule.attemptDeadlineSeconds = attemptDeadlineSeconds;
-    }
     return { scheduleTrigger: bkSchedule };
   } else if ("taskQueueTrigger" in endpoint) {
     const taskQueueTrigger: backend.TaskQueueTrigger = {};

src/deploy/functions/runtimes/discovery/v1alpha1.spec.ts

@@ -716,7 +716,6 @@ describe("buildFromV1Alpha", () => {
           maxRetrySeconds: 120,
           maxDoublings: 10,
         },
-        attemptDeadlineSeconds: 300,
       };
 
       const yaml: v1alpha1.WireManifest = {
@@ -745,7 +744,6 @@ describe("buildFromV1Alpha", () => {
           maxRetrySeconds: "{{ params.RETRY_DURATION }}",
           maxDoublings: "{{ params.DOUBLINGS }}",
         },
-        attemptDeadlineSeconds: "{{ params.ATTEMPT_DEADLINE }}",
       };
 
       const yaml: v1alpha1.WireManifest = {

src/deploy/functions/runtimes/discovery/v1alpha1.ts

@@ -222,7 +222,6 @@ function assertBuildEndpoint(ep: WireEndpoint, id: string): void {
       schedule: "Field<string>",
       timeZone: "Field<string>?",
       retryConfig: "object?",
-      attemptDeadlineSeconds: "Field<number>?",
     });
     if (ep.scheduleTrigger.retryConfig) {
       assertKeyTypes(prefix + ".scheduleTrigger.retryConfig", ep.scheduleTrigger.retryConfig, {
@@ -378,7 +377,6 @@ function parseEndpointForBuild(
     } else if (ep.scheduleTrigger.retryConfig === null) {
       st.retryConfig = null;
     }
-    copyIfPresent(st, ep.scheduleTrigger, "attemptDeadlineSeconds");
     triggered = { scheduleTrigger: st };
   } else if (build.isTaskQueueTriggered(ep)) {
     const tq: build.TaskQueueTrigger = {};

src/deploy/functions/validate.spec.ts

@@ -4,6 +4,7 @@ import * as sinon from "sinon";
 import { FirebaseError } from "../../error";
 import * as fsutils from "../../fsutils";
 import * as validate from "./validate";
+import * as utils from "../../utils";
 import * as projectPath from "../../projectPath";
 import * as secretManager from "../../gcp/secretManager";
 import * as backend from "./backend";
@@ -463,6 +464,23 @@ describe("validate", () => {
 
       expect(() => validate.endpointsAreValid(want)).to.not.throw();
     });
+
+    it("warns for scheduled functions with timeout > 1800s", () => {
+      const logStub = sinon.stub(utils, "logLabeledWarning");
+      try {
+        const ep: backend.Endpoint = {
+          ...ENDPOINT_BASE,
+          scheduleTrigger: {
+            schedule: "every 1 minutes",
+          },
+          timeoutSeconds: 1801,
+        };
+        validate.endpointsAreValid(backend.of(ep));
+        expect(logStub.calledOnce).to.be.true;
+      } finally {
+        logStub.restore();
+      }
+    });
   });
 
   describe("endpointsAreUnqiue", () => {

src/deploy/functions/validate.ts

@@ -10,6 +10,13 @@ import * as utils from "../../utils";
 import * as secrets from "../../functions/secrets";
 import { serviceForEndpoint } from "./services";
 
+/**
+ * Cloud Scheduler requires attempt deadline in range [15s, 30min] with default of 3min.
+ * See https://cloud.google.com/scheduler/docs/reference/rest/v1/projects.locations.jobs#Job.FIELDS.attempt_deadline
+ */
+export const DEFAULT_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS = 180;
+export const MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS = 1800;
+
 function matchingIds(
   endpoints: backend.Endpoint[],
   filter: (endpoint: backend.Endpoint) => boolean,
@@ -28,11 +35,28 @@ const cpu = (endpoint: backend.Endpoint): number => {
     : endpoint.cpu ?? backend.memoryToGen2Cpu(mem(endpoint));
 };
 
+function validateScheduledTimeout(ep: backend.Endpoint): void {
+  if (
+    backend.isScheduleTriggered(ep) &&
+    ep.timeoutSeconds &&
+    ep.timeoutSeconds > MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS
+  ) {
+    utils.logLabeledWarning(
+      "functions",
+      `Scheduled function ${ep.id} has a timeout of ${ep.timeoutSeconds} seconds, ` +
+        `which exceeds the maximum attempt deadline of ${MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS} seconds for Cloud Scheduler. ` +
+        "This is probably not what you want! Having a timeout longer than the attempt deadline may lead to unexpected retries and multiple function executions. " +
+        `The attempt deadline will be capped at ${MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS} seconds.`,
+    );
+  }
+}
+
 /** Validate that the configuration for endpoints are valid. */
 export function endpointsAreValid(wantBackend: backend.Backend): void {
   const endpoints = backend.allEndpoints(wantBackend);
   functionIdsAreValid(endpoints);
   for (const ep of endpoints) {
+    validateScheduledTimeout(ep);
     serviceForEndpoint(ep).validateTrigger(ep, wantBackend);
   }
 

src/gcp/cloudscheduler.spec.ts

@@ -246,34 +246,54 @@ describe("cloudscheduler", () => {
     });
 
     it("should copy optional fields for v2 endpoints", async () => {
+      const ep: backend.Endpoint = {
+        ...V2_ENDPOINT,
+        scheduleTrigger: {
+          schedule: "every 1 minutes",
+          timeZone: "America/Los_Angeles",
+          retryConfig: {
+            maxDoublings: 2,
+            maxBackoffSeconds: 20,
+            minBackoffSeconds: 1,
+            maxRetrySeconds: 60,
+          },
+        },
+      };
+      expect(await cloudscheduler.jobFromEndpoint(ep, "region", "1234567")).to.deep.equal({
+        name: "projects/project/locations/region/jobs/firebase-schedule-id-region",
+        schedule: "every 1 minutes",
+        timeZone: "America/Los_Angeles",
+        retryConfig: {
+          maxDoublings: 2,
+          maxBackoffDuration: "20s",
+          minBackoffDuration: "1s",
+          maxRetryDuration: "60s",
+        },
+        httpTarget: {
+          uri: "https://my-uri.com",
+          httpMethod: "POST",
+          oidcToken: {
+            serviceAccountEmail: "1234567-compute@developer.gserviceaccount.com",
+          },
+        },
+      });
+    });
+
+    it("should sync attemptDeadline with timeoutSeconds for v2 endpoints", async () => {
       expect(
         await cloudscheduler.jobFromEndpoint(
           {
             ...V2_ENDPOINT,
-            scheduleTrigger: {
-              schedule: "every 1 minutes",
-              timeZone: "America/Los_Angeles",
-              retryConfig: {
-                maxDoublings: 2,
-                maxBackoffSeconds: 20,
-                minBackoffSeconds: 1,
-                maxRetrySeconds: 60,
-              },
-            },
+            timeoutSeconds: 300,
           },
           V2_ENDPOINT.region,
           "1234567",
         ),
       ).to.deep.equal({
         name: "projects/project/locations/region/jobs/firebase-schedule-id-region",
         schedule: "every 1 minutes",
-        timeZone: "America/Los_Angeles",
-        retryConfig: {
-          maxDoublings: 2,
-          maxBackoffDuration: "20s",
-          minBackoffDuration: "1s",
-          maxRetryDuration: "60s",
-        },
+        timeZone: "UTC",
+        attemptDeadline: "300s",
         httpTarget: {
           uri: "https://my-uri.com",
           httpMethod: "POST",
@@ -284,41 +304,39 @@ describe("cloudscheduler", () => {
       });
     });
 
-    it("should not copy attemptDeadlineSeconds for v1 endpoints", async () => {
+    it("should cap attemptDeadline at 1800s for v2 endpoints", async () => {
       expect(
         await cloudscheduler.jobFromEndpoint(
           {
-            ...V1_ENDPOINT,
-            scheduleTrigger: {
-              schedule: "every 1 minutes",
-              attemptDeadlineSeconds: 300,
-            },
+            ...V2_ENDPOINT,
+            // This is bad configuration.
+            // Users are discouraged from setting timeout >1800s for scheduled functions.
+            timeoutSeconds: 3600,
           },
-          "appEngineLocation",
+          V2_ENDPOINT.region,
           "1234567",
         ),
       ).to.deep.equal({
-        name: "projects/project/locations/appEngineLocation/jobs/firebase-schedule-id-region",
+        name: "projects/project/locations/region/jobs/firebase-schedule-id-region",
         schedule: "every 1 minutes",
-        timeZone: "America/Los_Angeles",
-        pubsubTarget: {
-          topicName: "projects/project/topics/firebase-schedule-id-region",
-          attributes: {
-            scheduled: "true",
+        timeZone: "UTC",
+        attemptDeadline: "1800s",
+        httpTarget: {
+          uri: "https://my-uri.com",
+          httpMethod: "POST",
+          oidcToken: {
+            serviceAccountEmail: "1234567-compute@developer.gserviceaccount.com",
           },
         },
       });
     });
 
-    it("should copy attemptDeadlineSeconds for v2 endpoints", async () => {
+    it("should floor attemptDeadline at 180s for v2 endpoints", async () => {
       expect(
         await cloudscheduler.jobFromEndpoint(
           {
             ...V2_ENDPOINT,
-            scheduleTrigger: {
-              schedule: "every 1 minutes",
-              attemptDeadlineSeconds: 300,
-            },
+            timeoutSeconds: 60,
           },
           V2_ENDPOINT.region,
           "1234567",
@@ -327,7 +345,7 @@ describe("cloudscheduler", () => {
         name: "projects/project/locations/region/jobs/firebase-schedule-id-region",
         schedule: "every 1 minutes",
         timeZone: "UTC",
-        attemptDeadline: "300s",
+        attemptDeadline: "180s",
         httpTarget: {
           uri: "https://my-uri.com",
           httpMethod: "POST",

src/gcp/cloudscheduler.ts

@@ -4,10 +4,14 @@ import { FirebaseError } from "../error";
 import { logger } from "../logger";
 import { cloudschedulerOrigin } from "../api";
 import { Client } from "../apiv2";
+import { assertExhaustive, nullsafeVisitor } from "../functional";
+import {
+  DEFAULT_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS,
+  MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS,
+} from "../deploy/functions/validate";
 import * as backend from "../deploy/functions/backend";
 import * as proto from "./proto";
 import * as gce from "../gcp/computeEngine";
-import { assertExhaustive, nullsafeVisitor } from "../functional";
 
 const VERSION = "v1";
 const DEFAULT_TIME_ZONE_V1 = "America/Los_Angeles";
@@ -263,13 +267,21 @@ export async function jobFromEndpoint(
   }
   job.schedule = endpoint.scheduleTrigger.schedule;
   if (endpoint.platform === "gcfv2" || endpoint.platform === "run") {
-    proto.convertIfPresent(
-      job,
-      endpoint.scheduleTrigger,
-      "attemptDeadline",
-      "attemptDeadlineSeconds",
-      nullsafeVisitor(proto.durationFromSeconds),
-    );
+    proto.convertIfPresent(job, endpoint, "attemptDeadline", "timeoutSeconds", (timeout) => {
+      if (timeout === null) {
+        return null;
+      }
+      // Cloud Scheduler has an attempt deadline range of [15s, 1800s], and defaults to 180s.
+      // We floor at 180s to be safe, even if the function timeout is shorter.
+      // This is because GCF/Cloud Run will already terminate the function at its configured timeout,
+      // so Cloud Scheduler won't actually wait the full 180s unless GCF itself fails to respond.
+      // Setting it shorter than 180s might cause premature retries due to network latency.
+      const attemptDeadlineSeconds = Math.max(
+        Math.min(timeout, MAX_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS),
+        DEFAULT_V2_SCHEDULE_ATTEMPT_DEADLINE_SECONDS,
+      );
+      return proto.durationFromSeconds(attemptDeadlineSeconds);
+    });
   }
   if (endpoint.scheduleTrigger.retryConfig) {
     job.retryConfig = {};