Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSDK-9672 Insulate grpc Status #350

Merged
merged 5 commits into from
Jan 8, 2025
Merged

RSDK-9672 Insulate grpc Status #350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e003d82
refer to grpc status by pointer in grpc exception
lia-viam Jan 7, 2025
546d782
make service_helper private
lia-viam Jan 7, 2025
1b67b11
use shared_ptr and make dtors override = default
lia-viam Jan 7, 2025
5723aa5
remove semicolons for linter
lia-viam Jan 7, 2025
8671c5b
fix equality typo
lia-viam Jan 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/viam/sdk/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ target_sources(viamsdk
common/linear_algebra.cpp
common/pose.cpp
common/proto_value.cpp
common/service_helper.cpp
common/utils.cpp
common/version_metadata.cpp
common/world_state.cpp
common/private/service_helper.cpp
components/arm.cpp
components/base.cpp
components/board.cpp
Expand Down Expand Up @@ -147,7 +147,6 @@ target_sources(viamsdk
../../viam/sdk/common/pose.hpp
../../viam/sdk/common/proto_convert.hpp
../../viam/sdk/common/proto_value.hpp
../../viam/sdk/common/service_helper.hpp
../../viam/sdk/common/utils.hpp
../../viam/sdk/common/version_metadata.hpp
../../viam/sdk/common/world_state.hpp
Expand Down
9 changes: 7 additions & 2 deletions src/viam/sdk/common/client_helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <cstdlib>

#include <grpcpp/client_context.h>
#include <grpcpp/support/status.h>

#include <boost/log/trivial.hpp>

Expand All @@ -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>()) {
Expand Down
25 changes: 11 additions & 14 deletions src/viam/sdk/common/client_helper.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)`.
Expand All @@ -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()))) {
Copy link
Member

@stuqdog stuqdog Jan 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) delete blank newline here? jk formatting made it look weird on my computer, ignore!

return;
}

std::forward<ErrorHandlerCallable>(ehc)(result);
client_helper_details::errorHandlerReturnedUnexpectedly(result);
std::forward<ErrorHandlerCallable>(ehc)(&result);
client_helper_details::errorHandlerReturnedUnexpectedly(&result);
}

private:
Expand Down
17 changes: 9 additions & 8 deletions src/viam/sdk/common/exception.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +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){};

Exception::Exception(const std::string& what) : Exception(ErrorCondition::k_general, what){};
: std::runtime_error("viam::sdk::Exception: " + what), condition_(condition) {}

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

const std::error_condition& Exception::condition() const noexcept {
return condition_;
Expand Down Expand Up @@ -45,11 +45,12 @@ 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_shared<grpc::Status>(*status)) {}

const grpc::Status& GRPCException::status() const noexcept {
return status_;
const grpc::Status* GRPCException::status() const noexcept {
return status_.get();
}

} // namespace sdk
Expand Down
20 changes: 14 additions & 6 deletions src/viam/sdk/common/exception.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

#include <viam/sdk/resource/resource_api.hpp>
class Status;

} // namespace grpc

namespace viam {
namespace sdk {
Expand Down Expand Up @@ -36,7 +42,8 @@ class Exception : public std::runtime_error {
public:
explicit Exception(ErrorCondition condition, const std::string& what);
explicit Exception(const std::string& what);
virtual ~Exception();

~Exception() override = default;

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

Expand All @@ -49,12 +56,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() override = default;

const grpc::Status& status() const noexcept;
const grpc::Status* status() const noexcept;

private:
grpc::Status status_;
std::shared_ptr<grpc::Status> status_;
};

} // namespace sdk
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/common/service_helper.cpp → ...iam/sdk/common/private/service_helper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>

#include <sstream>

Expand Down
0 src/viam/sdk/common/service_helper.hpp → ...iam/sdk/common/private/service_helper.hpp
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/arm_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/arm_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>

namespace viam {
namespace sdk {
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/base_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include <viam/api/component/base/v1/base.pb.h>

#include <viam/sdk/common/linear_algebra.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/base.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
4 changes: 2 additions & 2 deletions src/viam/sdk/components/private/board_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/board_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <viam/sdk/components/private/board_server.hpp>

#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/board.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/camera_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#include <google/protobuf/util/time_util.h>
#include <grpcpp/support/status.h>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/camera.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/encoder_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <viam/sdk/components/private/encoder_server.hpp>

#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/encoder.hpp>
#include <viam/sdk/components/private/encoder.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/gantry_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/gantry_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>

namespace viam {
namespace sdk {
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/generic_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/generic_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/components/generic.hpp>
#include <viam/sdk/rpc/server.hpp>

Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/gripper_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/gripper_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>

namespace viam {
namespace sdk {
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/motor_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <viam/sdk/components/private/motor_server.hpp>

#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/motor.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/movement_sensor_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include <viam/sdk/components/private/movement_sensor_server.hpp>

#include <viam/sdk/common/linear_algebra.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/movement_sensor.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/pose_tracker_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

#include <viam/api/component/posetracker/v1/pose_tracker.pb.h>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/pose_tracker.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/power_sensor_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/power_sensor_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/power_sensor.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/sensor_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/sensor_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/sensor.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/components/private/servo_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/components/private/servo_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/components/servo.hpp>
#include <viam/sdk/config/resource.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/generic_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#include <viam/sdk/services/private/generic_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/rpc/server.hpp>
#include <viam/sdk/services/generic.hpp>

Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/mlmodel_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/mlmodel_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

#include <viam/sdk/services/private/mlmodel_server.hpp>

#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/rpc/server.hpp>
#include <viam/sdk/services/mlmodel.hpp>
#include <viam/sdk/services/private/mlmodel.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/motion_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/common/pose.hpp>
#include <viam/sdk/common/private/repeated_ptr_convert.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/services/motion.hpp>
#include <viam/sdk/services/private/motion_server.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/navigation_server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@

#include <viam/sdk/common/pose.hpp>
#include <viam/sdk/common/private/repeated_ptr_convert.hpp>
#include <viam/sdk/common/private/service_helper.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/common/service_helper.hpp>
#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/services/motion.hpp>
#include <viam/sdk/services/private/navigation_server.hpp>
Expand Down
Loading