Thanks for an excellent stand-alone test case.

FWIW, this behavior changed in 4d2ac54 which was a follow-up to 5dd372b.

A patch like,

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 9164d0d..e715a03 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -1383,7 +1383,9 @@ func (pc *persistConn) readLoop() {
                if err == nil {
                        resp, err = pc.readResponse(rc, trace)
                } else {
-                       err = errServerClosedConn
+                       if err == io.EOF {
+                               err = errServerClosedConn
+                       }
                        closeErr = err
                }

... fixes your test case, but breaks this:

$ go test -v -run=TestRetryIdempotentRequestsOnError -count=5 net/http
=== RUN   TestRetryIdempotentRequestsOnError
--- FAIL: TestRetryIdempotentRequestsOnError (0.00s)
        transport_test.go:2566: Get http://127.0.0.1:50091: read tcp 127.0.0.1:38416->127.0.0.1:50091: read: connection reset by peer
=== RUN   TestRetryIdempotentRequestsOnError
--- PASS: TestRetryIdempotentRequestsOnError (0.00s)
        transport_test.go:2573: finished after 0 runs
=== RUN   TestRetryIdempotentRequestsOnError
--- FAIL: TestRetryIdempotentRequestsOnError (0.01s)
        transport_test.go:2566: Get http://127.0.0.1:35428: read tcp 127.0.0.1:56660->127.0.0.1:35428: read: connection reset by peer
=== RUN   TestRetryIdempotentRequestsOnError
--- PASS: TestRetryIdempotentRequestsOnError (0.00s)
        transport_test.go:2573: finished after 0 runs
=== RUN   TestRetryIdempotentRequestsOnError
--- FAIL: TestRetryIdempotentRequestsOnError (0.00s)
        transport_test.go:2566: Get http://127.0.0.1:34613: read tcp 127.0.0.1:55003->127.0.0.1:34613: read: connection reset by peer
FAIL
exit status 1
FAIL    net/http        0.025s

(Note how it becomes flaky)

What's happening is that the HTTP transport (client code) is interpreting any read error from the server while the client is idle as the HTTP server just getting bored of us and disconnecting. The errors can take various forms: io.EOF, read: connection reset by peer, etc.

I suppose we could write a function to detect more of them, but they also vary by OS (Windows vs Unix, especially) and there's nothing in the net package to help us distinguish.

Maybe there's something safe we can do for Go 1.7. I'll think.

@ianlancetaylor, any thoughts?

It's probably too special-cased, but what about testing if err is a net.Error that is Timeout (maybe Temporary as well). Not clear to me if the original timeout error is making it this far, though.

Along those lines, I had been thinking that a net.IsTimeout(err error) would be nice. Tests both that an error implements Timeout() and that it returns true.

I should also note: In the real code, I do not care about the type of error that is generated. I care more that the request is aborted should the server not respond. I wrote a test (used to derive the test case in this issue) so I could prove to myself that a timeout was happening. I am OK adjusting how I perform that validation.

It's probably too special-cased, but what about testing if err is a net.Error that is Timeout (maybe Temporary as well).

That's approximately what I was thinking.

Not clear to me if the original timeout error is making it this far, though.

It is.

It's kind of weird but for ECONNRESET calling Temporary() will return true (that is so because of #6163). This will be passed up to the net package's Temporary() method. But that is really too ugly.

Perhaps we should add a new Closed() method to the net package errors. That might also help with #4373, maybe.

Looking at this more, but one observation:

Neither https://golang.org/pkg/net/http/#RoundTripper nor https://golang.org/pkg/net/http/#Transport.RoundTrip make any documentation promises about the type of errors returned, and we obviously have never added unit tests locking in a certain behavior for the past 7 releases. Also, I think this code has changed a fair bit for each of those 7 releases, so I wouldn't be surprised if the exactly handling of this type of error has shifted over time. (though your test does seem to pass with Go 1.5 and Go 1.4 too)

I've mailed out https://golang.org/cl/25153 which I'm happy with, but it might be too late for Go 1.7. I'll let @ianlancetaylor and @adg chime in. I'm slightly in favor of fixing it for Go 1.7 considering it would mean keeping the behavior from Go 1.4, Go 1.5, and Go 1.6.

CL https://golang.org/cl/25153 mentions this issue.

@bradfitz happy for this to go in for 1.7.