Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
earthboundkid opened this issue Mar 27, 2023 · 1 comment
Open

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

earthboundkid opened this issue Mar 27, 2023 · 1 comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@earthboundkid
Copy link
Contributor

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

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Proposal labels Mar 27, 2023
@seankhliao seankhliao changed the title docs: net/http: clarify relationship of RoundTripper to Client net/http: clarify relationship of RoundTripper to Client Mar 27, 2023
@MukeshGKastala
Copy link

MukeshGKastala commented May 15, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants