Loading
feat(featureflag): guarantee non-nil HTTP transport
Summary
Guarantees that the *http.Client passed to the Flipt provider in v2/featureflag always has a non-nil Transport, while preserving any caller-supplied transport. Prevents panics in downstream callers that wrap or introspect the transport — previously the field was left nil and relied on the implicit stdlib fallback to http.DefaultTransport.
Behaviour
A new internal helper resolveHTTPClient is the single point of responsibility for HTTP client resolution in NewWithConfig:
| Caller input | Resulting client passed to Flipt |
|---|---|
cfg.HTTPClient == nil |
New &http.Client{Timeout: defaultHTTPClientTimeout, Transport: http.DefaultTransport} |
cfg.HTTPClient != nil, Transport != nil |
Used as-is, preserved by identity |
cfg.HTTPClient != nil, Transport == nil |
Shallow-cloned with Transport = http.DefaultTransport. Caller's struct is not mutated. |
The shallow-clone branch (clone := *cfg.HTTPClient) copies Timeout, Jar, and CheckRedirect verbatim and is the reason a caller-supplied client with a nil transport doesn't have its struct mutated as a side effect.
What's in this MR
v2/featureflag/client.go— newresolveHTTPClienthelper;NewWithConfignow calls it;Config.HTTPClientdoc comment documents the guarantee.v2/featureflag/transport_test.go— three unit tests (nil input, explicit transport preserved, nil transport cloned without mutation), inpackage featureflagso they can callresolveHTTPClientdirectly.
Test plan
-
go test ./featureflag/...passes locally (full v2 module also green) -
golangci-lint run ./featureflag/...reports 0 issues - Three new unit tests cover the three input cases enumerated above
Notes for reviewers
- The default transport is
http.DefaultTransport— the same value the stdlib falls back to implicitly. This MR makes the existing fallback explicit; transport tuning (MaxIdleConns,IdleConnTimeout, etc.) is intentionally out of scope. resolveHTTPClientstays unexported. The transport tests are inpackage featureflag(white-box) rather thanpackage featureflag_test, so no export shim is needed.
Edited by Elliot Forbes