diff --git a/go/mysql/proto/rpcerror_bson.go b/go/mysql/proto/rpcerror_bson.go index 4ecbdfd8a0..1cb1a9a05a 100644 --- a/go/mysql/proto/rpcerror_bson.go +++ b/go/mysql/proto/rpcerror_bson.go @@ -19,7 +19,7 @@ func (rPCError *RPCError) MarshalBson(buf *bytes2.ChunkedWriter, key string) { bson.EncodeOptionalPrefix(buf, bson.Object, key) lenWriter := bson.NewLenWriter(buf) - bson.EncodeInt(buf, "Code", rPCError.Code) + bson.EncodeInt64(buf, "Code", rPCError.Code) bson.EncodeString(buf, "Message", rPCError.Message) lenWriter.Close() @@ -40,7 +40,7 @@ func (rPCError *RPCError) UnmarshalBson(buf *bytes.Buffer, kind byte) { for kind := bson.NextByte(buf); kind != bson.EOO; kind = bson.NextByte(buf) { switch bson.ReadCString(buf) { case "Code": - rPCError.Code = bson.DecodeInt(buf, kind) + rPCError.Code = bson.DecodeInt64(buf, kind) case "Message": rPCError.Message = bson.DecodeString(buf, kind) default: diff --git a/go/mysql/proto/structs.go b/go/mysql/proto/structs.go index c073b69c28..efa18f571a 100644 --- a/go/mysql/proto/structs.go +++ b/go/mysql/proto/structs.go @@ -91,7 +91,7 @@ type QueryResult struct { // RPCError is the structure that is returned by each RPC call, which contains // the error information for that call. type RPCError struct { - Code int + Code int64 Message string } diff --git a/go/vt/tabletserver/gorpcqueryservice/sqlquery.go b/go/vt/tabletserver/gorpcqueryservice/sqlquery.go index c8b1be9726..e0cc65c5ef 100644 --- a/go/vt/tabletserver/gorpcqueryservice/sqlquery.go +++ b/go/vt/tabletserver/gorpcqueryservice/sqlquery.go @@ -5,6 +5,8 @@ package gorpcqueryservice import ( + "flag" + mproto "github.com/youtube/vitess/go/mysql/proto" "github.com/youtube/vitess/go/vt/callinfo" "github.com/youtube/vitess/go/vt/servenv" @@ -14,6 +16,10 @@ import ( "golang.org/x/net/context" ) +var ( + rpcErrorOnlyInReply = flag.Bool("rpc-error-only-in-reply", false, "if true, supported RPC calls will only return errors as part of the RPC server response") +) + // SqlQuery is the server object for gorpc SqlQuery type SqlQuery struct { server queryservice.QueryService @@ -48,7 +54,10 @@ func (sq *SqlQuery) Execute(ctx context.Context, query *proto.Query, reply *mpro defer sq.server.HandlePanic(&err) execErr := sq.server.Execute(callinfo.RPCWrapCallInfo(ctx), query, reply) tabletserver.AddTabletErrorToQueryResult(execErr, reply) - return nil + if *rpcErrorOnlyInReply { + return nil + } + return execErr } // StreamExecute is exposing tabletserver.SqlQuery.StreamExecute diff --git a/go/vt/tabletserver/tablet_error.go b/go/vt/tabletserver/tablet_error.go index a704de2772..7c21d1b34d 100644 --- a/go/vt/tabletserver/tablet_error.go +++ b/go/vt/tabletserver/tablet_error.go @@ -223,7 +223,7 @@ func AddTabletErrorToQueryResult(err error, reply *mproto.QueryResult) { if ok { rpcErr = mproto.RPCError{ // Transform TabletError code to VitessError code - Code: terr.ErrorType + vterrors.TabletError, + Code: int64(terr.ErrorType) + vterrors.TabletError, // Make sure the the VitessError message is identical to the TabletError // err, so that downstream consumers will see identical messages no matter // which endpoint they're using. diff --git a/go/vt/tabletserver/tabletconntest/tabletconntest.go b/go/vt/tabletserver/tabletconntest/tabletconntest.go index f5228ae052..972691fab2 100644 --- a/go/vt/tabletserver/tabletconntest/tabletconntest.go +++ b/go/vt/tabletserver/tabletconntest/tabletconntest.go @@ -22,6 +22,7 @@ import ( // FakeQueryService has the server side of this fake type FakeQueryService struct { t *testing.T + hasError bool panics bool streamExecutePanicsEarly bool } @@ -170,7 +171,11 @@ func (f *FakeQueryService) Execute(ctx context.Context, query *proto.Query, repl if query.TransactionId != executeTransactionId { f.t.Errorf("invalid Execute.Query.TransactionId: got %v expected %v", query.TransactionId, executeTransactionId) } - *reply = executeQueryResult + if f.hasError { + *reply = executeQueryResultError + } else { + *reply = executeQueryResult + } return nil } @@ -207,6 +212,13 @@ var executeQueryResult = mproto.QueryResult{ }, } +var executeQueryResultError = mproto.QueryResult{ + Err: mproto.RPCError{ + Code: 1000, + Message: "succeeded despite err", + }, +} + func testExecute(t *testing.T, conn tabletconn.TabletConn) { t.Log("testExecute") ctx := context.Background() @@ -219,6 +231,19 @@ func testExecute(t *testing.T, conn tabletconn.TabletConn) { } } +func testExecuteError(t *testing.T, conn tabletconn.TabletConn) { + t.Log("testExecuteError") + ctx := context.Background() + _, err := conn.Execute(ctx, executeQuery, executeBindVars, executeTransactionId) + if err == nil { + t.Fatalf("Execute was expecting an error, didn't get one") + } + expectedErr := "vttablet: succeeded despite err" + if err.Error() != expectedErr { + t.Errorf("Unexpected error from Execute: got %v wanted %v", err, expectedErr) + } +} + func testExecutePanics(t *testing.T, conn tabletconn.TabletConn) { ctx := context.Background() if _, err := conn.Execute(ctx, executeQuery, executeBindVars, executeTransactionId); err == nil || !strings.Contains(err.Error(), "caught test panic") { @@ -531,6 +556,11 @@ func TestSuite(t *testing.T, conn tabletconn.TabletConn, fake *FakeQueryService) testExecuteBatch(t, conn) testSplitQuery(t, conn) + // fake should return an error, make sure errors are handled properly + fake.hasError = true + testExecuteError(t, conn) + fake.hasError = false + // force panics, make sure they're caught fake.panics = true testBeginPanics(t, conn) diff --git a/go/vt/vterrors/vterrors.go b/go/vt/vterrors/vterrors.go index 3de7d376b7..16b07f7637 100644 --- a/go/vt/vterrors/vterrors.go +++ b/go/vt/vterrors/vterrors.go @@ -26,7 +26,7 @@ const ( // VitessError is the error type that we use internally for passing structured errors type VitessError struct { // Error code of the Vitess error - Code int + Code int64 // Additional context string, distinct from the error message. For example, if // you wanted an error like "foo error: original error", the Message string // should be "foo error: " @@ -53,7 +53,6 @@ func (e *VitessError) AsString() string { // FromRPCError recovers a VitessError from an RPCError (which is how VitessErrors // are transmitted across RPC boundaries). func FromRPCError(rpcErr mproto.RPCError) error { - fmt.Printf("RPCError: %v\n", rpcErr) if rpcErr.Code == 0 && rpcErr.Message == "" { return nil } @@ -63,8 +62,8 @@ func FromRPCError(rpcErr mproto.RPCError) error { } } -// AddPrefix allows a string to be prefixed to an error, without nesting a new VitessError. -func AddPrefix(prefix string, in error) error { +// WithPrefix allows a string to be prefixed to an error, without nesting a new VitessError. +func WithPrefix(prefix string, in error) error { vtErr, ok := in.(*VitessError) if !ok { return fmt.Errorf("%s: %s", prefix, in) diff --git a/test/tablet.py b/test/tablet.py index c06ec07612..51aa3ac900 100644 --- a/test/tablet.py +++ b/test/tablet.py @@ -484,6 +484,8 @@ class Tablet(object): if supports_backups: args.extend(['-restore_from_backup'] + get_backup_storage_flags()) + args.extend(['-rpc-error-only-in-reply=true']) + if extra_args: args.extend(extra_args)