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

Make read "connection reset" errors retryable. #2926

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions aws/request/connection_reset_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,6 @@ import (
)

func isErrConnectionReset(err error) bool {
if strings.Contains(err.Error(), "read: connection reset") {
return false
}

if strings.Contains(err.Error(), "connection reset") ||
strings.Contains(err.Error(), "broken pipe") {
return true
Expand Down
2 changes: 1 addition & 1 deletion aws/request/connection_reset_error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestSerializationErrConnectionReset_accept(t *testing.T) {
},
"read not temporary": {
Err: errReadConnectionResetStub,
ExpectAttempts: 1,
ExpectAttempts: 6,
},
"write with temporary": {
Err: errWriteConnectionResetStub,
Expand Down
57 changes: 57 additions & 0 deletions aws/request/request_retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,63 @@ func TestShouldRetryError_cancelled(t *testing.T) {
}
}

func TestShouldRetryError_connection_reset(t *testing.T) {
reqWait := make(chan bool)
var ln net.Listener
go func() {
// Start server that listens for one connection, accepts it, and
// then closes it abruptly with a RST.
var err error
ln, err = net.Listen("tcp", "127.0.0.1:0")
if err != nil {
t.Fatalf("Error starting listener: %s", err)
}
defer ln.Close()
close(reqWait)

// Accept one connection.
conn, err := ln.Accept()
if err != nil {
t.Fatalf("Error accepting connection: %s", err)
}

// Force an RST, credit to Jake Pittis:
// https://gist.github.com/jpittis/4357d817dc425ae99fbf719828ab1800
tcpConn := conn.(*net.TCPConn)
tcpConn.SetLinger(0)

// Close the connction to send the RST
err = conn.Close()
if err != nil {
t.Fatalf("Error closing connection: %s", err)
}
}()

// Wait for listener
<-reqWait

// Perform request
tr := &http.Transport{}
defer tr.CloseIdleConnections()
client := http.Client{
Transport: tr,
}
r := newRequest(t, "http://"+ln.Addr().String())
resp, err := client.Do(r)
if resp != nil {
resp.Body.Close()
}
if err == nil {
t.Fatal("This should have failed.")
}

debugerr(t, err)

if shouldRetryError(err) == false {
t.Errorf("this connection was reset by peer and should be retried")
}
}

func TestShouldRetry(t *testing.T) {

syscallError := os.SyscallError{
Expand Down
7 changes: 4 additions & 3 deletions aws/request/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,19 @@ var (
isTemp: true, op: "accept", msg: "connection reset",
}

// net.OpError read for ECONNRESET is not temporary.
// net.OpError read for ECONNRESET may not be temporary, but is treated as
// temporary by the SDK.
errReadConnectionResetStub = &tempNetworkError{
isTemp: false, op: "read", msg: "connection reset",
}

// net.OpError write for ECONNRESET may not be temporary, but is treaded as
// net.OpError write for ECONNRESET may not be temporary, but is treated as
// temporary by the SDK.
errWriteConnectionResetStub = &tempNetworkError{
isTemp: false, op: "write", msg: "connection reset",
}

// net.OpError write for broken pipe may not be temporary, but is treaded as
// net.OpError write for broken pipe may not be temporary, but is treated as
// temporary by the SDK.
errWriteBrokenPipeStub = &tempNetworkError{
isTemp: false, op: "write", msg: "broken pipe",
Expand Down