Merge pull request #26 from coder/spike/proc-close

spikecurtis · web-flow · commit ce30d9aec7af · 2022-11-02T08:40:10.000-07:00
Fix proc.Close() when there are no errors
diff --git a/client.go b/client.go
@@ -266,13 +266,18 @@ func (r remoteProcess) Close() error {
 
 func joinErrs(errs ...error) error {
 	var str string
+	foundErr := false
 	for _, e := range errs {
 		if e != nil {
+			foundErr = true
 			if str != "" {
 				str += ", "
 			}
 			str += e.Error()
 		}
 	}
-	return xerrors.New(str)
+	if foundErr {
+		return xerrors.New(str)
+	}
+	return nil
 }
diff --git a/client_test.go b/client_test.go
@@ -15,6 +15,7 @@ import (
 	"cdr.dev/slog/sloggers/slogtest/assert"
 	"cdr.dev/wsep/internal/proto"
 	"github.com/google/go-cmp/cmp"
+	"golang.org/x/xerrors"
 	"nhooyr.io/websocket"
 )
 
@@ -84,6 +85,46 @@ func TestRemoteExec(t *testing.T) {
 	testExecer(ctx, t, execer)
 }
 
+func TestRemoveClose(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+
+	ws, server := mockConn(ctx, t, nil)
+	defer server.Close()
+
+	execer := RemoteExecer(ws)
+	cmd := Command{
+		Command: "/bin/bash",
+		TTY:     true,
+		Stdin:   true,
+		Env:     []string{"TERM=linux"},
+	}
+
+	proc, err := execer.Start(ctx, cmd)
+	assert.Success(t, "execer Start", err)
+
+	i := proc.Stdin()
+	o := proc.Stdout()
+	buf := make([]byte, 2048)
+	_, err = i.Write([]byte("echo foo"))
+	assert.Success(t, "echo", err)
+	bldr := strings.Builder{}
+	for !strings.Contains(bldr.String(), "foo") {
+		n, err := o.Read(buf)
+		if xerrors.Is(err, io.EOF) {
+			break
+		}
+		assert.Success(t, "read", err)
+		_, err = bldr.Write(buf[:n])
+		assert.Success(t, "write to builder", err)
+	}
+	err = proc.Close()
+	assert.Success(t, "close proc", err)
+	// note that proc.Close() also closes the websocket.
+}
+
 func TestRemoteExecFail(t *testing.T) {
 	t.Parallel()
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)

Original file line number	Diff line number	Diff line change
`@@ -266,13 +266,18 @@ func (r remoteProcess) Close() error {`
`266`	`266`
`267`	`267`	`func joinErrs(errs ...error) error {`
`268`	`268`	`var str string`
	`269`	`+ foundErr := false`
`269`	`270`	`for _, e := range errs {`
`270`	`271`	`if e != nil {`
	`272`	`+ foundErr = true`
`271`	`273`	`if str != "" {`
`272`	`274`	`str += ", "`
`273`	`275`	`}`
`274`	`276`	`str += e.Error()`
`275`	`277`	`}`
`276`	`278`	`}`
`277`		`- return xerrors.New(str)`
	`279`	`+ if foundErr {`
	`280`	`+ return xerrors.New(str)`
	`281`	`+ }`
	`282`	`+ return nil`
`278`	`283`	`}`