Skip to content

Commit

Permalink
godoc/redirect: close HTTP response bodies in TestRedirect
Browse files Browse the repository at this point in the history
In https://go.dev/issue/50879, we observe ECONNRESET errors from
'dial' in TestRedirect for various paths. That seems to imply that a
unique connection is being dialed for each path — but these
connections are all going through http.DefaultTransport, which has a
30-second keepalive, and the test takes well under that amount of time
to complete.

The only reason we would be dialing a connection per request would be
if the connection itself leaks — and, indeed, inspecting the test in
more detail it fails to close the response body.

I don't know why failing to close the response body would lead to
ECONNRESET errors, but at the very least fixing that issue should
reduce the number of 'dial' operations and thus the number of
platform-specific failure modes.

Fixes golang/go#50879.
(Maybe.)

Change-Id: I5af47ee3683c54106424c66e94b021a0ec7e6d2d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/381736
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
  • Loading branch information
Bryan C. Mills committed Jan 28, 2022
1 parent 0f0bbfd commit 939c2c0
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions godoc/redirect/redirect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func TestRedirects(t *testing.T) {
t.Errorf("(path: %q) unexpected error: %v", path, err)
continue
}
resp.Body.Close() // We only care about the headers, so close the body immediately.

if resp.StatusCode != want.status {
t.Errorf("(path: %q) got status %d, want %d", path, resp.StatusCode, want.status)
Expand Down

0 comments on commit 939c2c0

Please sign in to comment.