Skip to content

fix(gengapic): Ensure stable order of dynamic routing header parameters#1661

Merged
bhshkh merged 2 commits intogoogleapis:mainfrom
bhshkh:fix/ordered-header-vals
Nov 3, 2025
Merged

fix(gengapic): Ensure stable order of dynamic routing header parameters#1661
bhshkh merged 2 commits intogoogleapis:mainfrom
bhshkh:fix/ordered-header-vals

Conversation

@bhshkh
Copy link
Copy Markdown
Contributor

@bhshkh bhshkh commented Nov 1, 2025

Description:

This PR fixes an intermittent InvalidArgument error in the Go client library for Firestore when calling the ExecutePipeline RPC.

Problem:

The Firestore backend now enforces a strict order for the parameters in the x-goog-request-params routing header. This was causing intermittent InvalidArgument errors in the Go client library for Firestore, as the ExecutePipeline RPC was failing with the following error:

rpc error: code = InvalidArgument desc = Invalid request routing header database_id=database-enterprise-01&project_id=cndb-sdk-golang-firestore. Please fill in the request header with format x-goog-request-params:project_id=cndb-sdk-golang-firestore&database_id=database-enterprise-01. 

The root cause of this issue is that the Go client generator was using a map to store the routing header parameters, which does not preserve the order of the parameters. This resulted in the project_id and database_id parameters being sent in a random order, leading to the intermittent failures.

Solution:

This PR fixes the issue by using a slice of strings to store the routing header parameters, which preserves the order in which they are added. This ensures that the x-goog-request-params header is always sent in the correct format, with project_id appearing before database_id.
It also ensures that existing behaviour of last value being used for the header is preserved. For e.g., if there are 2 "foo_name" keys, the last value will be used similar to existing flow.

This fix is based on the AIP for client libraries, which states that the order of fields in the routing header should be stable.

Bug: b/456536006

@bhshkh bhshkh requested a review from a team November 1, 2025 01:04
@bhshkh bhshkh enabled auto-merge (squash) November 1, 2025 01:07
Comment thread internal/gengapic/gengapic.go Outdated
@@ -458,12 +457,24 @@ func (g *generator) insertDynamicRequestHeaders(m *descriptorpb.MethodDescriptor
g.printf(" routingHeadersMap[%q] = %s", headerName, regexHelper)
Copy link
Copy Markdown
Member

@hongalex hongalex Nov 3, 2025

Choose a reason for hiding this comment

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

What if we just built routingHeaders like before at this line? Is routingHeadersMap needed to be constructed here (and if not can we get rid of this)

Maybe de-duplication is necessary but I didn't catch that from the description.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated

@bhshkh bhshkh force-pushed the fix/ordered-header-vals branch from 16d677c to 8c35986 Compare November 3, 2025 21:53
@bhshkh bhshkh merged commit 6b4035e into googleapis:main Nov 3, 2025
7 checks passed
@bhshkh bhshkh deleted the fix/ordered-header-vals branch November 3, 2025 23:42
bhshkh added a commit to googleapis/google-cloud-go that referenced this pull request Nov 10, 2025
#13283)

Changes in this PR:

1. firestore_client.go : Updated generated client as per
googleapis/gapic-generator-go#1661 . Removed
retries from tests since the headers have now been fixed.
2. Remove Equivalent since it was removed from backend.
3. Add/update comments
4. Add timestamp truncate (pending from
#13194) and string
split (pending from
#13245) functions.
5. add all the private preview general, key, logical (except iferror),
type and object functions.
See "Firestore Features (Pipeline)" sheet in
[go/firestore-query-tracker](http://go/firestore-query-tracker) for the
list of functions.
    Java reference:
-
https://github.com/googleapis/java-firestore/blob/ccaf9d4fac5bd87a4da3d37493ca66fdc7681bc3/google-cloud-firestore/src/main/java/com/google/cloud/firestore/pipeline/expressions/Expression.java



Previous pull requests

- #12217
- #12425
- #12538
- #13147
- #13199
- #13218
- #13194
- #13245
- #13270
- #13271
- #13279
- #13280
- #13281
- #13282
- googleapis/gapic-generator-go#1661
shollyman added a commit that referenced this pull request Nov 13, 2025
shollyman added a commit that referenced this pull request Nov 13, 2025
…1661)"

This reverts commit 6b4035e.

Reverts #1661

Related: b/460298510 and a regression in the storage API
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.

2 participants