feat(client): add option to treat nil options pointers as no options
What does this MR do?
The review of !3063, the joint merge request that came out of #2300, asked me to split two of its changes into merge requests of their own, and this is the first: the commit reviewed there as commit 2, cherry-picked unchanged onto v3.15.0, with one commit on top answering the review here. The second is !3066. Like the rest of that work, it comes from maintaining gitlab-mcp-server, an MCP server built on this library.
A nil *Options held in an any is not nil, so NewRequestToURL sends the four bytes null as the body of a POST, PUT or PATCH given one, and replaces a query already on the URL for any other method. CancelJob and PublishAllDraftNotes reach it on their own, through their WithOptions siblings. The client option WithNilOptionsOmitted() makes such a pointer mean no options, as an untyped nil does. Nobody can know whether a caller relies on the null, so, as the earlier review asked, the option is off by default and no request changes unless a caller opts in. #2301 tracks making it the default in 4.0, where the option will have no effect, and removing the option in 5.0.
The change
client_options.go:WithNilOptionsOmitted(), modelled onWithOnlyIdempotentRetries. Its comment says the behavior is expected to become the default in the next major release, where the option will have no effect, and the option to be removed in the one after.gitlab.go: an unexportedomitNilOptionsfield onClient, andisNonNilOptions, which reports whetheroptcarries options, that is, whether it is neither nil nor a nil pointer.NewRequestToURLconsults it only when the option is set, in the body branch and in the query branch alike, so without the option both branches decide exactly as before.- The commit message carries the evidence: the Go FAQ entry on nil values in interfaces, and the two routes the SDK sends
nullto on its own.
gitlab-mcp-server keeps the record of this defect, and of everything else it finds and contributes in the projects it depends on and in sibling projects, in upstream-bugs.md (entry 51). The server works around it by always passing a non-nil options value to the two WithOptions siblings it calls.
Is this a breaking change?
No. The option is off by default, so every request is what it was before, and the tests pin that: TestPublishAllDraftNotes still asserts the null body. The change adds one exported function, WithNilOptionsOmitted, and two unexported names, the omitNilOptions field and the isNonNilOptions helper, and no service interface changes, so no mock is regenerated.
How was this tested?
Tests and lint
The tests pin both sides of the option:
TestNewRequest_NilOptionsPointer_Body: a POST given a nil options pointer sendsnullby default, and no body withWithNilOptionsOmitted().TestNewRequestToURL_NilOptionsPointer_Query: a GET given one clears a query already on the URL by default, and keeps it with the option.TestNewRequestToURL_NilOptionsOmitted_OtherOptions: with the option, an untyped nil still sends no body, and real options are still sent as the body or as the query.TestJobsService_CancelJob_NilOptionsOmittedandTestPublishAllDraftNotes_NilOptionsOmitted: the two methods that delegate with a nil pointer send no body with the option, andTestJobsService_CancelJobWithOptionsnow asserts the body it sends when options are given.
On the branch, with Go 1.27.1:
go test -race ./... ./config/...in workspace mode, andgo test ./...withGOWORK=off, pass.golangci-lint runover both modules with golangci-lint 2.13.2, the version.tool-versionspins, reports 0 issues, andgofumpt -lreports nothing.- No
.protofile and no service interface changes, somake generatehas nothing to regenerate.
Related to #2300