golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.72k stars 17.62k forks source link

net/http: clarify relationship of RoundTripper to Client #59266

Open earthboundkid opened 1 year ago

earthboundkid commented 1 year ago

The docs for http.RoundTripper say

// RoundTrip executes a single HTTP transaction, returning
// a Response for the provided Request.
//
// RoundTrip should not attempt to interpret the response. In
// particular, RoundTrip must return err == nil if it obtained
// a response, regardless of the response's HTTP status code.
// A non-nil err should be reserved for failure to obtain a
// response. Similarly, RoundTrip should not attempt to
// handle higher-level protocol details such as redirects,
// authentication, or cookies.

But Google itself publishes an OAuth RoundTripper that handles authentication via multiple requests, so I don't think this actually reflects current best practices.

The docs for http.Client say, "A Client is higher-level than a RoundTripper (such as Transport) and additionally handles HTTP details such as cookies and redirects." But it's not "such as". It just is cookies and redirects. That's all that Client adds over the RoundTripper now, and I don't think there are any plans to add anything else.

I think the documentation should be clarified to explain that a RoundTripper can do more or less whatever it wants (as long as it doesn't modify a Request or return an error for a valid response), but Client will handle the cookie jar and redirects.

I think the solution is to simplify the docs like this:

RoundTrip executes an HTTP transaction, returning a Response for the provided Request.

RoundTrip should [NB: not "must"] return err == nil if it obtained a response, regardless of the response's HTTP status code. A non-nil err should be reserved for failure to obtain a response. Similarly, RoundTrip should not attempt to handle redirects and cookies.

A Client is higher-level than a RoundTripper (such as Transport) and additionally handles cookies and redirects.

CC: @neild @bradfitz

MukeshGKastala commented 1 year ago

Note: The oauth2 package also returns an error for non-2xx status codes: https://cs.opensource.google/go/x/oauth2/+/refs/tags/v0.8.0:internal/token.go;l=246