From df36e4f0cb30aa7aa2adde4d4e9383043db24c2f Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Tue, 18 May 2021 11:35:36 -0400 Subject: [PATCH] internal/jsonrpc2_v2: consider ErrClosedPipe as a closing error 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 Run-TryBot: Robert Findley gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Ian Cottrell --- internal/jsonrpc2/serve.go | 9 +++++++-- internal/jsonrpc2/serve_test.go | 16 ++++++++++------ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/internal/jsonrpc2/serve.go b/internal/jsonrpc2/serve.go index 6a1ecac..ba1db52 100644 --- a/internal/jsonrpc2/serve.go +++ b/internal/jsonrpc2/serve.go @@ -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. diff --git a/internal/jsonrpc2/serve_test.go b/internal/jsonrpc2/serve_test.go index 0041c37..d1c119e 100644 --- a/internal/jsonrpc2/serve_test.go +++ b/internal/jsonrpc2/serve_test.go @@ -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) @@ -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{}, @@ -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 }