From 3b7e2ce0ca2986011c4f96d94bb11e8c412f7592 Mon Sep 17 00:00:00 2001
From: "Sahdev P. Zala" <spzala@us.ibm.com>
Date: Mon, 8 Jan 2018 20:14:54 -0500
Subject: [PATCH] StatusFromError: handle return value of the function

status.FromError can return nil, false. We are handling the return values
most places in code but some places we aren't. Fixing it herewith.

Fixes #9117
---
 clientv3/client.go                            | 23 ++++++++++---------
 clientv3/integration/server_shutdown_test.go  |  5 +++-
 clientv3/leasing/kv.go                        |  2 +-
 clientv3/retry.go                             |  8 ++++---
 etcdserver/api/v3rpc/rpctypes/error_test.go   |  6 ++---
 .../command/lease_renewer_command.go          |  3 +--
 6 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/clientv3/client.go b/clientv3/client.go
index a2c313d35..685401084 100644
--- a/clientv3/client.go
+++ b/clientv3/client.go
@@ -537,18 +537,19 @@ func toErr(ctx context.Context, err error) error {
 	if _, ok := err.(rpctypes.EtcdError); ok {
 		return err
 	}
-	ev, _ := status.FromError(err)
-	code := ev.Code()
-	switch code {
-	case codes.DeadlineExceeded:
-		fallthrough
-	case codes.Canceled:
-		if ctx.Err() != nil {
-			err = ctx.Err()
+	if ev, ok := status.FromError(err); ok {
+		code := ev.Code()
+		switch code {
+		case codes.DeadlineExceeded:
+			fallthrough
+		case codes.Canceled:
+			if ctx.Err() != nil {
+				err = ctx.Err()
+			}
+		case codes.Unavailable:
+		case codes.FailedPrecondition:
+			err = grpc.ErrClientConnClosing
 		}
-	case codes.Unavailable:
-	case codes.FailedPrecondition:
-		err = grpc.ErrClientConnClosing
 	}
 	return err
 }
diff --git a/clientv3/integration/server_shutdown_test.go b/clientv3/integration/server_shutdown_test.go
index 8a89acc58..dcda528db 100644
--- a/clientv3/integration/server_shutdown_test.go
+++ b/clientv3/integration/server_shutdown_test.go
@@ -361,7 +361,10 @@ func isServerCtxTimeout(err error) bool {
 	if err == nil {
 		return false
 	}
-	ev, _ := status.FromError(err)
+	ev, ok := status.FromError(err)
+	if !ok {
+		return false
+	}
 	code := ev.Code()
 	return code == codes.DeadlineExceeded && strings.Contains(err.Error(), "context deadline exceeded")
 }
diff --git a/clientv3/leasing/kv.go b/clientv3/leasing/kv.go
index 5a5e2312b..051a8fceb 100644
--- a/clientv3/leasing/kv.go
+++ b/clientv3/leasing/kv.go
@@ -285,7 +285,7 @@ func (lkv *leasingKV) acquire(ctx context.Context, key string, op v3.Op) (*v3.Tx
 		if _, ok := err.(rpctypes.EtcdError); ok {
 			return nil, err
 		}
-		if ev, _ := status.FromError(err); ev.Code() != codes.Unavailable {
+		if ev, ok := status.FromError(err); ok && ev.Code() != codes.Unavailable {
 			return nil, err
 		}
 	}
diff --git a/clientv3/retry.go b/clientv3/retry.go
index 7f89ba641..f923f74ba 100644
--- a/clientv3/retry.go
+++ b/clientv3/retry.go
@@ -52,7 +52,10 @@ func isRepeatableStopError(err error) bool {
 		return true
 	}
 	// only retry if unavailable
-	ev, _ := status.FromError(err)
+	ev, ok := status.FromError(err)
+	if !ok {
+		return false
+	}
 	return ev.Code() != codes.Unavailable
 }
 
@@ -68,8 +71,7 @@ func isRepeatableStopError(err error) bool {
 // Returning "true" means retry should stop, otherwise it violates
 // write-at-most-once semantics.
 func isNonRepeatableStopError(err error) bool {
-	ev, _ := status.FromError(err)
-	if ev.Code() != codes.Unavailable {
+	if ev, ok := status.FromError(err); ok && ev.Code() != codes.Unavailable {
 		return true
 	}
 	desc := rpctypes.ErrorDesc(err)
diff --git a/etcdserver/api/v3rpc/rpctypes/error_test.go b/etcdserver/api/v3rpc/rpctypes/error_test.go
index e1b396822..525d96983 100644
--- a/etcdserver/api/v3rpc/rpctypes/error_test.go
+++ b/etcdserver/api/v3rpc/rpctypes/error_test.go
@@ -29,16 +29,14 @@ func TestConvert(t *testing.T) {
 	if e1.Error() != e2.Error() {
 		t.Fatalf("expected %q == %q", e1.Error(), e2.Error())
 	}
-	ev1, _ := status.FromError(e1)
-	if ev1.Code() != e3.(EtcdError).Code() {
+	if ev1, ok := status.FromError(e1); ok && ev1.Code() != e3.(EtcdError).Code() {
 		t.Fatalf("expected them to be equal, got %v / %v", ev1.Code(), e3.(EtcdError).Code())
 	}
 
 	if e1.Error() == e3.Error() {
 		t.Fatalf("expected %q != %q", e1.Error(), e3.Error())
 	}
-	ev2, _ := status.FromError(e2)
-	if ev2.Code() != e3.(EtcdError).Code() {
+	if ev2, ok := status.FromError(e2); ok && ev2.Code() != e3.(EtcdError).Code() {
 		t.Fatalf("expected them to be equal, got %v / %v", ev2.Code(), e3.(EtcdError).Code())
 	}
 }
diff --git a/tools/functional-tester/etcd-runner/command/lease_renewer_command.go b/tools/functional-tester/etcd-runner/command/lease_renewer_command.go
index 2df7c1c17..85a022626 100644
--- a/tools/functional-tester/etcd-runner/command/lease_renewer_command.go
+++ b/tools/functional-tester/etcd-runner/command/lease_renewer_command.go
@@ -68,8 +68,7 @@ func runLeaseRenewerFunc(cmd *cobra.Command, args []string) {
 
 		for {
 			lk, err = c.Lease.KeepAliveOnce(ctx, l.ID)
-			ev, _ := status.FromError(err)
-			if ev.Code() == codes.NotFound {
+			if ev, ok := status.FromError(err); ok && ev.Code() == codes.NotFound {
 				if time.Since(expire) < 0 {
 					log.Fatalf("bad renew! exceeded: %v", time.Since(expire))
 					for {