feat: sync scheduled function timeout with attempt deadline

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 @@
   schedule?: string;
   timeZone?: string | null;
   retryConfig?: ScheduleRetryConfig | null;
-  attemptDeadlineSeconds?: number | null;
 }
 
 /** Something that has a ScheduleTrigger */
@@ -183,20 +182,10 @@
   return allMemoryOptions.includes(mem as MemoryOptions);
 }
 
 export function isValidEgressSetting(egress: unknown): egress is VpcEgressSettings {
   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 {
@@ -562,8 +551,8 @@
           existingBackend.endpoints[endpoint.region] || {};
         existingBackend.endpoints[endpoint.region][endpoint.id] = endpoint;
       }
     } catch (err: any) {
       logger.debug(err.message);
       unreachableRegions.run = ["unknown"];
     }
   } else {

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 @@
   schedule: string | Expression<string>;
   timeZone?: Field<string>;
   retryConfig?: ScheduleRetryConfig | null;
-  attemptDeadlineSeconds?: Field<number>;
 }
 
 export type HttpsTriggered = { httpsTrigger: HttpsTrigger };
@@ -457,7 +456,7 @@
       // List param, we try resolving a String param instead.
       try {
         regions = params.resolveList(bdEndpoint.region, paramValues);
       } catch (err: any) {
         if (err instanceof ExprParseError) {
           regions = [params.resolveString(bdEndpoint.region, paramValues)];
         } else {
@@ -604,18 +603,6 @@
     } 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 @@
       schedule: "Field<string>",
       timeZone: "Field<string>?",
       retryConfig: "object?",
-      attemptDeadlineSeconds: "Field<number>?",
     });
     if (ep.scheduleTrigger.retryConfig) {
       assertKeyTypes(prefix + ".scheduleTrigger.retryConfig", ep.scheduleTrigger.retryConfig, {
@@ -288,8 +287,8 @@
       retry: ep.eventTrigger.retry,
     };
     // Allow serviceAccountEmail but prefer serviceAccount
     if ("serviceAccountEmail" in (ep.eventTrigger as any)) {
       eventTrigger.serviceAccount = (ep.eventTrigger as any).serviceAccountEmail;
     }
     copyIfPresent(
       eventTrigger,
@@ -378,7 +377,6 @@
     } 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 = {};
@@ -416,7 +414,7 @@
     ...triggered,
   };
   // Allow "serviceAccountEmail" but prefer "serviceAccount"
   if ("serviceAccountEmail" in (ep as any)) {
     parsed.serviceAccount = (ep as any).serviceAccountEmail;
   }
   copyIfPresent(

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 = {};