fix(v2): validate timeoutSeconds per trigger type (addition)

CHANGELOG.md

@@ -0,0 +1 @@
+- Validate `timeoutSeconds` per v2 trigger type (540s for events, 3600s for HTTPS/callable, 1800s for task queues) so misconfigured values fail at function-definition or manifest-extraction time instead of at deploy time. (#1874)

spec/v2/options.spec.ts

@@ -21,8 +21,15 @@
 // SOFTWARE.
 
 import { expect } from "chai";
-import { defineJsonSecret, defineSecret } from "../../src/params";
-import { GlobalOptions, optionsToEndpoint, RESET_VALUE } from "../../src/v2/options";
+import { defineInt, defineJsonSecret, defineSecret } from "../../src/params";
+import {
+  assertTimeoutSecondsValid,
+  GlobalOptions,
+  optionsToEndpoint,
+  optionsToTriggerAnnotations,
+  RESET_VALUE,
+  setGlobalOptions,
+} from "../../src/v2/options";
 
 describe("GlobalOptions", () => {
   it("should accept all valid secret types in secrets array (type test)", () => {
@@ -92,3 +99,148 @@ describe("optionsToEndpoint", () => {
     expect(endpoint.vpc).to.equal(RESET_VALUE);
   });
 });
+
+describe("assertTimeoutSecondsValid", () => {
+  afterEach(() => {
+    setGlobalOptions({});
+  });
+
+  it("is a no-op when timeoutSeconds is undefined", () => {
+    expect(() => assertTimeoutSecondsValid({}, "event")).to.not.throw();
+    expect(() => assertTimeoutSecondsValid({}, "https")).to.not.throw();
+    expect(() => assertTimeoutSecondsValid({}, "task")).to.not.throw();
+  });
+
+  it("accepts values within each kind's limit", () => {
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 540 }, "event")).to.not.throw();
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3600 }, "https")).to.not.throw();
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1800 }, "task")).to.not.throw();
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1 }, "event")).to.not.throw();
+  });
+
+  it("throws when timeoutSeconds exceeds the event-handler limit", () => {
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3600 }, "event")).to.throw(
+      /between 1 and 540 for event-handling functions/
+    );
+  });
+
+  it("throws when timeoutSeconds exceeds the HTTPS limit", () => {
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3601 }, "https")).to.throw(
+      /between 1 and 3600 for HTTPS and callable functions/
+    );
+  });
+
+  it("throws when timeoutSeconds exceeds the task-queue limit", () => {
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1801 }, "task")).to.throw(
+      /between 1 and 1800 for task queue functions/
+    );
+  });
+
+  it("throws when timeoutSeconds is negative", () => {
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: -1 }, "event")).to.throw(
+      /between 1 and 540/
+    );
+  });
+
+  it("skips validation for Expression timeouts", () => {
+    const expr = { timeoutSeconds: defineInt("TIMEOUT") };
+    expect(() => assertTimeoutSecondsValid(expr, "event")).to.not.throw();
+  });
+
+  it("skips validation for RESET_VALUE timeouts", () => {
+    const opts = { timeoutSeconds: RESET_VALUE as unknown as number };
+    expect(() => assertTimeoutSecondsValid(opts, "event")).to.not.throw();
+  });
+
+  it("throws when timeoutSeconds has an invalid non-number type", () => {
+    const opts = { timeoutSeconds: "30" as unknown as number };
+    expect(() => assertTimeoutSecondsValid(opts, "event")).to.throw(
+      /must be a number, Expression, or RESET_VALUE/
+    );
+  });
+
+  it("throws when global timeoutSeconds has an invalid non-number type", () => {
+    setGlobalOptions({ timeoutSeconds: true as unknown as number });
+    expect(() => assertTimeoutSecondsValid({}, "event")).to.throw(
+      /must be a number, Expression, or RESET_VALUE/
+    );
+  });
+
+  it("falls back to the global timeoutSeconds when the function-level option is absent", () => {
+    setGlobalOptions({ timeoutSeconds: 3600 });
+    expect(() => assertTimeoutSecondsValid({}, "event")).to.throw(
+      /between 1 and 540 for event-handling functions/
+    );
+    expect(() => assertTimeoutSecondsValid({}, "https")).to.not.throw();
+  });
+
+  it("prefers the function-level timeoutSeconds over the global one", () => {
+    setGlobalOptions({ timeoutSeconds: 60 });
+    expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1000 }, "event")).to.throw(
+      /between 1 and 540/
+    );
+  });
+
+  it("treats a function-level RESET_VALUE as a clear of an out-of-range global", () => {
+    setGlobalOptions({ timeoutSeconds: 3600 });
+    expect(() =>
+      assertTimeoutSecondsValid({ timeoutSeconds: RESET_VALUE as unknown as number }, "event")
+    ).to.not.throw();
+  });
+});
+
+describe("optionsToEndpoint timeout validation", () => {
+  afterEach(() => {
+    setGlobalOptions({});
+  });
+
+  it("does not validate when kind is omitted (backwards compatibility)", () => {
+    expect(() => optionsToEndpoint({ timeoutSeconds: 9999 })).to.not.throw();
+  });
+
+  it("throws when kind is provided and timeoutSeconds exceeds the limit", () => {
+    expect(() => optionsToEndpoint({ timeoutSeconds: 3600 }, "event")).to.throw(
+      /between 1 and 540/
+    );
+    expect(() => optionsToEndpoint({ timeoutSeconds: 3601 }, "https")).to.throw(
+      /between 1 and 3600/
+    );
+    expect(() => optionsToEndpoint({ timeoutSeconds: 1801 }, "task")).to.throw(
+      /between 1 and 1800/
+    );
+  });
+
+  it("is a no-op for in-range timeouts when kind is provided", () => {
+    expect(() => optionsToEndpoint({ timeoutSeconds: 540 }, "event")).to.not.throw();
+    expect(() => optionsToEndpoint({ timeoutSeconds: 3600 }, "https")).to.not.throw();
+    expect(() => optionsToEndpoint({ timeoutSeconds: 1800 }, "task")).to.not.throw();
+  });
+});
+
+describe("optionsToTriggerAnnotations timeout validation", () => {
+  afterEach(() => {
+    setGlobalOptions({});
+  });
+
+  it("does not validate when kind is omitted (backwards compatibility)", () => {
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 9999 })).to.not.throw();
+  });
+
+  it("throws when kind is provided and timeoutSeconds exceeds the limit", () => {
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3600 }, "event")).to.throw(
+      /between 1 and 540/
+    );
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3601 }, "https")).to.throw(
+      /between 1 and 3600/
+    );
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 1801 }, "task")).to.throw(
+      /between 1 and 1800/
+    );
+  });
+
+  it("is a no-op for in-range timeouts when kind is provided", () => {
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 540 }, "event")).to.not.throw();
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3600 }, "https")).to.not.throw();
+    expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 1800 }, "task")).to.not.throw();
+  });
+});

spec/v2/providers/https.spec.ts

@@ -336,6 +336,14 @@ describe("onRequest", () => {
     await runHandler(func, req);
     expect(hello).to.equal("world");
   });
+
+  it("rejects timeoutSeconds above the 3600s HTTPS limit", () => {
+    expect(() =>
+      https.onRequest({ timeoutSeconds: 3601 }, (_req, res) => {
+        res.end();
+      })
+    ).to.throw(/between 1 and 3600 for HTTPS and callable functions/);
+  });
 });
 
 describe("onCall", () => {
@@ -605,6 +613,12 @@ describe("onCall", () => {
     expect(hello).to.equal("world");
   });
 
+  it("rejects timeoutSeconds above the 3600s HTTPS limit", () => {
+    expect(() => https.onCall({ timeoutSeconds: 3601 }, () => 42)).to.throw(
+      /between 1 and 3600 for HTTPS and callable functions/
+    );
+  });
+
   describe("authPolicy", () => {
     before(() => {
       sinon.stub(debug, "isDebugFeatureEnabled").withArgs("skipTokenVerification").returns(true);

spec/v2/providers/pubsub.spec.ts

@@ -134,6 +134,19 @@ describe("onMessagePublished", () => {
     expect(res).to.equal("input");
   });
 
+  it("rejects timeoutSeconds above the 540s event-handler limit", () => {
+    expect(() =>
+      pubsub.onMessagePublished({ topic: "topic", timeoutSeconds: 3600 }, () => 42)
+    ).to.throw(/between 1 and 540 for event-handling functions/);
+  });
+
+  it("rejects a global timeoutSeconds above the 540s event-handler limit", () => {
+    options.setGlobalOptions({ timeoutSeconds: 3600 });
+    expect(() => pubsub.onMessagePublished("topic", () => 42)).to.throw(
+      /between 1 and 540 for event-handling functions/
+    );
+  });
+
   it("should parse pubsub messages", async () => {
     let json: unknown;
     const messageJSON = {

spec/v2/providers/storage.spec.ts

@@ -503,6 +503,14 @@ describe("v2/storage", () => {
       await storage.onObjectFinalized("bucket", () => null)(event);
       expect(hello).to.equal("world");
     });
+
+    it("rejects timeoutSeconds above the 540s event-handler limit on __endpoint access", () => {
+      const func = storage.onObjectFinalized(
+        { bucket: "my-bucket", timeoutSeconds: 3600 },
+        () => 42
+      );
+      expect(() => func.__endpoint).to.throw(/between 1 and 540 for event-handling functions/);
+    });
   });
 
   describe("onObjectDeleted", () => {

spec/v2/providers/tasks.spec.ts

@@ -324,6 +324,12 @@ describe("onTaskDispatched", () => {
     expect(hello).to.equal("world");
   });
 
+  it("rejects timeoutSeconds above the 1800s task-queue limit", () => {
+    expect(() => onTaskDispatched({ timeoutSeconds: 1801 }, () => null)).to.throw(
+      /between 1 and 1800 for task queue functions/
+    );
+  });
+
   describe("v1-compatible getters", () => {
     it("should provide v1-compatible context on the request object", async () => {
       let capturedRequest: any;

spec/v2/providers/timeout.spec.ts

@@ -0,0 +1,146 @@
+import { expect } from "chai";
+import * as ai from "../../../src/v2/providers/ai";
+import * as database from "../../../src/v2/providers/database";
+import * as dataconnect from "../../../src/v2/providers/dataconnect";
+import * as eventarc from "../../../src/v2/providers/eventarc";
+import * as firestore from "../../../src/v2/providers/firestore";
+import * as https from "../../../src/v2/providers/https";
+import * as identity from "../../../src/v2/providers/identity";
+import * as options from "../../../src/v2/options";
+import * as pubsub from "../../../src/v2/providers/pubsub";
+import * as remoteConfig from "../../../src/v2/providers/remoteConfig";
+import * as scheduler from "../../../src/v2/providers/scheduler";
+import * as storage from "../../../src/v2/providers/storage";
+import * as tasks from "../../../src/v2/providers/tasks";
+import * as testLab from "../../../src/v2/providers/testLab";
+
+interface TimeoutCase {
+  name: string;
+  build: () => unknown;
+  expectedError: RegExp;
+  validateOnEndpointAccess?: boolean;
+}
+
+function expectTimeoutError(testCase: TimeoutCase): void {
+  expect(() => {
+    const fn = testCase.build() as { __endpoint?: unknown };
+    if (testCase.validateOnEndpointAccess) {
+      void fn.__endpoint;
+    }
+  }).to.throw(testCase.expectedError);
+}
+
+describe("v2 provider timeout validation", () => {
+  beforeEach(() => {
+    process.env.GCLOUD_PROJECT = "aProject";
+  });
+
+  afterEach(() => {
+    options.setGlobalOptions({});
+    delete process.env.GCLOUD_PROJECT;
+  });
+
+  const cases: TimeoutCase[] = [
+    {
+      name: "https.onRequest rejects HTTPS timeouts above 3600s",
+      build: () =>
+        https.onRequest({ timeoutSeconds: 3601 }, (_req, res) => {
+          res.end();
+        }),
+      expectedError: /between 1 and 3600 for HTTPS and callable functions/,
+    },
+    {
+      name: "https.onCall rejects HTTPS timeouts above 3600s",
+      build: () => https.onCall({ timeoutSeconds: 3601 }, () => 42),
+      expectedError: /between 1 and 3600 for HTTPS and callable functions/,
+    },
+    {
+      name: "ai.beforeGenerateContent rejects HTTPS timeouts above 3600s",
+      build: () => ai.beforeGenerateContent({ timeoutSeconds: 3601 }, () => ({})),
+      expectedError: /between 1 and 3600 for HTTPS and callable functions/,
+    },
+    {
+      name: "identity.beforeUserCreated rejects HTTPS timeouts above 3600s",
+      build: () => identity.beforeUserCreated({ timeoutSeconds: 3601 }, () => undefined),
+      expectedError: /between 1 and 3600 for HTTPS and callable functions/,
+    },
+    {
+      name: "tasks.onTaskDispatched rejects task timeouts above 1800s",
+      build: () => tasks.onTaskDispatched({ timeoutSeconds: 1801 }, () => null),
+      expectedError: /between 1 and 1800 for task queue functions/,
+    },
+    {
+      name: "pubsub.onMessagePublished rejects event timeouts above 540s",
+      build: () => pubsub.onMessagePublished({ topic: "topic", timeoutSeconds: 3600 }, () => 42),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "storage.onObjectFinalized rejects event timeouts above 540s",
+      build: () =>
+        storage.onObjectFinalized({ bucket: "bucket", timeoutSeconds: 3600 }, () => null),
+      expectedError: /between 1 and 540 for event-handling functions/,
+      validateOnEndpointAccess: true,
+    },
+    {
+      name: "database.onValueCreated rejects event timeouts above 540s",
+      build: () => {
+        return database.onValueCreated(
+          { ref: "/foo", instance: "instance", timeoutSeconds: 3600 },
+          () => null
+        );
+      },
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "firestore.onDocumentCreated rejects event timeouts above 540s",
+      build: () =>
+        firestore.onDocumentCreated({ document: "foo/{bar}", timeoutSeconds: 3600 }, () => null),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "eventarc.onCustomEventPublished rejects event timeouts above 540s",
+      build: () =>
+        eventarc.onCustomEventPublished(
+          { eventType: "event-type", timeoutSeconds: 3600 },
+          () => 42
+        ),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "remoteConfig.onConfigUpdated rejects event timeouts above 540s",
+      build: () => remoteConfig.onConfigUpdated({ timeoutSeconds: 3600 }, () => 42),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "scheduler.onSchedule rejects event timeouts above 540s",
+      build: () =>
+        scheduler.onSchedule({ schedule: "* * * * *", timeoutSeconds: 3600 }, () => null),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "testLab.onTestMatrixCompleted rejects event timeouts above 540s",
+      build: () => testLab.onTestMatrixCompleted({ timeoutSeconds: 3600 }, () => 42),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+    {
+      name: "dataconnect.onMutationExecuted rejects event timeouts above 540s",
+      build: () =>
+        dataconnect.onMutationExecuted(
+          {
+            service: "service",
+            connector: "connector",
+            operation: "operation",
+            timeoutSeconds: 3600,
+          },
+          () => true
+        ),
+      expectedError: /between 1 and 540 for event-handling functions/,
+    },
+  ];
+
+  for (const testCase of cases) {
+    it(testCase.name, () => {
+      expectTimeoutError(testCase);
+    });
+  }
+});