Skip to content

Commit

Permalink
commandconn: return original error while closing
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Laura Brehm <[email protected]>
(cherry picked from commit d5f564a)
Signed-off-by: Sebastiaan van Stijn <[email protected]>
  • Loading branch information
laurazard and thaJeztah committed Jun 30, 2023
1 parent 520e360 commit 2d5f041
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
10 changes: 4 additions & 6 deletions cli/connhelper/commandconn/commandconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions cli/connhelper/commandconn/commandconn_unix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package commandconn
import (
"context"
"io"
"io/fs"
"testing"
"time"

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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))
}

0 comments on commit 2d5f041

Please sign in to comment.