From 65ed3c6f6b3032c21bac2bd2fe25ca679e7c9f05 Mon Sep 17 00:00:00 2001
From: Alain Jobart <alainjobart@google.com>
Date: Wed, 17 Jun 2015 09:31:11 -0700
Subject: [PATCH] Simplifying tablet error to RPC error.

---
 go/vt/tabletserver/grpcqueryservice/server.go | 48 +++++++++----------
 go/vt/tabletserver/tablet_error.go            | 19 ++++----
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/go/vt/tabletserver/grpcqueryservice/server.go b/go/vt/tabletserver/grpcqueryservice/server.go
index 9c8d51d050..7afe931602 100644
--- a/go/vt/tabletserver/grpcqueryservice/server.go
+++ b/go/vt/tabletserver/grpcqueryservice/server.go
@@ -38,11 +38,10 @@ func (q *Query) GetSessionId(ctx context.Context, request *pb.GetSessionIdReques
 		Shard:    request.Shard,
 	}, sessionInfo)
 
-	response = &pb.GetSessionIdResponse{
+	return &pb.GetSessionIdResponse{
 		SessionId: sessionInfo.SessionId,
-	}
-	tabletserver.AddTabletErrorToResult(gsiErr, &response.Error)
-	return response, nil
+		Error:     tabletserver.TabletErrorToRPCError(gsiErr),
+	}, nil
 }
 
 // Execute is part of the queryservice.QueryServer interface
@@ -58,9 +57,9 @@ func (q *Query) Execute(ctx context.Context, request *pb.ExecuteRequest) (respon
 		TransactionId: request.TransactionId,
 	}, reply)
 	if execErr != nil {
-		response := new(pb.ExecuteResponse)
-		tabletserver.AddTabletErrorToResult(execErr, &response.Error)
-		return response, nil
+		return &pb.ExecuteResponse{
+			Error: tabletserver.TabletErrorToRPCError(execErr),
+		}, nil
 	}
 	return &pb.ExecuteResponse{
 		Result: proto.QueryResultToProto3(reply),
@@ -79,9 +78,9 @@ func (q *Query) ExecuteBatch(ctx context.Context, request *pb.ExecuteBatchReques
 		TransactionId: request.TransactionId,
 	}, reply)
 	if execErr != nil {
-		response := new(pb.ExecuteBatchResponse)
-		tabletserver.AddTabletErrorToResult(execErr, &response.Error)
-		return response, nil
+		return &pb.ExecuteBatchResponse{
+			Error: tabletserver.TabletErrorToRPCError(execErr),
+		}, nil
 	}
 	return &pb.ExecuteBatchResponse{
 		Results: proto.QueryResultListToProto3(reply.List),
@@ -103,8 +102,9 @@ func (q *Query) StreamExecute(request *pb.StreamExecuteRequest, stream pbs.Query
 		})
 	})
 	if seErr != nil {
-		response := new(pb.StreamExecuteResponse)
-		tabletserver.AddTabletErrorToResult(seErr, &response.Error)
+		response := &pb.StreamExecuteResponse{
+			Error: tabletserver.TabletErrorToRPCError(seErr),
+		}
 		if err := stream.Send(response); err != nil {
 			return err
 		}
@@ -121,9 +121,9 @@ func (q *Query) Begin(ctx context.Context, request *pb.BeginRequest) (response *
 	if beginErr := q.server.Begin(ctx, &proto.Session{
 		SessionId: request.SessionId,
 	}, txInfo); beginErr != nil {
-		response := new(pb.BeginResponse)
-		tabletserver.AddTabletErrorToResult(beginErr, &response.Error)
-		return response, nil
+		return &pb.BeginResponse{
+			Error: tabletserver.TabletErrorToRPCError(beginErr),
+		}, nil
 	}
 
 	return &pb.BeginResponse{
@@ -140,9 +140,9 @@ func (q *Query) Commit(ctx context.Context, request *pb.CommitRequest) (response
 		SessionId:     request.SessionId,
 		TransactionId: request.TransactionId,
 	})
-	response = new(pb.CommitResponse)
-	tabletserver.AddTabletErrorToResult(commitErr, &response.Error)
-	return response, nil
+	return &pb.CommitResponse{
+		Error: tabletserver.TabletErrorToRPCError(commitErr),
+	}, nil
 }
 
 // Rollback is part of the queryservice.QueryServer interface
@@ -155,9 +155,9 @@ func (q *Query) Rollback(ctx context.Context, request *pb.RollbackRequest) (resp
 		TransactionId: request.TransactionId,
 	})
 
-	response = new(pb.RollbackResponse)
-	tabletserver.AddTabletErrorToResult(rollbackErr, &response.Error)
-	return response, nil
+	return &pb.RollbackResponse{
+		Error: tabletserver.TabletErrorToRPCError(rollbackErr),
+	}, nil
 }
 
 // SplitQuery is part of the queryservice.QueryServer interface
@@ -172,9 +172,9 @@ func (q *Query) SplitQuery(ctx context.Context, request *pb.SplitQueryRequest) (
 		SplitCount:  int(request.SplitCount),
 		SessionID:   request.SessionId,
 	}, reply); sqErr != nil {
-		response = new(pb.SplitQueryResponse)
-		tabletserver.AddTabletErrorToResult(sqErr, &response.Error)
-		return response, nil
+		return &pb.SplitQueryResponse{
+			Error: tabletserver.TabletErrorToRPCError(sqErr),
+		}, nil
 	}
 	return &pb.SplitQueryResponse{
 		Queries: proto.QuerySplitsToProto3(reply.Queries),
diff --git a/go/vt/tabletserver/tablet_error.go b/go/vt/tabletserver/tablet_error.go
index e4e8f2fe95..ddb14a0005 100644
--- a/go/vt/tabletserver/tablet_error.go
+++ b/go/vt/tabletserver/tablet_error.go
@@ -239,15 +239,14 @@ func AddTabletErrorToQueryResult(err error, reply *mproto.QueryResult) {
 	reply.Err = rpcErr
 }
 
-// AddTabletErrorToResult sets the provided pointer to the provided error,
+// TabletErrorToRPCError transforms the provided error to a RPCError,
 // if any.
-func AddTabletErrorToResult(err error, responseError **vtrpc.RPCError) {
+func TabletErrorToRPCError(err error) *vtrpc.RPCError {
 	if err == nil {
-		return
+		return nil
 	}
-	var rpcErr *vtrpc.RPCError
 	if terr, ok := err.(*TabletError); ok {
-		rpcErr = &vtrpc.RPCError{
+		return &vtrpc.RPCError{
 			// Transform TabletError code to VitessError code
 			Code: vtrpc.ErrorCode(int64(terr.ErrorType) + vterrors.TabletError),
 			// Make sure the the VitessError message is identical to the TabletError
@@ -255,11 +254,9 @@ func AddTabletErrorToResult(err error, responseError **vtrpc.RPCError) {
 			// which endpoint they're using.
 			Message: terr.Error(),
 		}
-	} else {
-		rpcErr = &vtrpc.RPCError{
-			Code:    vtrpc.ErrorCode_UnknownTabletError,
-			Message: err.Error(),
-		}
 	}
-	*responseError = rpcErr
+	return &vtrpc.RPCError{
+		Code:    vtrpc.ErrorCode_UnknownTabletError,
+		Message: err.Error(),
+	}
 }