From 2d5f041bde4edae1a9fb42b0c072c83e8d23c9d8 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 (cherry picked from commit d5f564adaa7aff4868af5155e288ba71c3ebd1ba) Signed-off-by: Sebastiaan van Stijn --- 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 a01c27bdc3..835c4c48df 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 bc43ea2318..c51974732b 100644 --- a/cli/connhelper/commandconn/commandconn_unix_test.go +++ b/cli/connhelper/commandconn/commandconn_unix_test.go @@ -6,6 +6,7 @@ package commandconn import ( "context" "io" + "io/fs" "testing" "time" @@ -179,7 +180,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) { @@ -209,5 +211,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)) }