From 9ea0146da6d7003afaaf7f585b9dc60d331f341f Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Mon, 27 Apr 2020 12:22:50 -0400 Subject: [PATCH] internal/jsonrpc2: remove jsonrpc2.ErrDisconnected It was not very useful, it basically renamed io.EOF under some limited circumstances, and was only there for the befit of tests. Instead, the test now checks io.EOF directly, but also checks for io.ErrClosedPipe which was also happening. We also make sure we wrap errors rather than replacing them. This prevents some weird random test failures due to races in the way they were closed. Change-Id: I236b03ac5ba16bf763299b95d882cf58b1f74776 Reviewed-on: https://go-review.googlesource.com/c/tools/+/230303 Run-TryBot: Ian Cottrell Reviewed-by: Robert Findley --- internal/jsonrpc2/jsonrpc2.go | 3 --- internal/jsonrpc2/jsonrpc2_test.go | 2 +- internal/jsonrpc2/stream.go | 9 +-------- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/internal/jsonrpc2/jsonrpc2.go b/internal/jsonrpc2/jsonrpc2.go index 88a1f4910a..c59d4ad56c 100644 --- a/internal/jsonrpc2/jsonrpc2.go +++ b/internal/jsonrpc2/jsonrpc2.go @@ -22,9 +22,6 @@ import ( const ( // ErrIdleTimeout is returned when serving timed out waiting for new connections. ErrIdleTimeout = constError("timed out waiting for new connections") - - // ErrDisconnected signals that the stream or connection exited normally. - ErrDisconnected = constError("disconnected") ) // Conn is a JSON RPC 2 client server connection. diff --git a/internal/jsonrpc2/jsonrpc2_test.go b/internal/jsonrpc2/jsonrpc2_test.go index b7232caa70..d9cc307b5d 100644 --- a/internal/jsonrpc2/jsonrpc2_test.go +++ b/internal/jsonrpc2/jsonrpc2_test.go @@ -126,7 +126,7 @@ func run(ctx context.Context, t *testing.T, withHeaders bool, r io.ReadCloser, w wg.Done() }() err := conn.Run(ctx, testHandler(*logRPC)) - if err != nil && !errors.Is(err, jsonrpc2.ErrDisconnected) { + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrClosedPipe) { t.Errorf("Stream failed: %v", err) } }() diff --git a/internal/jsonrpc2/stream.go b/internal/jsonrpc2/stream.go index dc2ebd4a31..c7d73f7277 100644 --- a/internal/jsonrpc2/stream.go +++ b/internal/jsonrpc2/stream.go @@ -52,9 +52,6 @@ func (s *plainStream) Read(ctx context.Context) (Message, int64, error) { } var raw json.RawMessage if err := s.in.Decode(&raw); err != nil { - if err == io.EOF { - return nil, 0, ErrDisconnected - } return nil, 0, err } msg, err := DecodeMessage(raw) @@ -104,12 +101,8 @@ func (s *headerStream) Read(ctx context.Context) (Message, int64, error) { for { line, err := s.in.ReadString('\n') total += int64(len(line)) - if err == io.EOF { - // A normal disconnection will terminate with EOF before the next header. - return nil, total, ErrDisconnected - } if err != nil { - return nil, total, fmt.Errorf("failed reading header line %q", err) + return nil, total, fmt.Errorf("failed reading header line: %w", err) } line = strings.TrimSpace(line) // check we have a header line