Skip to content

Commit ddbf852

Browse files
lia-viamlia-viam
authored
RSDK-9672 Insulate grpc Status (#350)
1 parent e71d4c6 commit ddbf852

27 files changed

+64
-54
lines changed

‎src/viam/sdk/CMakeLists.txt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ target_sources(viamsdk
5353
common/linear_algebra.cpp
5454
common/pose.cpp
5555
common/proto_value.cpp
56-
common/service_helper.cpp
5756
common/utils.cpp
5857
common/version_metadata.cpp
5958
common/world_state.cpp
59+
common/private/service_helper.cpp
6060
components/arm.cpp
6161
components/base.cpp
6262
components/board.cpp
@@ -147,7 +147,6 @@ target_sources(viamsdk
147147
../../viam/sdk/common/pose.hpp
148148
../../viam/sdk/common/proto_convert.hpp
149149
../../viam/sdk/common/proto_value.hpp
150-
../../viam/sdk/common/service_helper.hpp
151150
../../viam/sdk/common/utils.hpp
152151
../../viam/sdk/common/version_metadata.hpp
153152
../../viam/sdk/common/world_state.hpp

‎src/viam/sdk/common/client_helper.cpp‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <cstdlib>
44

55
#include <grpcpp/client_context.h>
6+
#include <grpcpp/support/status.h>
67

78
#include <boost/log/trivial.hpp>
89

@@ -13,12 +14,16 @@ namespace sdk {
1314

1415
namespace client_helper_details {
1516

16-
[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status& status) noexcept {
17+
[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status* status) noexcept {
1718
BOOST_LOG_TRIVIAL(fatal) << "ClientHelper error handler callback returned instead of throwing: "
18-
<< status.error_message() << '(' << status.error_details() << ')';
19+
<< status->error_message() << '(' << status->error_details() << ')';
1920
std::abort();
2021
}
2122

23+
bool isStatusCancelled(int status) noexcept {
24+
return status == ::grpc::StatusCode::CANCELLED;
25+
}
26+
2227
} // namespace client_helper_details
2328

2429
ClientContext::ClientContext() : wrapped_context_(std::make_unique<GrpcClientContext>()) {

‎src/viam/sdk/common/client_helper.hpp‎

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,15 @@
55
#include <viam/sdk/common/private/utils.hpp>
66
#include <viam/sdk/common/proto_value.hpp>
77

8-
namespace grpc {
9-
10-
class Status;
11-
12-
} // namespace grpc
13-
148
namespace viam {
159
namespace sdk {
1610

1711
namespace client_helper_details {
1812

19-
[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status&) noexcept;
13+
[[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status*) noexcept;
14+
15+
// Helper function to test equality of status with grpc::StatusCode::CANCELLED.
16+
bool isStatusCancelled(int status) noexcept;
2017

2118
} // namespace client_helper_details
2219

@@ -62,7 +59,7 @@ template <typename ClientType,
6259
class ClientHelper {
6360
static void default_rsc_(RequestType&) {}
6461
static void default_rhc_(const ResponseType&) {}
65-
static void default_ehc_(const ::grpc::Status& status) {
62+
static void default_ehc_(const ::grpc::Status* status) {
6663
throw GRPCException(status);
6764
}
6865

@@ -111,8 +108,8 @@ class ClientHelper {
111108
const_cast<const ResponseType&>(response_));
112109
}
113110

114-
std::forward<ErrorHandlerCallable>(ehc)(result);
115-
client_helper_details::errorHandlerReturnedUnexpectedly(result);
111+
std::forward<ErrorHandlerCallable>(ehc)(&result);
112+
client_helper_details::errorHandlerReturnedUnexpectedly(&result);
116113
}
117114

118115
// A version of invoke for gRPC calls returning `(stream ResponseType)`.
@@ -138,13 +135,13 @@ class ClientHelper {
138135

139136
const auto result = reader->Finish();
140137

141-
if (result.ok() ||
142-
(cancelled_by_handler && result.error_code() == ::grpc::StatusCode::CANCELLED)) {
138+
if (result.ok() || (cancelled_by_handler &&
139+
client_helper_details::isStatusCancelled(result.error_code()))) {
143140
return;
144141
}
145142

146-
std::forward<ErrorHandlerCallable>(ehc)(result);
147-
client_helper_details::errorHandlerReturnedUnexpectedly(result);
143+
std::forward<ErrorHandlerCallable>(ehc)(&result);
144+
client_helper_details::errorHandlerReturnedUnexpectedly(&result);
148145
}
149146

150147
private:

‎src/viam/sdk/common/exception.cpp‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
#include <viam/sdk/common/exception.hpp>
22

3+
#include <grpcpp/support/status.h>
4+
35
namespace viam {
46
namespace sdk {
57

68
Exception::Exception(ErrorCondition condition, const std::string& what)
7-
: std::runtime_error("viam::sdk::Exception: " + what), condition_(condition){};
8-
9-
Exception::Exception(const std::string& what) : Exception(ErrorCondition::k_general, what){};
9+
: std::runtime_error("viam::sdk::Exception: " + what), condition_(condition) {}
1010

11-
Exception::~Exception() = default;
11+
Exception::Exception(const std::string& what) : Exception(ErrorCondition::k_general, what) {}
1212

1313
const std::error_condition& Exception::condition() const noexcept {
1414
return condition_;
@@ -45,11 +45,12 @@ std::error_condition make_error_condition(ErrorCondition e) {
4545
return {static_cast<int>(e), errorCategory};
4646
}
4747

48-
GRPCException::GRPCException(grpc::Status status)
49-
: Exception(ErrorCondition::k_grpc, status.error_message()), status_(std::move(status)){};
48+
GRPCException::GRPCException(const grpc::Status* status)
49+
: Exception(ErrorCondition::k_grpc, status->error_message()),
50+
status_(std::make_shared<grpc::Status>(*status)) {}
5051

51-
const grpc::Status& GRPCException::status() const noexcept {
52-
return status_;
52+
const grpc::Status* GRPCException::status() const noexcept {
53+
return status_.get();
5354
}
5455

5556
} // namespace sdk

‎src/viam/sdk/common/exception.hpp‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@
22
///
33
/// @brief Defines custom exceptions for the SDK.
44
#pragma once
5-
#include <grpcpp/support/status.h>
5+
6+
#include <memory>
67
#include <stdexcept>
78
#include <string>
9+
#include <system_error>
10+
11+
namespace grpc {
812

9-
#include <viam/sdk/resource/resource_api.hpp>
13+
class Status;
14+
15+
} // namespace grpc
1016

1117
namespace viam {
1218
namespace sdk {
@@ -36,7 +42,8 @@ class Exception : public std::runtime_error {
3642
public:
3743
explicit Exception(ErrorCondition condition, const std::string& what);
3844
explicit Exception(const std::string& what);
39-
virtual ~Exception();
45+
46+
~Exception() override = default;
4047

4148
const std::error_condition& condition() const noexcept;
4249

@@ -49,12 +56,13 @@ class Exception : public std::runtime_error {
4956
/// @ingroup Exception
5057
class GRPCException : public Exception {
5158
public:
52-
explicit GRPCException(grpc::Status status);
59+
explicit GRPCException(const grpc::Status* status);
60+
~GRPCException() override = default;
5361

54-
const grpc::Status& status() const noexcept;
62+
const grpc::Status* status() const noexcept;
5563

5664
private:
57-
grpc::Status status_;
65+
std::shared_ptr<grpc::Status> status_;
5866
};
5967

6068
} // namespace sdk

‎src/viam/sdk/common/service_helper.cpp‎ renamed to ‎src/viam/sdk/common/private/service_helper.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#include <viam/sdk/common/service_helper.hpp>
1+
#include <viam/sdk/common/private/service_helper.hpp>
22

33
#include <sstream>
44

‎src/viam/sdk/common/service_helper.hpp‎ renamed to ‎src/viam/sdk/common/private/service_helper.hpp‎

File renamed without changes.

‎src/viam/sdk/components/private/arm_server.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#include <viam/sdk/components/private/arm_server.hpp>
22

3-
#include <viam/sdk/common/service_helper.hpp>
3+
#include <viam/sdk/common/private/service_helper.hpp>
44

55
namespace viam {
66
namespace sdk {

‎src/viam/sdk/components/private/base_server.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
#include <viam/api/component/base/v1/base.pb.h>
44

55
#include <viam/sdk/common/linear_algebra.hpp>
6-
#include <viam/sdk/common/service_helper.hpp>
6+
#include <viam/sdk/common/private/service_helper.hpp>
77
#include <viam/sdk/common/utils.hpp>
88
#include <viam/sdk/components/base.hpp>
99
#include <viam/sdk/config/resource.hpp>

‎src/viam/sdk/components/private/board_client.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ Board::analog_response BoardClient::read_analog(const std::string& analog_reader
123123

124124
const grpc::Status status = stub_->ReadAnalogReader(ctx, request, &response);
125125
if (!status.ok()) {
126-
throw GRPCException(status);
126+
throw GRPCException(&status);
127127
}
128128
return Board::analog_response{
129129
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
153153

154154
const grpc::Status status = stub_->GetDigitalInterruptValue(ctx, request, &response);
155155
if (!status.ok()) {
156-
throw GRPCException(status);
156+
throw GRPCException(&status);
157157
}
158158
return response.value();
159159
}

0 commit comments

Comments
 (0)