From 5942afc785815d364bf0f2692dc3c8d779020ae7 Mon Sep 17 00:00:00 2001 From: Sandertv Date: Fri, 21 Jun 2024 22:01:34 +0200 Subject: [PATCH] conn.go: Only log errors from writing to a connection that aren't net.ErrClosed instead of returning. Other errors are recoverable through resending and don't need to be propagated into error chains. --- conn.go | 25 +++++++++++++++++++------ dial.go | 18 ++++++++---------- handler.go | 12 +++++++++++- listener.go | 3 ++- 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/conn.go b/conn.go index 55ef6c7..e867aec 100644 --- a/conn.go +++ b/conn.go @@ -537,7 +537,7 @@ func (conn *Conn) sendAcknowledgement(packets []uint24, bitflag byte, buf *bytes // We managed to write n packets in the ACK with this MTU size, write // the next of the packets in a new ACK. ack.packets = ack.packets[n:] - if _, err := conn.conn.WriteTo(buf.Bytes(), conn.raddr); err != nil { + if err := conn.writeTo(buf.Bytes(), conn.raddr); err != nil { return fmt.Errorf("send acknowlegement: %w", err) } buf.Reset() @@ -603,15 +603,28 @@ func (conn *Conn) sendDatagram(pk *packet) error { seq := conn.seq.Inc() writeUint24(conn.buf, seq) pk.write(conn.buf) + defer conn.buf.Reset() - // We then send the pk to the connection. - if _, err := conn.conn.WriteTo(conn.buf.Bytes(), conn.raddr); err != nil { - return fmt.Errorf("send datagram: write to %v: %w", conn.raddr, err) - } // We then re-add the pk to the recovery queue in case the new one gets // lost too, in which case we need to resend it again. conn.retransmission.add(seq, pk) - conn.buf.Reset() + + if err := conn.writeTo(conn.buf.Bytes(), conn.raddr); err != nil { + return fmt.Errorf("send datagram: %w", err) + } + return nil +} + +// writeTo calls WriteTo on the underlying UDP connection and returns an error +// only if the error returned is net.ErrClosed. In any other case, the error +// is logged but not returned. This is done because at this stage, packets +// being lost to an error can be recovered through resending. +func (conn *Conn) writeTo(p []byte, raddr net.Addr) error { + if _, err := conn.conn.WriteTo(p, raddr); errors.Is(err, net.ErrClosed) { + return fmt.Errorf("write to: %w", err) + } else if err != nil { + conn.handler.log().Error("write to: " + err.Error()) + } return nil } diff --git a/dial.go b/dial.go index 29e67e6..6e16d42 100644 --- a/dial.go +++ b/dial.go @@ -233,7 +233,7 @@ func (dialer Dialer) DialContext(ctx context.Context, address string) (*Conn, er // dial finishes the RakNet connection sequence and returns a Conn if // successful. func (dialer Dialer) connect(ctx context.Context, state *connState) (*Conn, error) { - conn := newConn(internal.ConnToPacketConn(state.conn), state.raddr, state.mtu, dialerConnectionHandler{}) + conn := newConn(internal.ConnToPacketConn(state.conn), state.raddr, state.mtu, dialerConnectionHandler{l: dialer.ErrorLog}) if err := conn.send((&message.ConnectionRequest{ClientGUID: state.id, RequestTime: timestamp()})); err != nil { return nil, dialer.error("dial", fmt.Errorf("send connection request: %w", err)) } @@ -259,17 +259,15 @@ func (dialer Dialer) clientListen(rakConn *Conn, conn net.Conn) { b := make([]byte, rakConn.effectiveMTU()) for { n, err := conn.Read(b) + if err == nil && n != 0 { + err = rakConn.receive(b[:n]) + } if err != nil { - if !errors.Is(err, net.ErrClosed) { - // Errors reading a packet other than the connection being - // may be worth logging. - dialer.ErrorLog.Error("client: read packet: " + err.Error()) + if errors.Is(err, net.ErrClosed) { + return } - return - } else if n == 0 { - continue - } - if err := rakConn.receive(b[:n]); err != nil { + // Errors reading a packet other than the connection being + // closed may be worth logging. dialer.ErrorLog.Error("client: " + err.Error()) } } diff --git a/handler.go b/handler.go index a79f22e..261d20c 100644 --- a/handler.go +++ b/handler.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/sandertv/go-raknet/internal/message" "hash/crc32" + "log/slog" "net" "time" ) @@ -14,6 +15,7 @@ type connectionHandler interface { handle(conn *Conn, b []byte) (handled bool, err error) limitsEnabled() bool close(conn *Conn) + log() *slog.Logger } type listenerConnectionHandler struct { @@ -26,6 +28,10 @@ var ( errUnexpectedAdditionalNIC = errors.New("unexpected additional NEW_INCOMING_CONNECTION packet") ) +func (h listenerConnectionHandler) log() *slog.Logger { + return h.l.conf.ErrorLog +} + func (h listenerConnectionHandler) limitsEnabled() bool { return true } @@ -185,7 +191,7 @@ func (h listenerConnectionHandler) handleNewIncomingConnection(conn *Conn) error return nil } -type dialerConnectionHandler struct{} +type dialerConnectionHandler struct{ l *slog.Logger } var ( errUnexpectedCR = errors.New("unexpected CONNECTION_REQUEST packet") @@ -193,6 +199,10 @@ var ( errUnexpectedNIC = errors.New("unexpected NEW_INCOMING_CONNECTION packet") ) +func (h dialerConnectionHandler) log() *slog.Logger { + return h.l +} + func (h dialerConnectionHandler) close(conn *Conn) { _ = conn.conn.Close() } diff --git a/listener.go b/listener.go index a40a372..6b743ad 100644 --- a/listener.go +++ b/listener.go @@ -1,6 +1,7 @@ package raknet import ( + "errors" "fmt" "github.com/sandertv/go-raknet/internal" "log/slog" @@ -182,7 +183,7 @@ func (listener *Listener) listen() { } else if n == 0 || listener.sec.blocked(addr) { continue } - if err = listener.handle(b[:n], addr); err != nil { + if err = listener.handle(b[:n], addr); err != nil && !errors.Is(err, net.ErrClosed) { listener.conf.ErrorLog.Error("listener: handle packet: "+err.Error(), "address", addr.String(), "block-duration", max(0, listener.conf.BlockDuration)) listener.sec.block(addr) }