From 012f31fc2de04bba93bbb37af13a842e66dba16c Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Thu, 17 Sep 2020 01:42:06 +0000 Subject: [PATCH 1/5] Use errors.Is() in IsErrEOF() IsErrEOF returns false when it should return true in a couple of cases: 1. if the error has been wrapped in another error (for example, if EOF is wrapped in an RPC error) 2. if the error has been created from an Error field in an RPC response (as it is the case in CallWithCodec in the net-rpc-msgpackrpc package for example) --- lib/eof.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/eof.go b/lib/eof.go index f77844fd64..75408c8c0d 100644 --- a/lib/eof.go +++ b/lib/eof.go @@ -1,6 +1,7 @@ package lib import ( + "errors" "io" "strings" @@ -13,13 +14,14 @@ var yamuxSessionShutdown = yamux.ErrSessionShutdown.Error() // IsErrEOF returns true if we get an EOF error from the socket itself, or // an EOF equivalent error from yamux. func IsErrEOF(err error) bool { - if err == io.EOF { + if errors.Is(err, io.EOF) { return true } errStr := err.Error() if strings.Contains(errStr, yamuxStreamClosed) || - strings.Contains(errStr, yamuxSessionShutdown) { + strings.Contains(errStr, yamuxSessionShutdown) || + strings.HasSuffix(errStr, io.EOF.Error()) { return true } From 0e64d73f83c8b0fea4c5fb0dbc29c30abda07976 Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Thu, 17 Sep 2020 21:43:04 +0000 Subject: [PATCH 2/5] Add unit tests for isErrEOF() --- lib/eof_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 lib/eof_test.go diff --git a/lib/eof_test.go b/lib/eof_test.go new file mode 100644 index 0000000000..b3810f1c87 --- /dev/null +++ b/lib/eof_test.go @@ -0,0 +1,38 @@ +package lib + +import ( + "fmt" + "io" + "testing" + + "github.com/hashicorp/yamux" + "github.com/stretchr/testify/require" +) + +func TestErrIsEOF(t *testing.T) { + t.Parallel() + + var tests = []struct { + name string + err error + }{ + {name: "EOF", err: io.EOF}, + {name: "yamuxStreamClosed", err: yamux.ErrStreamClosed}, + {name: "yamuxSessionShutdown", err: yamux.ErrSessionShutdown}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + require.True(t, IsErrEOF(tt.err)) + }) + t.Run(fmt.Sprintf("Wrapped %s", tt.name), func(t *testing.T) { + t.Parallel() + require.True(t, IsErrEOF(fmt.Errorf("test: %w", tt.err))) + }) + t.Run(fmt.Sprintf("String suffix is %s", tt.name), func(t *testing.T) { + t.Parallel() + require.True(t, IsErrEOF(fmt.Errorf("rpc error: %s", tt.err.Error()))) + }) + } +} From aa1875c3c705ccd2a06ee40c3a8bf55c26b736a5 Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Fri, 18 Sep 2020 01:16:01 +0000 Subject: [PATCH 3/5] remove t.Parallel() --- lib/eof_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/eof_test.go b/lib/eof_test.go index b3810f1c87..964f9e70e9 100644 --- a/lib/eof_test.go +++ b/lib/eof_test.go @@ -10,8 +10,6 @@ import ( ) func TestErrIsEOF(t *testing.T) { - t.Parallel() - var tests = []struct { name string err error @@ -23,15 +21,12 @@ func TestErrIsEOF(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - t.Parallel() require.True(t, IsErrEOF(tt.err)) }) t.Run(fmt.Sprintf("Wrapped %s", tt.name), func(t *testing.T) { - t.Parallel() require.True(t, IsErrEOF(fmt.Errorf("test: %w", tt.err))) }) t.Run(fmt.Sprintf("String suffix is %s", tt.name), func(t *testing.T) { - t.Parallel() require.True(t, IsErrEOF(fmt.Errorf("rpc error: %s", tt.err.Error()))) }) } From 352cf930fc47cc2cab7c34fd853ebdc9cf0f4b42 Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Sat, 19 Sep 2020 01:59:04 +0000 Subject: [PATCH 4/5] ServerError type check before EOF string comparison --- lib/eof.go | 13 +++++++++++-- lib/eof_test.go | 10 ++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/eof.go b/lib/eof.go index 75408c8c0d..3023a6ce02 100644 --- a/lib/eof.go +++ b/lib/eof.go @@ -2,7 +2,9 @@ package lib import ( "errors" + "fmt" "io" + "net/rpc" "strings" "github.com/hashicorp/yamux" @@ -20,10 +22,17 @@ func IsErrEOF(err error) bool { errStr := err.Error() if strings.Contains(errStr, yamuxStreamClosed) || - strings.Contains(errStr, yamuxSessionShutdown) || - strings.HasSuffix(errStr, io.EOF.Error()) { + strings.Contains(errStr, yamuxSessionShutdown) { return true } + if srvErr, ok := err.(rpc.ServerError); ok { + return strings.HasSuffix(srvErr.Error(), fmt.Sprintf(": %s", io.EOF.Error())) + } + + if srvErr, ok := errors.Unwrap(err).(rpc.ServerError); ok { + return strings.HasSuffix(srvErr.Error(), fmt.Sprintf(": %s", io.EOF.Error())) + } + return false } diff --git a/lib/eof_test.go b/lib/eof_test.go index 964f9e70e9..38106ae997 100644 --- a/lib/eof_test.go +++ b/lib/eof_test.go @@ -3,6 +3,7 @@ package lib import ( "fmt" "io" + "net/rpc" "testing" "github.com/hashicorp/yamux" @@ -15,19 +16,16 @@ func TestErrIsEOF(t *testing.T) { err error }{ {name: "EOF", err: io.EOF}, + {name: "Wrapped EOF", err: fmt.Errorf("test: %w", io.EOF)}, {name: "yamuxStreamClosed", err: yamux.ErrStreamClosed}, {name: "yamuxSessionShutdown", err: yamux.ErrSessionShutdown}, + {name: "ServerError(___: EOF)", err: rpc.ServerError(fmt.Sprintf("rpc error: %s", io.EOF.Error()))}, + {name: "Wrapped ServerError(___: EOF)", err: fmt.Errorf("rpc error: %w", rpc.ServerError(fmt.Sprintf("rpc error: %s", io.EOF.Error())))}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { require.True(t, IsErrEOF(tt.err)) }) - t.Run(fmt.Sprintf("Wrapped %s", tt.name), func(t *testing.T) { - require.True(t, IsErrEOF(fmt.Errorf("test: %w", tt.err))) - }) - t.Run(fmt.Sprintf("String suffix is %s", tt.name), func(t *testing.T) { - require.True(t, IsErrEOF(fmt.Errorf("rpc error: %s", tt.err.Error()))) - }) } } From f85fec6365c5927b32d82a72052cea242ab70e8c Mon Sep 17 00:00:00 2001 From: Pierre Cauchois Date: Thu, 24 Sep 2020 19:23:48 +0000 Subject: [PATCH 5/5] use errors.As() for wrapped ServerError --- lib/eof.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/eof.go b/lib/eof.go index 3023a6ce02..e29d151109 100644 --- a/lib/eof.go +++ b/lib/eof.go @@ -26,12 +26,9 @@ func IsErrEOF(err error) bool { return true } - if srvErr, ok := err.(rpc.ServerError); ok { - return strings.HasSuffix(srvErr.Error(), fmt.Sprintf(": %s", io.EOF.Error())) - } - - if srvErr, ok := errors.Unwrap(err).(rpc.ServerError); ok { - return strings.HasSuffix(srvErr.Error(), fmt.Sprintf(": %s", io.EOF.Error())) + var serverError rpc.ServerError + if errors.As(err, &serverError) { + return strings.HasSuffix(err.Error(), fmt.Sprintf(": %s", io.EOF.Error())) } return false