Propagate per-request headers through Go client#601
Conversation
WalkthroughAdds per-request header support across the Go client templates by introducing a Headers map in multiple option structs and propagating these options through all request execution paths. Updates tests to assert headers are sent and adjusts README examples to demonstrate header usage for Check. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant GoClient
participant HTTPAPI
App->>GoClient: Check(request, options{StoreId, Headers})
GoClient->>GoClient: Attach per-request options (StoreId, Headers)
GoClient->>HTTPAPI: HTTP request with headers
HTTPAPI-->>GoClient: Response
GoClient-->>App: Result
sequenceDiagram
participant App
participant GoClient
participant HTTPAPI
App->>GoClient: Write(items[], options{StoreId, Headers})
loop For each chunk
GoClient->>GoClient: Derive chunkOptions (incl. Headers)
GoClient->>HTTPAPI: Chunk request with headers
HTTPAPI-->>GoClient: Chunk response
end
GoClient-->>App: Aggregated result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (6)
config/clients/go/template/README_calling_api.mustache (5)
51-51: Fix missing method call separator in GetStore example
Options(options)Execute()is missing a dot beforeExecute().Apply this diff:
-store, err := fgaClient.GetStore(context.Background()).Options(options)Execute() +store, err := fgaClient.GetStore(context.Background()).Options(options).Execute()
188-188: Fix missing method call separator in ReadLatestAuthorizationModel example
Options(options)Execute()is missing a dot beforeExecute().Apply this diff:
-data, err := fgaClient.ReadLatestAuthorizationModel(context.Background()).Options(options)Execute() +data, err := fgaClient.ReadLatestAuthorizationModel(context.Background()).Options(options).Execute()
266-266: Correct variable name passed to Body() in Read example
requestBodyisn’t defined in this snippet; you declaredbody. UseBody(body).Apply this diff:
-data, err := fgaClient.Read(context.Background()).Body(requestBody).Options(options).Execute() +data, err := fgaClient.Read(context.Background()).Body(body).Options(options).Execute()
520-520: Fix Body() variable name in Expand, ListObjects, and ListRelations examplesThese examples define
body := ...but callBody(requestBody). UseBody(body)to avoid confusion.Apply these diffs:
- data, err := fgaClient.Expand(context.Background()).Body(requestBody).Options(options).Execute() + data, err := fgaClient.Expand(context.Background()).Body(body).Options(options).Execute()- data, err := fgaClient.ListObjects(context.Background()). - Body(requestBody). + data, err := fgaClient.ListObjects(context.Background()). + Body(body). Options(options). Execute()- data, err := fgaClient.ListRelations(context.Background()). - Body(requestBody). + data, err := fgaClient.ListRelations(context.Background()). + Body(body). Options(options). Execute()Also applies to: 552-555, 584-586
598-606: Fix List Users example to use the correct options type and client callThis section is for List Users but uses
ClientListRelationsOptionsand callsListRelations. UseClientListUsersOptionsandListUsers.Apply this diff:
-options := ClientListRelationsOptions{ +options := ClientListUsersOptions{ // You can rely on the model id set in the configuration or override it for this specific request AuthorizationModelId: {{packageName}}.PtrString("01GAHCE4YVKPQEKZQHT2R89MQV"), // You can rely on the store id set in the configuration or override it for this specific request StoreId: {{packageName}}.PtrString("01FQH7V8BEG3GPQW93KTRFR8JB"), // Max number of requests to issue in parallel, defaults to 10 - MaxParallelRequests: openfga.PtrInt32(5), + MaxParallelRequests: openfga.PtrInt32(5), // if applicable to ListUsers in your SDK }-data, err := fgaClient.ListRelations(context.Background()). +data, err := fgaClient.ListUsers(context.Background()). Body(requestBody). Options(options). Execute()Also applies to: 632-636
config/clients/go/template/client/client.mustache (1)
1717-1725: Bug: wrong chunk size var used for deletes (uses writeChunkSize)While
writeChunkSizecurrently equalsdeleteChunkSize, this is brittle and confusing. UsedeleteChunkSizefor the delete loop.Apply this diff:
- for i := 0; i < len(request.GetBody().Deletes); i += deleteChunkSize { - end := int(math.Min(float64(i+writeChunkSize), float64(len(request.GetBody().Deletes)))) + for i := 0; i < len(request.GetBody().Deletes); i += deleteChunkSize { + end := int(math.Min(float64(i+deleteChunkSize), float64(len(request.GetBody().Deletes))))
🧹 Nitpick comments (3)
config/clients/go/template/client/client_test.mustache (1)
1561-1737: Add tests to exercise headers on chunked Write and BatchCheck pathsGiven the PR’s scope, it’s worth adding assertions that:
- Write non-transaction (chunked) requests carry per-request headers on each chunk.
- ClientBatchCheck and server-side BatchCheck (
/batch-check) forward headers from their respective option structs.I can draft these tests to mirror the Check header assertions if you want.
config/clients/go/template/client/client.mustache (2)
1231-1235: Nit: add ,omitempty to StoreId in ClientReadChangesOptionsTo keep option structs consistent and avoid serializing empty fields (if ever marshaled), add
omitemptytostore_id.- StoreId *string `json:"store_id"` + StoreId *string `json:"store_id,omitempty"`
110-114: Optional: embed ClientRequestOptions into all per-call option structsYou manually duplicated
Headers map[string]stringacross many option types. Consider embeddingClientRequestOptionsin these structs to DRY the code and make future extensions (e.g., timeouts) single-source.Also applies to: 574-578, 649-652, 720-723, 865-869, 957-961, 1045-1048, 1141-1143, 1231-1235, 1345-1350, 1467-1471, 1937-1941, 2058-2063, 2438-2442, 2556-2560, 2679-2684, 2829-2833, 3062-3065
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
config/clients/go/template/README_calling_api.mustache(1 hunks)config/clients/go/template/client/client.mustache(43 hunks)config/clients/go/template/client/client_test.mustache(2 hunks)
🔇 Additional comments (7)
config/clients/go/template/README_calling_api.mustache (1)
388-393: Per-request headers example for Check looks goodThe README now clearly demonstrates how to set per-call headers via the
Headersmap inClientCheckOptions. Matches the new client surface and test coverage.config/clients/go/template/client/client_test.mustache (1)
2043-2050: Good addition: asserts headers are forwarded by CheckThe new
HeadersinClientCheckOptionsand responder-side assertions validate header propagation end-to-end for Check.Also applies to: 2058-2072
config/clients/go/template/client/client.mustache (5)
111-114: Adding Headers to ClientRequestOptions is a solid extension pointCentralizing per-call header injection via
Headers map[string]stringunlocks the desired feature without breaking callers.
600-617: Consistently applying .Options(...) before paging params looks correctFor ListStores, you attach options (with headers) and then page controls. This matches usage elsewhere and should ensure headers are applied.
1610-1621: Header propagation through Write (transaction and chunked) looks correct
- Transaction path:
req.Options(*request.GetOptions())ensures headers are sent.- Chunked path:
chunkOptions.Headers = request.GetOptions().Headerspushes headers into each sub-request.Also applies to: 1685-1692, 1733-1747
2141-2150: ClientBatchCheck: options consolidation is clean and safe for goroutinesCreating a single
checkOptions(including headers and consistency) and reusing it across go routines is fine as it’s immutable after construction.
2314-2331: Server-side BatchCheck: headers/options propagated correctly
singleBatchCheckapplies.Options(*options)beforeBody(body)andExecute(), ensuring headers and other options are sent.
| req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{ | ||
| Name: request.GetBody().Name, | ||
| }) | ||
| if request.GetOptions() != nil { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Guard against nil Body in CreateStoreExecute to prevent panics
request.GetBody() is dereferenced without a nil check. If a user omits Body(...), this will panic. Return a required-parameter error instead.
Apply this diff:
func (client *{{appShortName}}Client) CreateStoreExecute(request SdkClientCreateStoreRequestInterface) (*ClientCreateStoreResponse, error) {
- req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{
- Name: request.GetBody().Name,
- })
+ if request.GetBody() == nil {
+ return nil, FgaRequiredParamError{param: "Body"}
+ }
+ req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{
+ Name: request.GetBody().Name,
+ })
if request.GetOptions() != nil {
req = req.Options(*request.GetOptions())
}🏁 Script executed:
#!/bin/bash
# Show the interface definition for CreateStore request
rg -n "SdkClientCreateStoreRequestInterface" -C3 config/clients/go/template/client/client.mustache
# Show the CreateStoreExecute method context
rg -n -C5 "CreateStoreExecute" config/clients/go/template/client/client.mustacheLength of output: 3796
Prevent runtime panic by validating request body
Add a nil‐check for request.GetBody() before dereferencing its Name field in CreateStoreExecute to return a clear “missing parameter” error instead of panicking.
Affected location:
- config/clients/go/template/client/client.mustache,
CreateStoreExecuteimplementation (around line 681)
Apply this diff:
func (client *{{appShortName}}Client) CreateStoreExecute(request SdkClientCreateStoreRequestInterface) (*ClientCreateStoreResponse, error) {
- req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{
- Name: request.GetBody().Name,
- })
+ if request.GetBody() == nil {
+ return nil, FgaRequiredParamError{param: "Body"}
+ }
+ req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{
+ Name: request.GetBody().Name,
+ })
if request.GetOptions() != nil {
req = req.Options(*request.GetOptions())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{ | |
| Name: request.GetBody().Name, | |
| }) | |
| if request.GetOptions() != nil { | |
| func (client *{{appShortName}}Client) CreateStoreExecute(request SdkClientCreateStoreRequestInterface) (*ClientCreateStoreResponse, error) { | |
| if request.GetBody() == nil { | |
| return nil, FgaRequiredParamError{param: "Body"} | |
| } | |
| req := client.{{appShortName}}Api.CreateStore(request.GetContext()).Body(fgaSdk.CreateStoreRequest{ | |
| Name: request.GetBody().Name, | |
| }) | |
| if request.GetOptions() != nil { | |
| req = req.Options(*request.GetOptions()) | |
| } | |
| // … remainder of method … | |
| } |
🤖 Prompt for AI Agents
In config/clients/go/template/client/client.mustache around lines 682 to 685,
the code dereferences request.GetBody().Name without checking if
request.GetBody() is nil; add a nil-check before accessing Name and return a
clear missing-parameter error (or the client error type used in this codebase)
when body is nil so the function returns a controlled error instead of
panicking. Ensure the check occurs before building the CreateStoreRequest and
that the returned error matches surrounding patterns (HTTP 400 / validation
error) so callers receive a clear message about the missing request body.
|
replaced by #617 |
2 participants
Summary
Testing
go test ./...(fails: directory prefix . does not contain main module or its selected dependencies)make test-client-go(fails: docker: not found)https://chatgpt.com/codex/tasks/task_e_689fa4b9f5248322b0d8808ca53e854c
Summary by CodeRabbit