Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/e2e/queries: correctly ignore UnexpectedEOF errors #3372

Merged
merged 1 commit into from
Oct 5, 2020
Merged

go/e2e/queries: correctly ignore UnexpectedEOF errors #3372

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/3372.internal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
go/e2e/queries: correctly ignore UnexpectedEOF errors
52 changes: 52 additions & 0 deletions go/common/grpc/errors_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ package grpc

import (
"context"
"io"
"io/ioutil"
"os"
"testing"

"github.com/stretchr/testify/require"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/oasisprotocol/oasis-core/go/common/errors"
)
Expand All @@ -22,6 +25,7 @@ type ErrorTestResponse struct {

type ErrorTestService interface {
ErrorTest(context.Context, *ErrorTestRequest) (*ErrorTestResponse, error)
ErrorStatusTest(context.Context, *ErrorTestRequest) (*ErrorTestResponse, error)
}

type errorTestServer struct {
Expand All @@ -31,6 +35,10 @@ func (s *errorTestServer) ErrorTest(ctx context.Context, req *ErrorTestRequest)
return &ErrorTestResponse{}, errTest
}

func (s *errorTestServer) ErrorStatusTest(ctx context.Context, req *ErrorTestRequest) (*ErrorTestResponse, error) {
return nil, io.ErrUnexpectedEOF
}

type errorTestClient struct {
cc *grpc.ClientConn
}
Expand All @@ -44,6 +52,15 @@ func (c *errorTestClient) ErrorTest(ctx context.Context, req *ErrorTestRequest)
return rsp, nil
}

func (c *errorTestClient) ErrorStatusTest(ctx context.Context, req *ErrorTestRequest) (*ErrorTestResponse, error) {
rsp := new(ErrorTestResponse)
err := c.cc.Invoke(ctx, "/ErrorTestService/ErrorStatusTest", req, rsp)
if err != nil {
return nil, err
}
return rsp, nil
}

var errorTestServiceDesc = grpc.ServiceDesc{
ServiceName: "ErrorTestService",
HandlerType: (*ErrorTestService)(nil),
Expand All @@ -52,6 +69,10 @@ var errorTestServiceDesc = grpc.ServiceDesc{
MethodName: "ErrorTest",
Handler: handlerErrorTest,
},
{
MethodName: "ErrorStatusTest",
Handler: handlerErrorStatusTest,
},
},
Streams: []grpc.StreamDesc{},
}
Expand Down Expand Up @@ -79,6 +100,29 @@ func handlerErrorTest( // nolint: golint
return interceptor(ctx, req, info, handler)
}

func handlerErrorStatusTest( // nolint: golint
srv interface{},
ctx context.Context,
dec func(interface{}) error,
interceptor grpc.UnaryServerInterceptor,
) (interface{}, error) {
req := new(ErrorTestRequest)
if err := dec(req); err != nil {
return nil, err
}
if interceptor == nil {
return srv.(ErrorTestService).ErrorStatusTest(ctx, req)
}
info := &grpc.UnaryServerInfo{
Server: srv,
FullMethod: "/ErrorTestService/ErrorStatusTest",
}
handler := func(ctx context.Context, req interface{}) (interface{}, error) {
return srv.(ErrorTestService).ErrorStatusTest(ctx, req.(*ErrorTestRequest))
}
return interceptor(ctx, req, info, handler)
}

func TestErrorMapping(t *testing.T) {
require := require.New(t)

Expand Down Expand Up @@ -109,4 +153,12 @@ func TestErrorMapping(t *testing.T) {
_, err = client.ErrorTest(context.Background(), &ErrorTestRequest{})
require.Error(err, "ErrorTest should return an error")
require.Equal(err, errTest, "errors should be properly mapped")

_, err = client.ErrorStatusTest(context.Background(), &ErrorTestRequest{})
require.Error(err, "ErrorStatusTest should return an error")
require.True(IsErrorCode(err, codes.Unknown), "ErrorStatusTest should have code unknown")
st := GetErrorStatus(err)
require.NotNil(st, "GetErrorStatus should not be nil")
s, _ := status.FromError(io.ErrUnexpectedEOF)
require.EqualValues(s, st, "GetErrorStatus.Status should be io.ErrUnexpectedEOF")
}
7 changes: 5 additions & 2 deletions go/oasis-node/cmd/debug/txsource/workload/queries.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spf13/viper"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

"github.com/oasisprotocol/oasis-core/go/common"
"github.com/oasisprotocol/oasis-core/go/common/cbor"
Expand Down Expand Up @@ -239,9 +240,11 @@ func (q *queries) doConsensusQueries(ctx context.Context, rng *rand.Rand, height
"txs_with_results", txsWithRes,
"height", height,
"err", err,
"status", cmnGrpc.GetErrorStatus(err),
)
if status := cmnGrpc.GetErrorStatus(err); status != nil {
if status.Err() == io.ErrUnexpectedEOF {
if st := cmnGrpc.GetErrorStatus(err); st != nil {
s, _ := status.FromError(io.ErrUnexpectedEOF)
if st.Err().Error() == s.Err().Error() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh this seems awkward.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, unfortunately i didn't find a nicer way of doing this (the alternative would be directly matching on string - well which is kinda close to what this is doing, but at least here there's no hardcoded strings here :P)

// XXX: Connection seems to get occasionally reset with
// FLOW_CONTROL_ERROR in GetTransactionsWithResult during
// long-term tests, don't fail on this error until we
Expand Down