From 0e2dfe8a5ba0b35db5e40a9aac210af65fc4c2b5 Mon Sep 17 00:00:00 2001 From: John Behm Date: Sun, 15 Sep 2024 21:43:40 +0200 Subject: [PATCH 1/3] improve error messages that were previously overshadowed by read on closed connection error message due to the connection being closed --- teeworlds7/callbacks.go | 1 - teeworlds7/networking.go | 28 +++++++++++++++------------- teeworlds7/system.go | 16 +++++++++++++--- 3 files changed, 28 insertions(+), 17 deletions(-) diff --git a/teeworlds7/callbacks.go b/teeworlds7/callbacks.go index 34b5b91..c9c6b72 100644 --- a/teeworlds7/callbacks.go +++ b/teeworlds7/callbacks.go @@ -25,7 +25,6 @@ func userMsgCallback[T any](userCallbacks []func(T, DefaultAction) error, msg T, if len(userCallbacks) == 0 { if defaultAction != nil { return defaultAction() - } return nil } diff --git a/teeworlds7/networking.go b/teeworlds7/networking.go index 8aaa4b0..5e79949 100644 --- a/teeworlds7/networking.go +++ b/teeworlds7/networking.go @@ -69,6 +69,19 @@ func (client *Client) Connect(serverIp string, serverPort int) error { func (client *Client) ConnectContext(ctx context.Context, serverIp string, serverPort int) (err error) { ctx, cancelCause := context.WithCancelCause(ctx) + defer cancelCause(nil) // always cancel + + // wait for the reader goroutine to finish execution, before leaving this function scope + var wg sync.WaitGroup + defer wg.Wait() + + ch := make(chan []byte, maxPacksize) + var d net.Dialer + conn, err := d.DialContext(ctx, "udp", fmt.Sprintf("%s:%d", serverIp, serverPort)) + if err != nil { + return fmt.Errorf("failed to connect to server: %s:%d: %w", serverIp, serverPort, err) + } + client.Conn = conn defer func() { // only the first cancelation cause is relevant // subsequent cancelations will be ignored @@ -86,20 +99,9 @@ func (client *Client) ConnectContext(ctx context.Context, serverIp string, serve err = ctxErr return } - }() - // wait for the reader goroutine to finish execution, before leaving this function scope - var wg sync.WaitGroup - defer wg.Wait() - - ch := make(chan []byte, maxPacksize) - var d net.Dialer - conn, err := d.DialContext(ctx, "udp", fmt.Sprintf("%s:%d", serverIp, serverPort)) - if err != nil { - return fmt.Errorf("failed to connect to server: %s:%d: %w", serverIp, serverPort, err) - } - client.Conn = conn - defer func() { + // close connection after error handling in order not to + // hide the actual cause of the error closeErr := client.Conn.Close() if closeErr != nil { slog.Error("failed to close connection", "error", closeErr) diff --git a/teeworlds7/system.go b/teeworlds7/system.go index 51fb385..6550c64 100644 --- a/teeworlds7/system.go +++ b/teeworlds7/system.go @@ -109,7 +109,10 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto client.SnapshotStorage.PurgeUntil(deltaTick) for _, callback := range client.Callbacks.Snapshot { - callback(newFullSnap, nil) + err = callback(newFullSnap, noOpFunc) + if err != nil { + return fmt.Errorf("failed to execute snapshot callback: %w", err) + } } client.Game.Input.AckGameTick = msg.GameTick @@ -151,7 +154,10 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto client.SnapshotStorage.PurgeUntil(deltaTick) for _, callback := range client.Callbacks.Snapshot { - callback(newFullSnap, nil) + err = callback(newFullSnap, noOpFunc) + if err != nil { + return fmt.Errorf("failed to execute snapshot callback: %w", err) + } } client.Game.Input.AckGameTick = msg.GameTick @@ -193,7 +199,7 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto client.SnapshotStorage.PurgeUntil(deltaTick) for _, callback := range client.Callbacks.Snapshot { - err = callback(prevSnap, nil) + err = callback(prevSnap, noOpFunc) if err != nil { return fmt.Errorf("failed to execute snapshot callback: %w", err) } @@ -255,3 +261,7 @@ func (client *Client) processSystem(netMsg messages7.NetMessage, response *proto } return true, nil } + +func noOpFunc() error { + return nil +} From 70192bea07241179d358e177a393e0121f983d83 Mon Sep 17 00:00:00 2001 From: John Behm Date: Sun, 15 Sep 2024 21:45:43 +0200 Subject: [PATCH 2/3] make storage error accessible --- snapshot7/storage.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/snapshot7/storage.go b/snapshot7/storage.go index f6a6f6e..999cd06 100644 --- a/snapshot7/storage.go +++ b/snapshot7/storage.go @@ -17,6 +17,10 @@ const ( UninitializedTick = -1 ) +var ( + ErrNoAltInSnapStorage = errors.New("there is no alt snap in the storage") +) + // TODO: do we even need this? // // can we just put the snap as is in the map? @@ -85,7 +89,7 @@ func (s *Storage) OldestTick() int { func (s *Storage) altSnapshot() (*Snapshot, error) { if s.altSnap.snap == nil { - return nil, errors.New("there is no alt snap in the storage") + return nil, ErrNoAltInSnapStorage } return s.altSnap.snap, nil } From c6b60b38bbb4003ca1eef0a83b4731b86c80c0b5 Mon Sep 17 00:00:00 2001 From: John Behm Date: Sun, 15 Sep 2024 21:46:25 +0200 Subject: [PATCH 3/3] change error name --- snapshot7/storage.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/snapshot7/storage.go b/snapshot7/storage.go index 999cd06..5b33385 100644 --- a/snapshot7/storage.go +++ b/snapshot7/storage.go @@ -18,7 +18,7 @@ const ( ) var ( - ErrNoAltInSnapStorage = errors.New("there is no alt snap in the storage") + ErrNoAltSnapInSnapStorage = errors.New("there is no alt snap in the storage") ) // TODO: do we even need this? @@ -89,7 +89,7 @@ func (s *Storage) OldestTick() int { func (s *Storage) altSnapshot() (*Snapshot, error) { if s.altSnap.snap == nil { - return nil, ErrNoAltInSnapStorage + return nil, ErrNoAltSnapInSnapStorage } return s.altSnap.snap, nil }