Proposal: add `ModifyRequest func(*http.Request)` to HTTP client transports

[findleyr]

Following up on #522, where we discussed making it easier to modify HTTP client transport requests, for example by adding headers, we propose the following two new options:

type StreamableClientTransport struct {
  ...
  // If set, ModifyRequest is called before each outgoing HTTP request made by the client
  // connection. It can be use to, for example, add headers to outgoing requests.
  ModifyRequest func(*http.Request)
}

type SSEClientTransport struct {
  ...
  // <ditto>
  ModifyRequest func(*http.Request)
}

We believe these options will make it easier to do things like add auth headers. Currently, this can be achieved using the HTTPClient field, but only in the roundtripper, which requires cloning the request.

[manuelibar]

@findleyr I submitted a PR with proposed feature, please review at your best.
Any feedback is more than welcome!

[findleyr]

Thanks @manuelibar, we will take a look!

We have a policy of waiting at least a week before accepting any proposal, but this seems uncontroversial.

[findleyr]

From the proposal meeting: we actually need more Oauth investigation before committing to this proposal. Specifically, we'd like to avoid adding auth headers in the http.Client.RoundTripper: see golang/oauth2#500. From my superficial understanding, this indicates that we might actually want something more like the Doer interface proposed in #522 (we can still do that if needed, by deprecating+adding).

@jba indicates that this API can still be used in conjunction with http.Client to achieve what we want, but I think we should wait to see a concrete design of that before we add this API.

[findleyr]

See also golang/go#75814, a consequence of these discussions.
@jba and I will pair on an oauth implementation that builds off of the ModifyRequest option.

[fgrosse]

golang/go#75814 seems like a nice pragmatic solution, given that concrete *http.Client implementations are used in many places already 👍

I understand that this proposal will be on hold to see if the Go proposal gets accepted?

[n33levo]

Hello @findleyr —
I’ve just opened #569 . It keeps the client transports’ round-tripper untouched but adds an optional ModifyRequest callback so callers can inject auth headers (echoing the approach discussed in the meeting and #75814).
The PR includes tests that assert the hook runs for every POST/GET/DELETE path, and I verified it with go test ./....
Happy to adjust once you’ve had a chance to look!

[maciej-kisiel]

If I understand correctly this proposal was mostly driven by the need to inject the Authorization header. If so, please take a look at our latest client OAuth proposal at #785.

@n33levo @manuelibar