What
basecamp.WithHTTPClient(c *http.Client) sets client.httpClient, but NewClient then unconditionally builds its own http.Client (timeout, logging transport, redirect policy) and assigns it over the option's value. The option is dead: a caller's TLS config, proxy, cookie jar or timeout on that client is silently discarded. authorization_test.go passes the option and passes anyway, which is how it went unnoticed.
Why it matters
Every documented example (go/README.md, doc.go) that passes WithHTTPClient promises behaviour the SDK does not deliver. WithTransport and WithTimeout do work, so the realistic remedy is one of:
- honour the custom client: compose the SDK's logging transport, the SPEC §13 redirect policy and any
WithTransportWrapper over the custom client's Transport (nil meaning http.DefaultTransport), keep its other fields; or
- retire the option (deprecate, then remove) and point callers at
WithTransport/WithTimeout.
Whichever lands must keep the layering NewClient guarantees today — logging transport outermost, then any wrapper, then the caller's transport — because the SPEC §23 event-feed adapters rely on WithTransportWrapper seeing every wire response.
Surfaced by review on #899 (eventfeed.NewLive forwards client options unchanged, so it inherits the gap).
What
basecamp.WithHTTPClient(c *http.Client)setsclient.httpClient, butNewClientthen unconditionally builds its ownhttp.Client(timeout, logging transport, redirect policy) and assigns it over the option's value. The option is dead: a caller's TLS config, proxy, cookie jar or timeout on that client is silently discarded.authorization_test.gopasses the option and passes anyway, which is how it went unnoticed.Why it matters
Every documented example (
go/README.md,doc.go) that passesWithHTTPClientpromises behaviour the SDK does not deliver.WithTransportandWithTimeoutdo work, so the realistic remedy is one of:WithTransportWrapperover the custom client'sTransport(nil meaninghttp.DefaultTransport), keep its other fields; orWithTransport/WithTimeout.Whichever lands must keep the layering
NewClientguarantees today — logging transport outermost, then any wrapper, then the caller's transport — because the SPEC §23 event-feed adapters rely onWithTransportWrapperseeing every wire response.Surfaced by review on #899 (
eventfeed.NewLiveforwards client options unchanged, so it inherits the gap).