Skip to content

fix: route v2 callables through wrapV2 when platform is gcfv2 - #311

Merged
IzaakGough merged 16 commits into
masterfrom
@invertase/fix-wrap-v2-callable-detection
Oct 17, 2026
Merged

IzaakGough merged 16 commits into
masterfrom
@invertase/fix-wrap-v2-callable-detection

Conversation

@IzaakGough

@IzaakGough IzaakGough commented May 15, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #310.

With firebase-functions 7, wrap() treated v2 onCall functions as v1 callables, so the handler received no auth or other request fields.

This routes v2 callables to wrapV2 based on the function's trigger metadata instead of its parameter count.

  • The platform check is required. v1 callables also carry callable trigger metadata, so only functions marked gcfv2 take the v2 path.

Tested with unit tests for v2 callables, v2 onRequest and v1 callables, and against this MRE.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for identifying and wrapping GCFv2 callable functions by checking the platform field in the endpoint metadata. Key changes include exporting the isCallableV2Function utility, updating the wrap function to prioritize v2 callable detection, and adding a corresponding test case in spec/main.spec.ts to ensure correct routing. There are no review comments to address, and I have no further feedback to provide.

@IzaakGough
IzaakGough marked this pull request as ready for review May 15, 2026 14:00
@IzaakGough
IzaakGough requested a review from cabljac May 15, 2026 14:00

@cabljac cabljac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great. The __endpoint.platform === 'gcfv2' check is a much more stable signal than counting function parameters, and the regression test covers the routing path nicely. A couple of small questions below, neither blocking.

Comment thread spec/main.spec.ts
Comment thread src/main.ts
Comment thread src/main.ts Outdated

@cabljac cabljac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callable fix itself looks right to me: platform check first with the arity heuristic as fallback is the correct order, and the v1-callable negative test plus the realistic rest-param shape in the regression test address everything from my last review. Ran this against both firebase-functions 4.9.0 and 7.2.5 and the routing behaves. One behavioral concern inline before this goes in, plus a couple of smaller ones.

Comment thread src/main.ts Outdated
Comment thread src/main.ts
Comment thread spec/main.spec.ts
Comment thread src/main.ts

@cabljac cabljac left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with comments

Comment thread src/v2.ts Outdated
Comment thread src/v2.ts Outdated
Comment thread src/main.ts Outdated
Comment thread src/main.ts Outdated
IzaakGough and others added 5 commits October 15, 2026 13:06
Co-authored-by: Jacob Cable <32874567+cabljac@users.noreply.github.com>
Co-authored-by: Jacob Cable <32874567+cabljac@users.noreply.github.com>
Co-authored-by: Jacob Cable <32874567+cabljac@users.noreply.github.com>
Co-authored-by: Jacob Cable <32874567+cabljac@users.noreply.github.com>
@IzaakGough
IzaakGough merged commit a83a592 into master Oct 17, 2026
22 checks passed
@IzaakGough
IzaakGough deleted the @invertase/fix-wrap-v2-callable-detection branch October 17, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

firebase-functions-test@3.4.1 misdetects v2 callables

3 participants