Skip to content

Commit f78336e

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 f78336e

12 files changed

Lines changed: 1372 additions & 366 deletions

File tree

‎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: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,33 +11,51 @@
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

@@ -46,30 +64,30 @@ DeadlineMonitorBuilder DeadlineMonitorBuilder::add_deadline(const IdentTag& tag,
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: 59 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,42 +12,62 @@
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

4463
HealthMonitorBuilder HealthMonitorBuilder::add_deadline_monitor(const IdentTag& tag,
45-
deadline::DeadlineMonitorBuilder&& monitor) &&
64+
DeadlineMonitorBuilder&& monitor) &&
4665
{
4766
auto monitor_handle = monitor.drop_by_rust();
4867
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(monitor_handle.has_value());
4968
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle_.as_rust_handle().has_value());
5069

70+
// TODO: how errors should be handled?
5171
health_monitor_builder_add_deadline_monitor(
5272
health_monitor_builder_handle_.as_rust_handle().value(), &tag, monitor_handle.value());
5373
return std::move(*this);
@@ -65,18 +85,26 @@ HealthMonitorBuilder HealthMonitorBuilder::with_supervisor_api_cycle(std::chrono
6585
return std::move(*this);
6686
}
6787

68-
HealthMonitor HealthMonitorBuilder::build() &&
88+
score::cpp::expected<HealthMonitor, Error> HealthMonitorBuilder::build() &&
6989
{
70-
auto handle = health_monitor_builder_handle_.drop_by_rust();
71-
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(handle.has_value());
90+
auto health_monitor_builder_handle = health_monitor_builder_handle_.drop_by_rust();
91+
SCORE_LANGUAGE_FUTURECPP_PRECONDITION(health_monitor_builder_handle.has_value());
7292

7393
uint32_t supervisor_duration_ms = static_cast<uint32_t>(supervisor_api_cycle_duration_.count());
7494
uint32_t internal_duration_ms = static_cast<uint32_t>(internal_processing_cycle_duration_.count());
7595

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

79-
HealthMonitor::HealthMonitor(internal::FFIHandle handle) : health_monitor_(handle)
107+
HealthMonitor::HealthMonitor(FFIHandle handle) : health_monitor_(handle)
80108
{
81109
// Initialize health monitor
82110
}
@@ -87,20 +115,21 @@ HealthMonitor::HealthMonitor(HealthMonitor&& other)
87115
other.health_monitor_ = nullptr;
88116
}
89117

90-
score::cpp::expected<deadline::DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
118+
score::cpp::expected<DeadlineMonitor, Error> HealthMonitor::get_deadline_monitor(const IdentTag& tag)
91119
{
92-
auto maybe_monitor = health_monitor_get_deadline_monitor(health_monitor_, &tag);
93-
94-
if (maybe_monitor != nullptr)
120+
FFIHandle handle{nullptr};
121+
auto result{health_monitor_get_deadline_monitor(health_monitor_, &tag, &handle)};
122+
if (result != kSuccess)
95123
{
96-
97-
return score::cpp::expected<deadline::DeadlineMonitor, Error>(deadline::DeadlineMonitor{maybe_monitor});
124+
return score::cpp::unexpected(static_cast<Error>(result));
98125
}
99126

100-
return score::cpp::unexpected(Error::NotFound);
127+
return score::cpp::expected<DeadlineMonitor, Error>(DeadlineMonitor{handle});
101128
}
129+
102130
void HealthMonitor::start()
103131
{
132+
// TODO: how errors should be handled?
104133
health_monitor_start(health_monitor_);
105134
}
106135

0 commit comments

Comments
 (0)