Skip to content

Commit 75234bf

Browse files
authored
Merge branch 'master' into @invertase/oncallgenkit-rawrequest-signal
2 parents f21c01f + 6750b6e commit 75234bf

26 files changed

Lines changed: 506 additions & 36 deletions

.github/workflows/test.yaml

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ jobs:
1818
matrix:
1919
node-version:
2020
- 22.x
21+
- 24.x
2122
steps:
2223
- uses: actions/checkout@v4
2324
- uses: actions/setup-node@v4
@@ -52,8 +53,6 @@ jobs:
5253
strategy:
5354
matrix:
5455
node-version:
55-
- 18.x
56-
- 20.x
5756
- 22.x
5857
- 24.x
5958
steps:
@@ -71,8 +70,6 @@ jobs:
7170
strategy:
7271
matrix:
7372
node-version:
74-
- 18.x
75-
- 20.x
7673
- 22.x
7774
- 24.x
7875
steps:
@@ -96,8 +93,6 @@ jobs:
9693
strategy:
9794
matrix:
9895
node-version:
99-
- 18.x
100-
- 20.x
10196
- 22.x
10297
- 24.x
10398
steps:

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
1+
- Validate literal `timeoutSeconds` values per v2 trigger type (0-540s for events, 0-3600s for HTTPS/callable, 0-1800s for task queues, 0-7s for identity functions) so misconfigured values fail at function-definition or manifest-extraction time instead of at deploy time. (#1877)
2+
- chore: drop support for Node 18 and below (minimum supported version is now Node 20)
13
- feat: Add requiresAPI function to allow declaring Google Cloud API dependencies in code.
24
- fix(v1): Call onInit for schedule.onRun functions (#1801)

integration_test/run_tests.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ function cleanup {
9292
build_sdk
9393
delete_all_functions
9494

95-
for version in 14 16; do
95+
for version in 22 24; do
9696
create_package_json $TIMESTAMP $version "^10.0.0"
9797
install_deps
9898
announce "Re-deploying the same functions to Node $version runtime ..."

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -638,6 +638,6 @@
638638
}
639639
},
640640
"engines": {
641-
"node": ">=18.0.0"
641+
"node": ">=20.0.0"
642642
}
643643
}

spec/v2/options.spec.ts

Lines changed: 178 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,15 @@
2121
// SOFTWARE.
2222

2323
import { expect } from "chai";
24-
import { defineJsonSecret, defineSecret } from "../../src/params";
25-
import { GlobalOptions, optionsToEndpoint, RESET_VALUE } from "../../src/v2/options";
24+
import { defineInt, defineJsonSecret, defineSecret } from "../../src/params";
25+
import {
26+
assertTimeoutSecondsValid,
27+
GlobalOptions,
28+
optionsToEndpoint,
29+
optionsToTriggerAnnotations,
30+
RESET_VALUE,
31+
setGlobalOptions,
32+
} from "../../src/v2/options";
2633

2734
describe("GlobalOptions", () => {
2835
it("should accept all valid secret types in secrets array (type test)", () => {
@@ -92,3 +99,172 @@ describe("optionsToEndpoint", () => {
9299
expect(endpoint.vpc).to.equal(RESET_VALUE);
93100
});
94101
});
102+
103+
describe("assertTimeoutSecondsValid", () => {
104+
afterEach(() => {
105+
setGlobalOptions({});
106+
});
107+
108+
it("is a no-op when timeoutSeconds is undefined", () => {
109+
expect(() => assertTimeoutSecondsValid({}, "event")).to.not.throw();
110+
expect(() => assertTimeoutSecondsValid({}, "https")).to.not.throw();
111+
expect(() => assertTimeoutSecondsValid({}, "task")).to.not.throw();
112+
expect(() => assertTimeoutSecondsValid({}, "identity")).to.not.throw();
113+
});
114+
115+
it("accepts values within each kind's limit", () => {
116+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 540 }, "event")).to.not.throw();
117+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3600 }, "https")).to.not.throw();
118+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1800 }, "task")).to.not.throw();
119+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 0 }, "event")).to.not.throw();
120+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1 }, "event")).to.not.throw();
121+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 7 }, "identity")).to.not.throw();
122+
});
123+
124+
it("throws when timeoutSeconds exceeds the event-handler limit", () => {
125+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3600 }, "event")).to.throw(
126+
/between 0 and 540 for event-handling functions/
127+
);
128+
});
129+
130+
it("throws when timeoutSeconds exceeds the HTTPS limit", () => {
131+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 3601 }, "https")).to.throw(
132+
/between 0 and 3600 for HTTPS and callable functions/
133+
);
134+
});
135+
136+
it("throws when timeoutSeconds exceeds the task-queue limit", () => {
137+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1801 }, "task")).to.throw(
138+
/between 0 and 1800 for task queue functions/
139+
);
140+
});
141+
142+
it("throws when timeoutSeconds exceeds the identity limit", () => {
143+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 8 }, "identity")).to.throw(
144+
/between 0 and 7 for blocking functions/
145+
);
146+
});
147+
148+
it("throws when timeoutSeconds is negative", () => {
149+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: -1 }, "event")).to.throw(
150+
/between 0 and 540/
151+
);
152+
});
153+
154+
it("skips validation for Expression timeouts", () => {
155+
const expr = { timeoutSeconds: defineInt("TIMEOUT") };
156+
expect(() => assertTimeoutSecondsValid(expr, "event")).to.not.throw();
157+
});
158+
159+
it("skips validation for RESET_VALUE timeouts", () => {
160+
const opts = { timeoutSeconds: RESET_VALUE as unknown as number };
161+
expect(() => assertTimeoutSecondsValid(opts, "event")).to.not.throw();
162+
});
163+
164+
it("throws when timeoutSeconds has an invalid non-number type", () => {
165+
const opts = { timeoutSeconds: "30" as unknown as number };
166+
expect(() => assertTimeoutSecondsValid(opts, "event")).to.throw(
167+
/must be a number, Expression, or RESET_VALUE/
168+
);
169+
});
170+
171+
it("throws when timeoutSeconds is null", () => {
172+
const opts = { timeoutSeconds: null as unknown as number };
173+
expect(() => assertTimeoutSecondsValid(opts, "event")).to.throw(
174+
/must be a number, Expression, or RESET_VALUE/
175+
);
176+
});
177+
178+
it("throws when global timeoutSeconds has an invalid non-number type", () => {
179+
setGlobalOptions({ timeoutSeconds: true as unknown as number });
180+
expect(() => assertTimeoutSecondsValid({}, "event")).to.throw(
181+
/must be a number, Expression, or RESET_VALUE/
182+
);
183+
});
184+
185+
it("falls back to the global timeoutSeconds when the function-level option is absent", () => {
186+
setGlobalOptions({ timeoutSeconds: 3600 });
187+
expect(() => assertTimeoutSecondsValid({}, "event")).to.throw(
188+
/between 0 and 540 for event-handling functions/
189+
);
190+
expect(() => assertTimeoutSecondsValid({}, "https")).to.not.throw();
191+
expect(() => assertTimeoutSecondsValid({}, "task")).to.throw();
192+
expect(() => assertTimeoutSecondsValid({}, "identity")).to.throw();
193+
});
194+
195+
it("prefers the function-level timeoutSeconds over the global one", () => {
196+
setGlobalOptions({ timeoutSeconds: 60 });
197+
expect(() => assertTimeoutSecondsValid({ timeoutSeconds: 1000 }, "event")).to.throw(
198+
/between 0 and 540/
199+
);
200+
});
201+
202+
it("treats a function-level RESET_VALUE as a clear of an out-of-range global", () => {
203+
setGlobalOptions({ timeoutSeconds: 3600 });
204+
expect(() =>
205+
assertTimeoutSecondsValid({ timeoutSeconds: RESET_VALUE as unknown as number }, "event")
206+
).to.not.throw();
207+
});
208+
});
209+
210+
describe("optionsToEndpoint timeout validation", () => {
211+
afterEach(() => {
212+
setGlobalOptions({});
213+
});
214+
215+
it("does not validate when kind is omitted (backwards compatibility)", () => {
216+
expect(() => optionsToEndpoint({ timeoutSeconds: 9999 })).to.not.throw();
217+
});
218+
219+
it("throws when kind is provided and timeoutSeconds exceeds the limit", () => {
220+
expect(() => optionsToEndpoint({ timeoutSeconds: 3600 }, "event")).to.throw(
221+
/between 0 and 540/
222+
);
223+
expect(() => optionsToEndpoint({ timeoutSeconds: 3601 }, "https")).to.throw(
224+
/between 0 and 3600/
225+
);
226+
expect(() => optionsToEndpoint({ timeoutSeconds: 1801 }, "task")).to.throw(
227+
/between 0 and 1800/
228+
);
229+
expect(() => optionsToEndpoint({ timeoutSeconds: 8 }, "identity")).to.throw(/between 0 and 7/);
230+
});
231+
232+
it("is a no-op for in-range timeouts when kind is provided", () => {
233+
expect(() => optionsToEndpoint({ timeoutSeconds: 540 }, "event")).to.not.throw();
234+
expect(() => optionsToEndpoint({ timeoutSeconds: 3600 }, "https")).to.not.throw();
235+
expect(() => optionsToEndpoint({ timeoutSeconds: 1800 }, "task")).to.not.throw();
236+
expect(() => optionsToEndpoint({ timeoutSeconds: 7 }, "identity")).to.not.throw();
237+
});
238+
});
239+
240+
describe("optionsToTriggerAnnotations timeout validation", () => {
241+
afterEach(() => {
242+
setGlobalOptions({});
243+
});
244+
245+
it("does not validate when kind is omitted (backwards compatibility)", () => {
246+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 9999 })).to.not.throw();
247+
});
248+
249+
it("throws when kind is provided and timeoutSeconds exceeds the limit", () => {
250+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3600 }, "event")).to.throw(
251+
/between 0 and 540/
252+
);
253+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3601 }, "https")).to.throw(
254+
/between 0 and 3600/
255+
);
256+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 1801 }, "task")).to.throw(
257+
/between 0 and 1800/
258+
);
259+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 8 }, "identity")).to.throw(
260+
/between 0 and 7/
261+
);
262+
});
263+
264+
it("is a no-op for in-range timeouts when kind is provided", () => {
265+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 540 }, "event")).to.not.throw();
266+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 3600 }, "https")).to.not.throw();
267+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 1800 }, "task")).to.not.throw();
268+
expect(() => optionsToTriggerAnnotations({ timeoutSeconds: 7 }, "identity")).to.not.throw();
269+
});
270+
});

spec/v2/providers/https.spec.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,14 @@ describe("onRequest", () => {
336336
await runHandler(func, req);
337337
expect(hello).to.equal("world");
338338
});
339+
340+
it("rejects timeoutSeconds above the 3600s HTTPS limit", () => {
341+
expect(() =>
342+
https.onRequest({ timeoutSeconds: 3601 }, (_req, res) => {
343+
res.end();
344+
})
345+
).to.throw(/between 0 and 3600 for HTTPS and callable functions/);
346+
});
339347
});
340348

341349
describe("onCall", () => {
@@ -605,6 +613,12 @@ describe("onCall", () => {
605613
expect(hello).to.equal("world");
606614
});
607615

616+
it("rejects timeoutSeconds above the 3600s HTTPS limit", () => {
617+
expect(() => https.onCall({ timeoutSeconds: 3601 }, () => 42)).to.throw(
618+
/between 0 and 3600 for HTTPS and callable functions/
619+
);
620+
});
621+
608622
describe("authPolicy", () => {
609623
before(() => {
610624
sinon.stub(debug, "isDebugFeatureEnabled").withArgs("skipTokenVerification").returns(true);

spec/v2/providers/pubsub.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,19 @@ describe("onMessagePublished", () => {
135135
expect(res).to.equal("input");
136136
});
137137

138+
it("rejects timeoutSeconds above the 540s event-handler limit", () => {
139+
expect(() =>
140+
pubsub.onMessagePublished({ topic: "topic", timeoutSeconds: 3600 }, () => 42)
141+
).to.throw(/between 0 and 540 for event-handling functions/);
142+
});
143+
144+
it("rejects a global timeoutSeconds above the 540s event-handler limit", () => {
145+
options.setGlobalOptions({ timeoutSeconds: 3600 });
146+
expect(() => pubsub.onMessagePublished("topic", () => 42)).to.throw(
147+
/between 0 and 540 for event-handling functions/
148+
);
149+
});
150+
138151
it("should parse pubsub messages", async () => {
139152
let json: unknown;
140153
const messageJSON = {

spec/v2/providers/storage.spec.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,6 +503,14 @@ describe("v2/storage", () => {
503503
await storage.onObjectFinalized("bucket", () => null)(event);
504504
expect(hello).to.equal("world");
505505
});
506+
507+
it("rejects timeoutSeconds above the 540s event-handler limit on __endpoint access", () => {
508+
const func = storage.onObjectFinalized(
509+
{ bucket: "my-bucket", timeoutSeconds: 3600 },
510+
() => 42
511+
);
512+
expect(() => func.__endpoint).to.throw(/between 0 and 540 for event-handling functions/);
513+
});
506514
});
507515

508516
describe("onObjectDeleted", () => {

spec/v2/providers/tasks.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,12 @@ describe("onTaskDispatched", () => {
324324
expect(hello).to.equal("world");
325325
});
326326

327+
it("rejects timeoutSeconds above the 1800s task-queue limit", () => {
328+
expect(() => onTaskDispatched({ timeoutSeconds: 1801 }, () => null)).to.throw(
329+
/between 0 and 1800 for task queue functions/
330+
);
331+
});
332+
327333
describe("v1-compatible getters", () => {
328334
it("should provide v1-compatible context on the request object", async () => {
329335
let capturedRequest: any;

0 commit comments

Comments
 (0)