fix(functions): correct httpsCallable timeout units on macOS/web#9077
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug where Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates httpsCallable and httpsCallableFromUrl to only convert the timeout option from milliseconds to seconds on native platforms (Android/iOS), leaving it in milliseconds on other platforms (web/macos). It also adds unit and E2E tests to verify this behavior. The review feedback points out that mutating the user-provided options object directly can cause unexpected side effects if the object is reused, and suggests creating a shallow copy instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
112c6bf to
0295167
Compare
0295167 to
1947f41
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9077 +/- ##
==========================================
+ Coverage 68.45% 68.46% +0.02%
==========================================
Files 301 301
Lines 15878 15878
Branches 3304 3334 +30
==========================================
+ Hits 10867 10870 +3
- Misses 4782 4850 +68
+ Partials 229 158 -71
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Description
HttpsCallableOptions.timeoutis documented in milliseconds, butnamespaced.tsalways divided by 1000 before calling the native/web layer. That conversion is correct for Android/iOS native (seconds), but wrong forisOtherplatforms (macOS, web) which route through the Firebase JS SDK and expect milliseconds. Custom timeouts on those platforms were effectively 1/1000th of the intended value (e.g. 30s → 30ms), causing spuriousdeadline-exceedederrors.Related issues
None, found while attempting to mitigate functions e2e flakes in the drive to eliminate flakiness entirely
Lesson in general: the gap between 99% and 100% (or even 95% to 99%) is where you discover all sorts of things, even some long-standing bugs!
Release Summary
Fix
httpsCallable/httpsCallableFromUrltimeout handling on macOS and other web-backed platforms.Checklist
Android(unchanged; regression unit test)iOS(unchanged; area e2e passed)Other(macOS, web) — fix targete2etests added or updated inpackages/**/e2ejesttests added or updated inpackages/**/__tests__Test Plan
yarn tests:jest packages/functions/__tests__/httpsCallable-timeout-other.test.ts packages/functions/__tests__/httpsCallable-timeout-native.test.tsHttpsCallableOptions.timeout honors millisecond values on web/macossleeperV2timeout test