Skip to content

Commit 507d936

Browse files
arkjedrzarkjedrz
committed
hmon: rework error handling in FFI
- Uniformly return `FFICode` in FFI functions. - Unit tests for FFI functions in Rust. - Rework comments for FFI functions. - Rework error handling in FFI layer. - Use factory method for object construction in C++. - To provide error handling. - Move `crate::common::ffi` to `crate::ffi`.
1 parent ab9e0e8 commit 507d936

13 files changed

Lines changed: 1408 additions & 382 deletions

File tree

‎examples/cpp_supervised_app/main.cpp‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,16 @@ int main(int argc, char** argv)
116116

117117
using namespace score::hm;
118118

119+
auto builder_create_result{deadline::DeadlineMonitorBuilder::create()};
120+
if (!builder_create_result)
121+
{
122+
std::cerr << "Failed to create deadline monitor builder" << std::endl;
123+
return EXIT_FAILURE;
124+
}
125+
auto deadline_monitor_builder{std::move(builder_create_result.value())};
126+
119127
auto builder_mon =
120-
deadline::DeadlineMonitorBuilder()
128+
deadline_monitor_builder
121129
.add_deadline(IdentTag("deadline_1"),
122130
TimeRange(std::chrono::milliseconds(50), std::chrono::milliseconds(150)))
123131
.add_deadline(IdentTag("deadline_2"),
@@ -128,20 +136,33 @@ int main(int argc, char** argv)
128136
IdentTag ident("monitor");
129137

130138
{
131-
auto hm = HealthMonitorBuilder()
132-
.add_deadline_monitor(ident, std::move(builder_mon))
133-
.with_internal_processing_cycle(std::chrono::milliseconds(50))
134-
.with_supervisor_api_cycle(std::chrono::milliseconds(50))
135-
.build();
139+
auto hmon_builder{HealthMonitorBuilder::create()};
140+
if (!hmon_builder)
141+
{
142+
std::cerr << "Failed to create health monitor builder" << std::endl;
143+
return EXIT_FAILURE;
144+
}
145+
146+
auto hmon_build_result{hmon_builder.value()
147+
.add_deadline_monitor(ident, std::move(builder_mon))
148+
.with_internal_processing_cycle(std::chrono::milliseconds(50))
149+
.with_supervisor_api_cycle(std::chrono::milliseconds(50))
150+
.build()};
151+
if (!hmon_build_result)
152+
{
153+
std::cerr << "Failed to build health monitor" << std::endl;
154+
return EXIT_FAILURE;
155+
}
156+
auto hmon{std::move(hmon_build_result.value())};
136157

137-
auto deadline_monitor_res = hm.get_deadline_monitor(ident);
158+
auto deadline_monitor_res = hmon.get_deadline_monitor(ident);
138159
if (!deadline_monitor_res.has_value())
139160
{
140161
std::cerr << "Failed to get deadline monitor" << std::endl;
141162
return EXIT_FAILURE;
142163
}
143164

144-
hm.start();
165+
hmon.start();
145166
score::lcm::LifecycleClient{}.ReportExecutionState(score::lcm::ExecutionState::kRunning);
146167

147168
auto deadline_mon = std::move(*deadline_monitor_res);

‎src/health_monitoring_lib/BUILD‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ PROC_MACRO_DEPS = [
2828
CC_SOURCES = [
2929
"cpp/common.cpp",
3030
"cpp/deadline_monitor.cpp",
31-
"cpp/ffi_helpers.h",
3231
"cpp/health_monitor.cpp",
3332
]
3433

‎src/health_monitoring_lib/cpp/deadline_monitor.cpp‎

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -11,65 +11,83 @@
1111
* SPDX-License-Identifier: Apache-2.0
1212
********************************************************************************/
1313
#include "score/hm/deadline/deadline_monitor.h"
14-
#include "ffi_helpers.h"
1514

15+
namespace
16+
{
1617
extern "C" {
1718
using namespace score::hm;
1819
using namespace score::hm::internal;
1920
using namespace score::hm::deadline;
2021

21-
// Deadline monitoring Foreign Function Interface Declarations that are exported by Rust implementation library
22-
internal::FFIHandle deadline_monitor_builder_create();
23-
void deadline_monitor_builder_destroy(internal::FFIHandle handle);
24-
void deadline_monitor_builder_add_deadline(internal::FFIHandle handler,
25-
const IdentTag* tag,
26-
uint32_t min,
27-
uint32_t max);
28-
int deadline_monitor_cpp_get_deadline(FFIHandle handler, const IdentTag* tag, FFIHandle* out);
29-
void deadline_monitor_cpp_destroy(FFIHandle handler);
30-
void deadline_destroy(FFIHandle deadline_handle);
31-
int deadline_start(FFIHandle deadline_handle);
32-
void deadline_stop(FFIHandle deadline_handle);
22+
// Functions below must match functions defined in `crate::deadline::ffi`.
23+
24+
FFICode deadline_monitor_builder_create(FFIHandle* deadline_monitor_builder_handle);
25+
FFICode deadline_monitor_builder_destroy(FFIHandle deadline_monitor_builder_handle);
26+
FFICode deadline_monitor_builder_add_deadline(FFIHandle deadline_monitor_builder_handle,
27+
const IdentTag* deadline_tag,
28+
uint32_t min,
29+
uint32_t max);
30+
FFICode deadline_monitor_get_deadline(FFIHandle deadline_monitor_handle,
31+
const IdentTag* deadline_tag,
32+
FFIHandle* deadline_handle);
33+
FFICode deadline_monitor_destroy(FFIHandle deadline_monitor_handle);
34+
FFICode deadline_destroy(FFIHandle deadline_handle);
35+
FFICode deadline_start(FFIHandle deadline_handle);
36+
FFICode deadline_stop(FFIHandle deadline_handle);
3337
}
38+
} // namespace
3439

3540
// C++ wrapper for Rust library - the API implementation obeys the Rust API semantics and it's invariants
3641

3742
namespace score::hm::deadline
3843
{
39-
DeadlineMonitorBuilder::DeadlineMonitorBuilder()
40-
: monitor_builder_handler_(deadline_monitor_builder_create(), &deadline_monitor_builder_destroy)
44+
45+
score::cpp::expected<DeadlineMonitorBuilder, Error> DeadlineMonitorBuilder::create()
46+
{
47+
FFIHandle handle{nullptr};
48+
auto result{deadline_monitor_builder_create(&handle)};
49+
if (result != kSuccess)
50+
{
51+
return score::cpp::unexpected(static_cast<Error>(result));
52+
}
53+
54+
return score::cpp::expected<DeadlineMonitorBuilder, Error>{DeadlineMonitorBuilder{handle}};
55+
}
56+
57+
DeadlineMonitorBuilder::DeadlineMonitorBuilder(FFIHandle handle)
58+
: monitor_builder_handler_(handle, &deadline_monitor_builder_destroy)
4159
{
4260
}
4361

44-
DeadlineMonitorBuilder DeadlineMonitorBuilder::add_deadline(const IdentTag& tag, const TimeRange& range) &&
62+
DeadlineMonitorBuilder DeadlineMonitorBuilder::add_deadline(const IdentTag& tag, const TimeRange& range)
4563
{
4664
auto handle = monitor_builder_handler_.as_rust_handle();
4765
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
4866

67+
// TODO: how errors should be handled?
4968
deadline_monitor_builder_add_deadline(handle.value(), &tag, range.min_ms(), range.max_ms());
5069

5170
return std::move(*this);
5271
}
5372

54-
DeadlineMonitor::DeadlineMonitor(FFIHandle handle) : monitor_handle_(handle, &deadline_monitor_cpp_destroy) {}
73+
DeadlineMonitor::DeadlineMonitor(FFIHandle handle) : monitor_handle_(handle, &deadline_monitor_destroy) {}
5574

5675
score::cpp::expected<Deadline, score::hm::Error> DeadlineMonitor::get_deadline(const IdentTag& tag)
5776
{
5877
auto handle = monitor_handle_.as_rust_handle();
5978
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
6079

61-
internal::FFIHandle ret = nullptr;
62-
auto result = deadline_monitor_cpp_get_deadline(handle.value(), &tag, &ret);
63-
80+
FFIHandle ret = nullptr;
81+
auto result = deadline_monitor_get_deadline(handle.value(), &tag, &ret);
6482
if (result != kSuccess)
6583
{
66-
return score::cpp::unexpected(::score::hm::ffi::fromRustError(result));
84+
return score::cpp::unexpected(static_cast<Error>(result));
6785
}
6886

6987
return score::cpp::expected<Deadline, score::hm::Error>(Deadline{ret});
7088
}
7189

72-
Deadline::Deadline(internal::FFIHandle handle) : deadline_handle_(handle, &deadline_destroy), has_handle_(false) {}
90+
Deadline::Deadline(FFIHandle handle) : deadline_handle_(handle, &deadline_destroy), has_handle_(false) {}
7391

7492
Deadline::~Deadline()
7593
{
@@ -90,7 +108,7 @@ score::cpp::expected<DeadlineHandle, score::hm::Error> Deadline::start()
90108
auto result = deadline_start(handle.value());
91109
if (result != kSuccess)
92110
{
93-
return score::cpp::unexpected(::score::hm::ffi::fromRustError(result));
111+
return score::cpp::unexpected(static_cast<Error>(result));
94112
}
95113

96114
has_handle_ = true;
@@ -109,6 +127,7 @@ void DeadlineHandle::stop()
109127
auto handle = deadline_.value().get().deadline_handle_.as_rust_handle();
110128
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
111129

130+
// TODO: how errors should be handled?
112131
deadline_stop(handle.value());
113132
}
114133
}

‎src/health_monitoring_lib/cpp/ffi_helpers.h‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎src/health_monitoring_lib/cpp/health_monitor.cpp‎

Lines changed: 61 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -12,71 +12,98 @@
1212
********************************************************************************/
1313
#include "score/hm/health_monitor.h"
1414

15+
namespace
16+
{
1517
extern "C" {
1618
using namespace score::hm;
17-
18-
// Health Monitor Foreign Function Interface Declarations that are exported by Rust implementation library
19-
internal::FFIHandle health_monitor_builder_create();
20-
void health_monitor_builder_destroy(internal::FFIHandle handler);
21-
22-
internal::FFIHandle health_monitor_builder_build(internal::FFIHandle health_monitor_builder_handle,
23-
uint32_t supervisor_cycle_ms,
24-
uint32_t internal_cycle_ms);
25-
void health_monitor_builder_add_deadline_monitor(internal::FFIHandle handle,
26-
const IdentTag* tag,
27-
internal::FFIHandle monitor_handle);
28-
29-
internal::FFIHandle health_monitor_get_deadline_monitor(internal::FFIHandle health_monitor_handle, const IdentTag* tag);
30-
void health_monitor_start(internal::FFIHandle health_monitor_handle);
31-
void health_monitor_destroy(internal::FFIHandle handler);
19+
using namespace score::hm::internal;
20+
using namespace score::hm::deadline;
21+
22+
// Functions below must match functions defined in `crate::ffi`.
23+
24+
FFICode health_monitor_builder_create(FFIHandle* health_monitor_builder_handle);
25+
FFICode health_monitor_builder_destroy(FFIHandle health_monitor_builder_handle);
26+
FFICode health_monitor_builder_build(FFIHandle health_monitor_builder_handle,
27+
uint32_t supervisor_cycle_ms,
28+
uint32_t internal_cycle_ms,
29+
FFIHandle* health_monitor_handle);
30+
FFICode health_monitor_builder_add_deadline_monitor(FFIHandle health_monitor_builder_handle,
31+
const IdentTag* monitor_tag,
32+
FFIHandle deadline_monitor_builder_handle);
33+
FFICode health_monitor_get_deadline_monitor(FFIHandle health_monitor_handle,
34+
const IdentTag* monitor_tag,
35+
FFIHandle* deadline_monitor_handle);
36+
FFICode health_monitor_start(FFIHandle health_monitor_handle);
37+
FFICode health_monitor_destroy(FFIHandle health_monitor_handle);
3238
}
39+
} // namespace
3340

3441
// C++ wrapper for Rust library - the API implementation obeys the Rust API semantics and it's invariants
3542

3643
namespace score::hm
3744
{
3845

39-
HealthMonitorBuilder::HealthMonitorBuilder()
40-
: health_monitor_builder_handle_{health_monitor_builder_create(), &health_monitor_builder_destroy}
46+
score::cpp::expected<HealthMonitorBuilder, Error> HealthMonitorBuilder::create()
47+
{
48+
FFIHandle handle{nullptr};
49+
auto result{health_monitor_builder_create(&handle)};
50+
if (result != kSuccess)
51+
{
52+
return score::cpp::unexpected(static_cast<Error>(result));
53+
}
54+
55+
return score::cpp::expected<HealthMonitorBuilder, Error>{HealthMonitorBuilder{handle}};
56+
}
57+
58+
HealthMonitorBuilder::HealthMonitorBuilder(FFIHandle handle)
59+
: health_monitor_builder_handle_(handle, &health_monitor_builder_destroy)
4160
{
4261
}
4362

44-
HealthMonitorBuilder HealthMonitorBuilder::add_deadline_monitor(const IdentTag& tag,
45-
deadline::DeadlineMonitorBuilder&& monitor) &&
63+
HealthMonitorBuilder HealthMonitorBuilder::add_deadline_monitor(const IdentTag& tag, DeadlineMonitorBuilder&& monitor)
4664
{
4765
auto monitor_handle = monitor.drop_by_rust();
4866
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(monitor_handle.has_value());
4967
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle_.as_rust_handle().has_value());
5068

69+
// TODO: how errors should be handled?
5170
health_monitor_builder_add_deadline_monitor(
5271
health_monitor_builder_handle_.as_rust_handle().value(), &tag, monitor_handle.value());
5372
return std::move(*this);
5473
}
5574

56-
HealthMonitorBuilder HealthMonitorBuilder::with_internal_processing_cycle(std::chrono::milliseconds cycle_duration) &&
75+
HealthMonitorBuilder HealthMonitorBuilder::with_internal_processing_cycle(std::chrono::milliseconds cycle_duration)
5776
{
5877
internal_processing_cycle_duration_ = cycle_duration;
5978
return std::move(*this);
6079
}
6180

62-
HealthMonitorBuilder HealthMonitorBuilder::with_supervisor_api_cycle(std::chrono::milliseconds cycle_duration) &&
81+
HealthMonitorBuilder HealthMonitorBuilder::with_supervisor_api_cycle(std::chrono::milliseconds cycle_duration)
6382
{
6483
supervisor_api_cycle_duration_ = cycle_duration;
6584
return std::move(*this);
6685
}
6786

68-
HealthMonitor HealthMonitorBuilder::build() &&
87+
score::cpp::expected<HealthMonitor, Error> HealthMonitorBuilder::build()
6988
{
70-
auto handle = health_monitor_builder_handle_.drop_by_rust();
71-
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
89+
auto health_monitor_builder_handle = health_monitor_builder_handle_.drop_by_rust();
90+
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle.has_value());
7291

7392
uint32_t supervisor_duration_ms = static_cast<uint32_t>(supervisor_api_cycle_duration_.count());
7493
uint32_t internal_duration_ms = static_cast<uint32_t>(internal_processing_cycle_duration_.count());
7594

76-
return HealthMonitor(health_monitor_builder_build(handle.value(), supervisor_duration_ms, internal_duration_ms));
95+
FFIHandle health_monitor_handle{nullptr};
96+
auto result{health_monitor_builder_build(
97+
health_monitor_builder_handle.value(), supervisor_duration_ms, internal_duration_ms, &health_monitor_handle)};
98+
if (result != kSuccess)
99+
{
100+
return score::cpp::unexpected{static_cast<Error>(result)};
101+
}
102+
103+
return score::cpp::expected<HealthMonitor, Error>{HealthMonitor{health_monitor_handle}};
77104
}
78105

79-
HealthMonitor::HealthMonitor(internal::FFIHandle handle) : health_monitor_(handle)
106+
HealthMonitor::HealthMonitor(FFIHandle handle) : health_monitor_(handle)
80107
{
81108
// Initialize health monitor
82109
}
@@ -87,20 +114,21 @@ HealthMonitor::HealthMonitor(HealthMonitor&& other)
87114
other.health_monitor_ = nullptr;
88115
}
89116

90-
score::cpp::expected<deadline::DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
117+
score::cpp::expected<DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
91118
{
92-
auto maybe_monitor = health_monitor_get_deadline_monitor(health_monitor_, &tag);
93-
94-
if (maybe_monitor != nullptr)
119+
FFIHandle handle{nullptr};
120+
auto result{health_monitor_get_deadline_monitor(health_monitor_, &tag, &handle)};
121+
if (result != kSuccess)
95122
{
96-
97-
return score::cpp::expected<deadline::DeadlineMonitor, Error>(deadline::DeadlineMonitor{maybe_monitor});
123+
return score::cpp::unexpected(static_cast<Error>(result));
98124
}
99125

100-
return score::cpp::unexpected(Error::NotFound);
126+
return score::cpp::expected<DeadlineMonitor, Error>(DeadlineMonitor{handle});
101127
}
128+
102129
void HealthMonitor::start()
103130
{
131+
// TODO: how errors should be handled?
104132
health_monitor_start(health_monitor_);
105133
}
106134

0 commit comments

Comments
 (0)