Skip to content

net/http: make Transport ignore 408 timeout messages from server [1.12 backport] #32367

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.

Sign up for GitHub

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

Jump to bottom
Closed
gopherbot opened this issue May 31, 2019 · 7 comments
Closed

net/http: make Transport ignore 408 timeout messages from server [1.12 backport] #32367

gopherbot opened this issue May 31, 2019 · 7 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.12.6

Comments

@gopherbot
Copy link
Contributor

@bradfitz requested issue #32310 to be considered for backport to the next 1.12 minor release.

@gopherbot, please backport.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label May 31, 2019
@gopherbot gopherbot mentioned this issue May 31, 2019
Closed
@gopherbot gopherbot added this to the Go1.12.6 milestone May 31, 2019
@bradfitz
Copy link
Contributor

This is an unusual backport rationale: popular HTTP servers on the Internet (at least, all of Google's) changed their behavior, and without this Go logs a warning to stderr, without a way to disable it.

I suspect people will get annoyed by that, so this is somewhat preemptive. I was annoyed when I started seeing it in my logs the other day. I don't know when Google's GFEs started sending 408s, but I suspect it was recently since I started seeing the log messages.

And if we do this, maybe Go 1.11 too (#32366).

/cc @ianlancetaylor @rsc @dmitshur @bcmills

@ianlancetaylor
Copy link
Contributor

I'm fine with backporting this. It may or may not be critical, but there is no workaround.

@bradfitz
Copy link
Contributor

I suppose one workaround is people can do log.SetOutput(filteringWriter{}), but that'll get tedious and gross pretty quickly.

@dmitshur
Copy link
Member

dmitshur commented Jun 7, 2019 •
edited
Loading

I'm going to approve this unusual backport because the workaround is quite complicated and error prone (it requires implementing a filteringWriter and not making a mistake in its implementation, or forking net/http). It's not a very serious issue itself, but it can spam log output and hide other important information from logs, which makes it more serious. I'm also taking into account that Brad and Ian are okay with it.

@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Jun 7, 2019
@dmitshur dmitshur mentioned this issue Jun 7, 2019
Closed
@gopherbot
Copy link
Contributor Author

Change https://golang.org/cl/181239 mentions this issue: [release-branch.go1.12] net/http: prevent Transport from spamming stderr on server 408 reply

@gopherbot
Copy link
Contributor Author

Closed by merging 918368e to release-branch.go1.12.

gopherbot pushed a commit that referenced this issue Jun 7, 2019
@bradfitz @dmitshur
[release-branch.go1.12] net/http: prevent Transport from spamming std…
918368e 
…err on server 408 reply

HTTP 408 responses now exist and are seen in the wild (e.g. from
Google's GFE), so make Go's HTTP client not spam about them when seen.
They're normal (now).

Fixes #32367
Updates #32310

Change-Id: I558eb4654960c74cf20db1902ccaae13d03310f6
Reviewed-on: https://go-review.googlesource.com/c/go/+/179457
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
(cherry picked from commit ba66d89)
Reviewed-on: https://go-review.googlesource.com/c/go/+/181239
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Brad Fitzpatrick <[email protected]>
@andybalholm
Copy link
Contributor

popular HTTP servers on the Internet (at least, all of Google's) changed their behavior, and without this Go logs a warning to stderr, without a way to disable it.

I've been seeing this behavior for months or years. (I don't remember what servers, though.) I never thought of filing an issue for it, but I'm glad to see it fixed.

@golang golang locked and limited conversation to collaborators Jun 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.12.6
Development

No branches or pull requests
5 participants
@bradfitz @andybalholm @dmitshur @ianlancetaylor @gopherbot