Removed grpc::StatusCode::OK from the definition of IsTransientFailure (#1203)

manish-qlogic · coryan · commit edce37799a6c · 2018-10-03T07:46:58.000-04:00
This fixes #741.
diff --git a/google/cloud/bigtable/rpc_retry_policy.h b/google/cloud/bigtable/rpc_retry_policy.h
@@ -31,7 +31,7 @@ struct SafeGrpcRetry {
   // Sometimes we need to use google::protobuf::rpc::Status, in which case this
   // is a easier function
   static inline bool IsTransientFailure(grpc::StatusCode code) {
-    return code == grpc::StatusCode::OK or code == grpc::StatusCode::ABORTED or
+    return code == grpc::StatusCode::ABORTED or
            code == grpc::StatusCode::UNAVAILABLE or
            code == grpc::StatusCode::DEADLINE_EXCEEDED;
   }
@@ -41,7 +41,7 @@ struct SafeGrpcRetry {
     return IsTransientFailure(status.error_code());
   }
   static inline bool IsPermanentFailure(grpc::Status const& status) {
-    return not IsTransientFailure(status);
+    return not IsOk(status) and not IsTransientFailure(status);
   }
 };
 
diff --git a/google/cloud/bigtable/rpc_retry_policy_test.cc b/google/cloud/bigtable/rpc_retry_policy_test.cc
@@ -59,6 +59,15 @@ TEST(LimitedTimeRetryPolicy, Simple) {
   CheckLimitedTime(tested);
 }
 
+/// @test A simple test for grpc::StatusCode::OK is not Permanent Error.
+TEST(LimitedTimeRetryPolicy, PermanentFailureCheck) {
+  bigtable::LimitedTimeRetryPolicy tested(kLimitedTimeTestPeriod);
+  EXPECT_FALSE(tested.IsPermanentFailure(
+      grpc::Status(grpc::StatusCode::OK, "No Error")));
+  EXPECT_FALSE(tested.IsPermanentFailure(CreateTransientError()));
+  EXPECT_TRUE(tested.IsPermanentFailure(CreatePermanentError()));
+}
+
 /// @test Test cloning for LimitedTimeRetryPolicy.
 TEST(LimitedTimeRetryPolicy, Clone) {
   using namespace google::cloud::testing_util::chrono_literals;

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ struct SafeGrpcRetry {`
`31`	`31`	`// Sometimes we need to use google::protobuf::rpc::Status, in which case this`
`32`	`32`	`// is a easier function`
`33`	`33`	`static inline bool IsTransientFailure(grpc::StatusCode code) {`
`34`		`- return code == grpc::StatusCode::OK or code == grpc::StatusCode::ABORTED or`
	`34`	`+ return code == grpc::StatusCode::ABORTED or`
`35`	`35`	`code == grpc::StatusCode::UNAVAILABLE or`
`36`	`36`	`code == grpc::StatusCode::DEADLINE_EXCEEDED;`
`37`	`37`	`}`
`@@ -41,7 +41,7 @@ struct SafeGrpcRetry {`
`41`	`41`	`return IsTransientFailure(status.error_code());`
`42`	`42`	`}`
`43`	`43`	`static inline bool IsPermanentFailure(grpc::Status const& status) {`
`44`		`- return not IsTransientFailure(status);`
	`44`	`+ return not IsOk(status) and not IsTransientFailure(status);`
`45`	`45`	`}`
`46`	`46`	`};`
`47`	`47`