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

Keep retrying in addrConn even on non-temporary errors #1856

Merged
merged 2 commits into from
Feb 14, 2018
Merged
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
9 changes: 0 additions & 9 deletions clientconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -1129,15 +1129,6 @@ func (ac *addrConn) createTransport(connectRetryNum, ridx int, backoffDeadline,
newTr, err := transport.NewClientTransport(connectCtx, ac.cc.ctx, target, copts, onPrefaceReceipt)
if err != nil {
cancel()
if e, ok := err.(transport.ConnectionError); ok && !e.Temporary() {
ac.mu.Lock()
if ac.state != connectivity.Shutdown {
ac.state = connectivity.TransientFailure
ac.cc.handleSubConnStateChange(ac.acbw, ac.state)
}
ac.mu.Unlock()
return false, err
}
ac.mu.Lock()
if ac.state == connectivity.Shutdown {
// ac.tearDown(...) has been invoked.
Expand Down
19 changes: 2 additions & 17 deletions transport/http2_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,18 +121,6 @@ func dial(ctx context.Context, fn func(context.Context, string) (net.Conn, error
}

func isTemporary(err error) bool {
switch err {
case io.EOF:
// Connection closures may be resolved upon retry, and are thus
// treated as temporary.
return true
case context.DeadlineExceeded:
// In Go 1.7, context.DeadlineExceeded implements Timeout(), and this
// special case is not needed. Until then, we need to keep this
// clause.
return true
}

switch err := err.(type) {
case interface {
Temporary() bool
Expand All @@ -145,7 +133,7 @@ func isTemporary(err error) bool {
// temporary.
return err.Timeout()
}
return false
return true
}

// newHTTP2Client constructs a connected ClientTransport to addr based on HTTP2
Expand Down Expand Up @@ -181,10 +169,7 @@ func newHTTP2Client(connectCtx, ctx context.Context, addr TargetInfo, opts Conne
scheme = "https"
conn, authInfo, err = creds.ClientHandshake(connectCtx, addr.Authority, conn)
if err != nil {
// Credentials handshake errors are typically considered permanent
// to avoid retrying on e.g. bad certificates.
temp := isTemporary(err)
return nil, connectionErrorf(temp, err, "transport: authentication handshake failed: %v", err)
return nil, connectionErrorf(isTemporary(err), err, "transport: authentication handshake failed: %v", err)
}
isSecure = true
}
Expand Down