refer to grpc status by pointer in grpc exception

viamrobotics · Jan 7, 2025 · e003d82 · e003d82
1 parent e71d4c6
commit e003d82
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 30 deletions.
diff --git a/src/viam/sdk/common/client_helper.cpp b/src/viam/sdk/common/client_helper.cpp
@@ -3,6 +3,7 @@
 #include <cstdlib>
 
 #include <grpcpp/client_context.h>
+#include <grpcpp/support/status.h>
 
 #include <boost/log/trivial.hpp>
 
@@ -13,12 +14,16 @@ namespace sdk {
 
 namespace client_helper_details {
 
-[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status& status) noexcept {
+[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status* status) noexcept {
     BOOST_LOG_TRIVIAL(fatal) << "ClientHelper error handler callback returned instead of throwing: "
-                             << status.error_message() << '(' << status.error_details() << ')';
+                             << status->error_message() << '(' << status->error_details() << ')';
     std::abort();
 }
 
+bool isStatusCancelled(int status) noexcept {
+    return status = ::grpc::StatusCode::CANCELLED;
+}
+
 }  // namespace client_helper_details
 
 ClientContext::ClientContext() : wrapped_context_(std::make_unique<GrpcClientContext>()) {

diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp
@@ -5,18 +5,15 @@
 #include <viam/sdk/common/private/utils.hpp>
 #include <viam/sdk/common/proto_value.hpp>
 
-namespace grpc {
-
-class Status;
-
-}  // namespace grpc
-
 namespace viam {
 namespace sdk {
 
 namespace client_helper_details {
 
-[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status&) noexcept;
+[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status*) noexcept;
+
+// Helper function to test equality of status with grpc::StatusCode::CANCELLED.
+bool isStatusCancelled(int status) noexcept;
 
 }  // namespace client_helper_details
 
@@ -62,7 +59,7 @@ template <typename ClientType,
 class ClientHelper {
     static void default_rsc_(RequestType&) {}
     static void default_rhc_(const ResponseType&) {}
-    static void default_ehc_(const ::grpc::Status& status) {
+    static void default_ehc_(const ::grpc::Status* status) {
         throw GRPCException(status);
     }
 
@@ -111,8 +108,8 @@ class ClientHelper {
                 const_cast<const ResponseType&>(response_));
         }
 
-        std::forward<ErrorHandlerCallable>(ehc)(result);
-        client_helper_details::errorHandlerReturnedUnexpectedly(result);
+        std::forward<ErrorHandlerCallable>(ehc)(&result);
+        client_helper_details::errorHandlerReturnedUnexpectedly(&result);
     }
 
     // A version of invoke for gRPC calls returning `(stream ResponseType)`.
@@ -138,13 +135,13 @@ class ClientHelper {
 
         const auto result = reader->Finish();
 
-        if (result.ok() ||
-            (cancelled_by_handler && result.error_code() == ::grpc::StatusCode::CANCELLED)) {
+        if (result.ok() || (cancelled_by_handler &&
+                            client_helper_details::isStatusCancelled(result.error_code()))) {
             return;
         }
 
-        std::forward<ErrorHandlerCallable>(ehc)(result);
-        client_helper_details::errorHandlerReturnedUnexpectedly(result);
+        std::forward<ErrorHandlerCallable>(ehc)(&result);
+        client_helper_details::errorHandlerReturnedUnexpectedly(&result);
     }
 
    private:

diff --git a/src/viam/sdk/common/exception.cpp b/src/viam/sdk/common/exception.cpp
@@ -1,12 +1,14 @@
 #include <viam/sdk/common/exception.hpp>
 
+#include <grpcpp/support/status.h>
+
 namespace viam {
 namespace sdk {
 
 Exception::Exception(ErrorCondition condition, const std::string& what)
-    : std::runtime_error("viam::sdk::Exception: " + what), condition_(condition){};
+    : std::runtime_error("viam::sdk::Exception: " + what), condition_(condition) {};
 
-Exception::Exception(const std::string& what) : Exception(ErrorCondition::k_general, what){};
+Exception::Exception(const std::string& what) : Exception(ErrorCondition::k_general, what) {};
 
 Exception::~Exception() = default;
 
@@ -45,11 +47,14 @@ std::error_condition make_error_condition(ErrorCondition e) {
     return {static_cast<int>(e), errorCategory};
 }
 
-GRPCException::GRPCException(grpc::Status status)
-    : Exception(ErrorCondition::k_grpc, status.error_message()), status_(std::move(status)){};
+GRPCException::GRPCException(const grpc::Status* status)
+    : Exception(ErrorCondition::k_grpc, status->error_message()),
+      status_(std::make_unique<grpc::Status>(*status)) {}
+
+GRPCException::~GRPCException() = default;
 
-const grpc::Status& GRPCException::status() const noexcept {
-    return status_;
+const grpc::Status* GRPCException::status() const noexcept {
+    return status_.get();
 }
 
 }  // namespace sdk

diff --git a/src/viam/sdk/common/exception.hpp b/src/viam/sdk/common/exception.hpp
@@ -2,11 +2,17 @@
 ///
 /// @brief Defines custom exceptions for the SDK.
 #pragma once
-#include <grpcpp/support/status.h>
+
+#include <memory>
 #include <stdexcept>
 #include <string>
+#include <system_error>
+
+namespace grpc {
+
+class Status;
 
-#include <viam/sdk/resource/resource_api.hpp>
+}  // namespace grpc
 
 namespace viam {
 namespace sdk {
@@ -49,12 +55,13 @@ class Exception : public std::runtime_error {
 /// @ingroup Exception
 class GRPCException : public Exception {
    public:
-    explicit GRPCException(grpc::Status status);
+    explicit GRPCException(const grpc::Status* status);
+    ~GRPCException();
 
-    const grpc::Status& status() const noexcept;
+    const grpc::Status* status() const noexcept;
 
    private:
-    grpc::Status status_;
+    std::unique_ptr<grpc::Status> status_;
 };
 
 }  // namespace sdk

diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp
@@ -123,7 +123,7 @@ Board::analog_response BoardClient::read_analog(const std::string& analog_reader
 
     const grpc::Status status = stub_->ReadAnalogReader(ctx, request, &response);
     if (!status.ok()) {
-        throw GRPCException(status);
+        throw GRPCException(&status);
     }
     return Board::analog_response{
         response.value(), response.min_range(), response.max_range(), response.step_size()};
@@ -153,7 +153,7 @@ Board::digital_value BoardClient::read_digital_interrupt(const std::string& digi
 
     const grpc::Status status = stub_->GetDigitalInterruptValue(ctx, request, &response);
     if (!status.ok()) {
-        throw GRPCException(status);
+        throw GRPCException(&status);
     }
     return response.value();
 }

diff --git a/src/viam/sdk/services/private/mlmodel_client.cpp b/src/viam/sdk/services/private/mlmodel_client.cpp
@@ -71,7 +71,7 @@ std::shared_ptr<MLModelService::named_tensor_views> MLModelServiceClient::infer(
 
     const auto result = stub_->Infer(ctx, *req, resp);
     if (!result.ok()) {
-        throw GRPCException(result);
+        throw GRPCException(&result);
     }
 
     for (const auto& kv : resp->output_tensors().tensors()) {