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 that (*Request).WithContext affects the Response too #26101

mark-rushakoff opened this issue Jun 28, 2018 · 6 comments


Copy link

@mark-rushakoff mark-rushakoff commented Jun 28, 2018

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

Same docs on

What did you do?

I read:

For outgoing client requests, the context controls cancelation.

I thought the context only affected its associated Request, and that the Response returned from Client.Do(Request) would be unaffected by the context.

I searched the net/http Godoc for any mentions of Context relating to a Response but didn't find anything.

Finally, after a long discussion with coworkers, we came up with a manual test case demonstrating that the request context does in fact affect the response:

package main

import (

func main() {
	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		log.Println("serving", r.URL)
		io.WriteString(w, "Hold on...")
		if f, ok := w.(http.Flusher); ok {
		} else {
			log.Println("cannot flush!")
		time.Sleep(10 * time.Second)
		io.WriteString(w, "... okay")
	defer s.Close()

	req, err := http.NewRequest("GET", s.URL, nil)
	if err != nil {
		log.Fatal("Error making request:", err)

	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
	defer cancel()

	req = req.WithContext(ctx)

	resp, err := http.DefaultClient.Do(req)
	if err != nil {
	defer resp.Body.Close()

	log.Println("client made request, starting to read response (this should time out in 1s)")
	body, err := ioutil.ReadAll(resp.Body)
	if err != nil {
		log.Fatal("Error reading response body:", err)
	log.Println("client read response:", string(body))

// Output like:
// 2018/06/27 17:07:06 serving /
// 2018/06/27 17:07:06 client made request, starting to read response (this should time out in 1s)
// 2018/06/27 17:07:07 Error reading response body:context deadline exceeded

We would have saved a lot of time today if the doc said something more like "For outgoing client requests and their corresponding responses, the context controls cancelation."

@agnivade agnivade added this to the Unplanned milestone Jun 28, 2018
Copy link

@meirf meirf commented Jun 28, 2018

It might be worthwhile to be more explicit on the details before deciding on the best documentation to balance conciseness and accuracy.
Expect: The request's context has dominion for the lifetime of http.DefaultClient.Do(req). As soon as Do returns/the initial headers are received, the context no longer applies.
Actual: If the response to a request has a body, the request's context has dominion until the response body is closed. (Notable exclusion being HEADs.)
Is this correct? Would you like to upload your suggestion?

Somewhat related: #23262

Copy link
Contributor Author

@mark-rushakoff mark-rushakoff commented Jun 28, 2018

Is your Actual note how it actually behaves?

I'm not sure of the precise actual behavior, and I'm also unsure of the correct language to use to be brief and accurate regarding the relationship between a context, request, and response; so I'm going to leave it to the Go authors or a more-ambitious-than-me contributor to take on a CL for this doc update.

Copy link

@nhooyr nhooyr commented Aug 13, 2018

Actual: If the response to a request has a body, the request's context has dominion until the response body is closed. (Notable exclusion being HEADs.)

That sounds fine to me to be the new documentation. Re the exclusions, the exclusion is any time there is no body or expected body correct?

Copy link
Contributor Author

@mark-rushakoff mark-rushakoff commented Aug 13, 2018

@bradfitz care to weigh in on the docs change being discussed here?

@bradfitz bradfitz self-assigned this Aug 13, 2018
Copy link

@bradfitz bradfitz commented Aug 13, 2018

Copy link

@gopherbot gopherbot commented Aug 13, 2018

Change mentions this issue: net/http: update request cancelation docs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.