Merge pull request #3112 from lucas-clemente/fix-timeout-error-logging

fix duplicate logging of errors when the first error was a timeout error
This commit is contained in:
Marten Seemann
2021-04-02 17:24:45 +07:00
committed by GitHub
3 changed files with 25 additions and 24 deletions

View File

@@ -5,6 +5,11 @@ import (
"net"
)
var (
ErrIdleTimeout = newTimeoutError("No recent network activity")
ErrHandshakeTimeout = newTimeoutError("Handshake did not complete in time")
)
// A QuicError consists of an error code plus a error reason
type QuicError struct {
ErrorCode ErrorCode
@@ -33,8 +38,8 @@ func NewErrorWithFrameType(errorCode ErrorCode, frameType uint64, errorMessage s
}
}
// NewTimeoutError creates a new QuicError instance for a timeout error
func NewTimeoutError(errorMessage string) *QuicError {
// newTimeoutError creates a new QuicError instance for a timeout error
func newTimeoutError(errorMessage string) *QuicError {
return &QuicError{
ErrorMessage: errorMessage,
isTimeout: true,

View File

@@ -31,7 +31,7 @@ var _ = Describe("QUIC Transport Errors", func() {
})
It("has a string representation for timeout errors", func() {
err := NewTimeoutError("foobar")
err := newTimeoutError("foobar")
Expect(err.Timeout()).To(BeTrue())
Expect(err.Error()).To(Equal("Timeout: foobar"))
})

View File

@@ -664,19 +664,13 @@ runLoop:
s.framer.QueueControlFrame(&wire.PingFrame{})
s.keepAlivePingSent = true
} else if !s.handshakeComplete && now.Sub(s.sessionCreationTime) >= s.config.handshakeTimeout() {
if s.tracer != nil {
s.tracer.ClosedConnection(logging.NewTimeoutCloseReason(logging.TimeoutReasonHandshake))
}
s.destroyImpl(qerr.NewTimeoutError("Handshake did not complete in time"))
s.destroyImpl(qerr.ErrHandshakeTimeout)
continue
} else {
idleTimeoutStartTime := s.idleTimeoutStartTime()
if (!s.handshakeComplete && now.Sub(idleTimeoutStartTime) >= s.config.HandshakeIdleTimeout) ||
(s.handshakeComplete && now.Sub(idleTimeoutStartTime) >= s.idleTimeout) {
if s.tracer != nil {
s.tracer.ClosedConnection(logging.NewTimeoutCloseReason(logging.TimeoutReasonIdle))
}
s.destroyImpl(qerr.NewTimeoutError("No recent network activity"))
s.destroyImpl(qerr.ErrIdleTimeout)
continue
}
}
@@ -1444,19 +1438,21 @@ func (s *session) handleCloseError(closeErr closeError) {
}
if s.tracer != nil {
// timeout errors are logged as soon as they occur (to distinguish between handshake and idle timeouts)
if nerr, ok := closeErr.err.(net.Error); !ok || !nerr.Timeout() {
var resetErr statelessResetErr
var vnErr errVersionNegotiation
if errors.As(closeErr.err, &resetErr) {
s.tracer.ClosedConnection(logging.NewStatelessResetCloseReason(resetErr.token))
} else if errors.As(closeErr.err, &vnErr) {
s.tracer.ClosedConnection(logging.NewVersionNegotiationError(vnErr.theirVersions))
} else if quicErr.IsApplicationError() {
s.tracer.ClosedConnection(logging.NewApplicationCloseReason(quicErr.ErrorCode, closeErr.remote))
} else {
s.tracer.ClosedConnection(logging.NewTransportCloseReason(quicErr.ErrorCode, closeErr.remote))
}
var resetErr statelessResetErr
var vnErr errVersionNegotiation
switch {
case errors.Is(closeErr.err, qerr.ErrIdleTimeout):
s.tracer.ClosedConnection(logging.NewTimeoutCloseReason(logging.TimeoutReasonIdle))
case errors.Is(closeErr.err, qerr.ErrHandshakeTimeout):
s.tracer.ClosedConnection(logging.NewTimeoutCloseReason(logging.TimeoutReasonHandshake))
case errors.As(closeErr.err, &resetErr):
s.tracer.ClosedConnection(logging.NewStatelessResetCloseReason(resetErr.token))
case errors.As(closeErr.err, &vnErr):
s.tracer.ClosedConnection(logging.NewVersionNegotiationError(vnErr.theirVersions))
case quicErr.IsApplicationError():
s.tracer.ClosedConnection(logging.NewApplicationCloseReason(quicErr.ErrorCode, closeErr.remote))
default:
s.tracer.ClosedConnection(logging.NewTransportCloseReason(quicErr.ErrorCode, closeErr.remote))
}
}