From 5e9ab191283a7de615e23afb2fb2c7a73730bb2b Mon Sep 17 00:00:00 2001 From: phil Date: Sun, 17 Nov 2024 18:22:16 -0500 Subject: [PATCH] Close the client connection after a read error The existing code doesn't `defer c.con.Close()` and exits early if reading encounters any error -- so `.Close()` is never called on the connection. This change stores any read and closing errors, `Join`ing them on return, ensuring that `.Close()` is always called. I'm not 100% sure but I hope this will help prevent the half-closed client connections referenced from https://github.com/bluesky-social/jetstream/pull/21 There is probably a more idiomatic way to do this (i am a go noob) but if i understand `errors.Join` and the current code, I think this should keep pretty close to the current intended behavior for any caller. --- pkg/client/client.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 5f2308c..f43f6f6 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -2,6 +2,7 @@ package client import ( "context" + "errors" "fmt" "log/slog" "net/http" @@ -121,15 +122,17 @@ func (c *Client) ConnectAndRead(ctx context.Context, cursor *int64) error { c.con = con + var readErr error = nil if err := c.readLoop(ctx); err != nil { - return fmt.Errorf("read loop failed: %w", err) + readErr = fmt.Errorf("read loop failed: %w", err) } + var closeErr error = nil if err := c.con.Close(); err != nil { - return fmt.Errorf("failed to close connection: %w", err) + closeErr = fmt.Errorf("failed to close connection: %w", err) } - return nil + return errors.Join(readErr, closeErr) } func (c *Client) readLoop(ctx context.Context) error {