diff --git a/internal/transport/http2_client.go b/internal/transport/http2_client.go index 0cd6da1e73f7..75586307435a 100644 --- a/internal/transport/http2_client.go +++ b/internal/transport/http2_client.go @@ -616,12 +616,22 @@ func (t *http2Client) getCallAuthData(ctx context.Context, audience string, call return callAuthData, nil } -// NewStreamError wraps an error and reports additional information. +// NewStreamError wraps an error and reports additional information. Typically +// NewStream errors result in transparent retry, as they mean nothing went onto +// the wire. However, there are two notable exceptions: +// +// 1. If the stream headers violate the max header list size allowed by the +// server. In this case there is no reason to retry at all, as it is +// assumed the RPC would continue to fail on subsequent attempts. +// 2. If the credentials errored when requesting their headers. In this case, +// it's possible a retry can fix the problem, but indefinitely transparently +// retrying is not appropriate as it is likely the credentials, if they can +// eventually succeed, would need I/O to do so. type NewStreamError struct { Err error - DoNotRetry bool - PerformedIO bool + DoNotRetry bool + DoNotTransparentRetry bool } func (e NewStreamError) Error() string { @@ -631,24 +641,10 @@ func (e NewStreamError) Error() string { // NewStream creates a stream and registers it into the transport as "active" // streams. All non-nil errors returned will be *NewStreamError. func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Stream, err error) { - defer func() { - if err != nil { - nse, ok := err.(*NewStreamError) - if !ok { - nse = &NewStreamError{Err: err} - } - if len(t.perRPCCreds) > 0 || callHdr.Creds != nil { - // We may have performed I/O in the per-RPC creds callback, so do not - // allow transparent retry. - nse.PerformedIO = true - } - err = nse - } - }() ctx = peer.NewContext(ctx, t.getPeer()) headerFields, err := t.createHeaderFields(ctx, callHdr) if err != nil { - return nil, err + return nil, &NewStreamError{Err: err, DoNotTransparentRetry: true} } s := t.newStream(ctx, callHdr) cleanup := func(err error) { @@ -748,7 +744,7 @@ func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Strea return true }, hdr) if err != nil { - return nil, err + return nil, &NewStreamError{Err: err} } if success { break @@ -759,12 +755,12 @@ func (t *http2Client) NewStream(ctx context.Context, callHdr *CallHdr) (_ *Strea firstTry = false select { case <-ch: - case <-s.ctx.Done(): - return nil, ContextErr(s.ctx.Err()) + case <-ctx.Done(): + return nil, &NewStreamError{Err: ContextErr(ctx.Err())} case <-t.goAway: - return nil, errStreamDrain + return nil, &NewStreamError{Err: errStreamDrain} case <-t.ctx.Done(): - return nil, ErrConnClosing + return nil, &NewStreamError{Err: ErrConnClosing} } } if t.statsHandler != nil { diff --git a/stream.go b/stream.go index 78b7e9e80153..625d47b34e59 100644 --- a/stream.go +++ b/stream.go @@ -547,7 +547,7 @@ func (cs *clientStream) shouldRetry(err error) (bool, error) { // In the event of a non-IO operation error from NewStream, we never // attempted to write anything to the wire, so we can retry // indefinitely. - if !nse.PerformedIO { + if !nse.DoNotTransparentRetry { return true, nil } }