Skip to content

Commit

Permalink
internal/jsonrpc2_v2: consider ErrClosedPipe as a closing error
Browse files Browse the repository at this point in the history
There was already handling in isClosingErr for errors from Stdin/Stdout
and TCP connections. Add handling for io.Pipe closing errors.

Change-Id: I8d171ab49a3fffe0fca9f0482f6b92d61b1fae1f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/320849
Trust: Robert Findley <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Reviewed-by: Ian Cottrell <[email protected]>
  • Loading branch information
findleyr committed May 18, 2021
1 parent 6da3d7a commit 17b3466
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
9 changes: 7 additions & 2 deletions internal/jsonrpc2_v2/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,21 @@ func isClosingError(err error) bool {
if err == nil {
return false
}
// fully unwrap the error, so the following tests work
// Fully unwrap the error, so the following tests work.
for wrapped := err; wrapped != nil; wrapped = errors.Unwrap(err) {
err = wrapped
}

// was it based on an EOF error?
// Was it based on an EOF error?
if err == io.EOF {
return true
}

// Was it based on a closed pipe?
if err == io.ErrClosedPipe {
return true
}

// Per https://github.com/golang/go/issues/4373, this error string should not
// change. This is not ideal, but since the worst that could happen here is
// some superfluous logging, it is acceptable.
Expand Down
16 changes: 10 additions & 6 deletions internal/jsonrpc2_v2/serve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,11 @@ func TestServe(t *testing.T) {
if err != nil {
t.Fatal(err)
}
conn, shutdown, err := newFake(ctx, fake)
conn, shutdown, err := newFake(t, ctx, fake)
if err != nil {
t.Fatal(err)
}
defer shutdown(ctx)
defer shutdown()
var got msg
if err := conn.Call(ctx, "ping", &msg{"ting"}).Await(ctx, &got); err != nil {
t.Fatal(err)
Expand All @@ -115,7 +115,7 @@ func TestServe(t *testing.T) {
}
}

func newFake(ctx context.Context, l jsonrpc2.Listener) (*jsonrpc2.Connection, func(context.Context), error) {
func newFake(t *testing.T, ctx context.Context, l jsonrpc2.Listener) (*jsonrpc2.Connection, func(), error) {
l = jsonrpc2.NewIdleListener(100*time.Millisecond, l)
server, err := jsonrpc2.Serve(ctx, l, jsonrpc2.ConnectionOptions{
Handler: fakeHandler{},
Expand All @@ -132,9 +132,13 @@ func newFake(ctx context.Context, l jsonrpc2.Listener) (*jsonrpc2.Connection, fu
if err != nil {
return nil, nil, err
}
return client, func(ctx context.Context) {
l.Close()
client.Close()
return client, func() {
if err := l.Close(); err != nil {
t.Fatal(err)
}
if err := client.Close(); err != nil {
t.Fatal(err)
}
server.Wait()
}, nil
}

0 comments on commit 17b3466

Please sign in to comment.