go-sdk icon indicating copy to clipboard operation
go-sdk copied to clipboard

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

Open findleyr opened this issue 3 months ago • 6 comments

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.

findleyr avatar Sep 26 '25 16:09 findleyr

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

manuelibar avatar Sep 30 '25 19:09 manuelibar

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 avatar Oct 01 '25 14:10 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 https://github.com/golang/oauth2/issues/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 avatar Oct 08 '25 18:10 findleyr

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

findleyr avatar Oct 08 '25 21:10 findleyr

https://github.com/golang/go/issues/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?

fgrosse avatar Oct 10 '25 14:10 fgrosse

Hello @findleyr — I’ve just opened https://github.com/modelcontextprotocol/go-sdk/pull/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!

n33levo avatar Oct 10 '25 17:10 n33levo