From d5f564adaa7aff4868af5155e288ba71c3ebd1ba Mon Sep 17 00:00:00 2001 From: Laura Brehm Date: Fri, 30 Jun 2023 15:03:19 +0100 Subject: [PATCH] commandconn: return original error while closing Changes the `Read` and `Write` error handling logic to return the original error while closing the connection. We still skip calling `handleEOF` if already closing the connection. Fixes the flaky `TestCloseWhileWriting` and `TestCloseWhileReading` tests. Co-authored-by: Sebastiaan van Stijn Signed-off-by: Laura Brehm --- cli/connhelper/commandconn/commandconn.go | 10 ++++------ cli/connhelper/commandconn/commandconn_unix_test.go | 6 ++++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/connhelper/commandconn/commandconn.go b/cli/connhelper/commandconn/commandconn.go index 2643ca30db..f697370ed9 100644 --- a/cli/connhelper/commandconn/commandconn.go +++ b/cli/connhelper/commandconn/commandconn.go @@ -163,9 +163,8 @@ func (c *commandConn) Read(p []byte) (int, error) { // Close might get called if c.closing.Load() { // If we're currently closing the connection - // we don't want to call onEOF, but we do want - // to return an io.EOF - return 0, io.EOF + // we don't want to call onEOF + return n, err } return n, c.handleEOF(err) @@ -178,9 +177,8 @@ func (c *commandConn) Write(p []byte) (int, error) { // Close might get called if c.closing.Load() { // If we're currently closing the connection - // we don't want to call onEOF, but we do want - // to return an io.EOF - return 0, io.EOF + // we don't want to call onEOF + return n, err } return n, c.handleEOF(err) diff --git a/cli/connhelper/commandconn/commandconn_unix_test.go b/cli/connhelper/commandconn/commandconn_unix_test.go index cc96e344ea..03bc469364 100644 --- a/cli/connhelper/commandconn/commandconn_unix_test.go +++ b/cli/connhelper/commandconn/commandconn_unix_test.go @@ -5,6 +5,7 @@ package commandconn import ( "context" "io" + "io/fs" "testing" "time" @@ -178,7 +179,8 @@ func TestCloseWhileWriting(t *testing.T) { assert.Check(t, !process.Alive(cmdConn.cmd.Process.Pid)) writeErr := <-writeErrC - assert.ErrorContains(t, writeErr, "EOF") + assert.ErrorContains(t, writeErr, "file already closed") + assert.Check(t, is.ErrorIs(writeErr, fs.ErrClosed)) } func TestCloseWhileReading(t *testing.T) { @@ -208,5 +210,5 @@ func TestCloseWhileReading(t *testing.T) { assert.Check(t, !process.Alive(cmdConn.cmd.Process.Pid)) readErr := <-readErrC - assert.ErrorContains(t, readErr, "EOF") + assert.Check(t, is.ErrorIs(readErr, fs.ErrClosed)) }