Skip to content

Commit

Permalink
TUN-8297: Improve write timeout logging on safe_stream.go
Browse files Browse the repository at this point in the history
## Summary:
In order to properly monitor what is happening with the new write timeouts that we introduced
in TUN-8244 we need proper logging. Right now we were logging write timeouts when the safe
stream was being closed which didn't make sense because it was miss leading, so this commit
prevents that by adding a flag that allows us to know whether we are closing the stream or not.
  • Loading branch information
jcsf committed Mar 13, 2024
1 parent 47ad323 commit da6fac4
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions quic/safe_stream.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"errors"
"net"
"sync"
"sync/atomic"
"time"

"github.com/quic-go/quic-go"
Expand All @@ -19,6 +20,7 @@ type SafeStreamCloser struct {
stream quic.Stream
writeTimeout time.Duration
log *zerolog.Logger
closing atomic.Bool
}

func NewSafeStreamCloser(stream quic.Stream, writeTimeout time.Duration, log *zerolog.Logger) *SafeStreamCloser {
Expand All @@ -44,27 +46,35 @@ func (s *SafeStreamCloser) Write(p []byte) (n int, err error) {
}
nBytes, err := s.stream.Write(p)
if err != nil {
s.handleTimeout(err)
s.handleWriteError(err)
}

return nBytes, err
}

// Handles the timeout error in case it happened, by canceling the stream write.
func (s *SafeStreamCloser) handleTimeout(err error) {
func (s *SafeStreamCloser) handleWriteError(err error) {
// If we are closing the stream we just ignore any write error.
if s.closing.Load() {
return
}
var netErr net.Error
if errors.As(err, &netErr) {
if netErr.Timeout() {
// We don't need to log if what cause the timeout was `no network activity`.
// We don't need to log if what cause the timeout was no network activity.
if !errors.Is(netErr, &idleTimeoutError) {
s.log.Error().Err(netErr).Msg("Closing quic stream due to timeout while writing")
}
// We need to explicitly cancel the write so that it frees all buffers.
s.stream.CancelWrite(0)
}
}
}

func (s *SafeStreamCloser) Close() error {
// Set this stream to a closing state.
s.closing.Store(true)

// Make sure a possible writer does not block the lock forever. We need it, so we can close the writer
// side of the stream safely.
_ = s.stream.SetWriteDeadline(time.Now())
Expand Down

0 comments on commit da6fac4

Please sign in to comment.