Skip to content

Commit

Permalink
conn.go: Only log errors from writing to a connection that aren't net…
Browse files Browse the repository at this point in the history
….ErrClosed instead of returning.

Other errors are recoverable through resending and don't need to be propagated into error chains.
  • Loading branch information
Sandertv committed Jun 21, 2024
1 parent e4fd2fe commit 5942afc
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 18 deletions.
25 changes: 19 additions & 6 deletions conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
}

Expand Down
18 changes: 8 additions & 10 deletions dial.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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())
}
}
Expand Down
12 changes: 11 additions & 1 deletion handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"github.com/sandertv/go-raknet/internal/message"
"hash/crc32"
"log/slog"
"net"
"time"
)
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -185,14 +191,18 @@ 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")
errUnexpectedAdditionalCRA = errors.New("unexpected additional CONNECTION_REQUEST_ACCEPTED packet")
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()
}
Expand Down
3 changes: 2 additions & 1 deletion listener.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package raknet

import (
"errors"
"fmt"
"github.com/sandertv/go-raknet/internal"
"log/slog"
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit 5942afc

Please sign in to comment.