Adding default timeout for dart functions - #10501
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request introduces a default timeout of 60 seconds for Dart functions when not running in the emulator and ensures that GCFv2 endpoints are correctly mapped to the "run" platform. Comprehensive unit tests were added to verify these behaviors under various conditions. The review feedback identifies several violations of the repository style guide regarding the use of "any" as a type escape hatch in the new test file, recommending the use of proper interfaces or specific type assertions instead.
Sorry, something went wrong.
|
/gemini review |
Sorry, something went wrong.
There was a problem hiding this comment.
Code Review
This pull request implements a default timeout of 60 seconds for Dart functions when not explicitly configured, applicable only when not running in the emulator. The changes include logic within the Dart runtime delegate to apply this default and ensure the platform is correctly set to 'run' for GCFv2 functions, supported by new unit tests verifying these behaviors and the preservation of user-defined timeouts. I have no feedback to provide.
Sorry, something went wrong.
wandamora
left a comment
There was a problem hiding this comment.
LGTM % small nit
Sorry, something went wrong.
ajperel
left a comment
There was a problem hiding this comment.
Thanks for the quick work. A few more nits.
Sorry, something went wrong.
* Adding default timeout for dart functions * fix: resolve lint errors and remove any casts in tests * Adding change log * changelog * Addressing reviews * Rehauling the logic to handle all functions deployed as run * Modularize resolveDefaultTimeout
Fixes: #10487
This PR refactors the handling of default timeouts for functions deployed to the Cloud Run platform (such as Dart functions). To improve maintainability, it introduces a dedicated resolveDefaultTimeout method in the Firebase CLI (src/deploy/functions/prepare.ts) to apply a 60-second default timeout to all functions with platform: "run" if not specified, matching the expectation set by Node.js functions.
Additionally, the PR preserves existing timeouts for already deployed functions. If a function is already live in the cloud, the CLI will keep that timeout if the local code does not specify a new one, rather than overriding it with the new default.
Comprehensive unit tests have been added to prepare.spec.ts and updated in index.spec.ts to verify these behaviors, and all existing tests passed successfully.