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.

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

Change safe APIs to return mssf Error #131

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 10 additions & 10 deletions crates/libs/core/src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ impl ApiTable {
}
}

pub fn fabric_get_last_error_message(&self) -> crate::Result<IFabricStringResult> {
pub fn fabric_get_last_error_message(&self) -> crate::WinResult<IFabricStringResult> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe { (self.fabric_get_last_error_message_fn)(std::ptr::addr_of_mut!(result)) }.ok()?;
assert!(!result.is_null());
Expand All @@ -186,7 +186,7 @@ impl ApiTable {
connectionstrings: &[windows_core::PCWSTR],
service_notification_handler: Option<&IFabricServiceNotificationEventHandler>,
client_connection_handler: Option<&IFabricClientConnectionEventHandler>,
) -> crate::Result<T> {
) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_create_client3_fn)(
Expand All @@ -206,7 +206,7 @@ impl ApiTable {
&self,
service_notification_handler: Option<&IFabricServiceNotificationEventHandler>,
client_connection_handler: Option<&IFabricClientConnectionEventHandler>,
) -> crate::Result<T> {
) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_create_local_client3_fn)(
Expand All @@ -225,7 +225,7 @@ impl ApiTable {
service_notification_handler: Option<&IFabricServiceNotificationEventHandler>,
client_connection_handler: Option<&IFabricClientConnectionEventHandler>,
clientrole: FABRIC_CLIENT_ROLE,
) -> crate::Result<T> {
) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_create_local_client4_fn)(
Expand All @@ -240,13 +240,13 @@ impl ApiTable {
Ok(unsafe { T::from_raw(result) })
}

pub fn fabric_create_runtime<T: Interface>(&self) -> crate::Result<T> {
pub fn fabric_create_runtime<T: Interface>(&self) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe { (self.fabric_create_runtime_fn)(&T::IID, std::ptr::addr_of_mut!(result)) }.ok()?;
Ok(unsafe { T::from_raw(result) })
}

pub fn fabric_get_activation_context<T: Interface>(&self) -> crate::Result<T> {
pub fn fabric_get_activation_context<T: Interface>(&self) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe { (self.fabric_get_activation_context_fn)(&T::IID, std::ptr::addr_of_mut!(result)) }
.ok()?;
Expand All @@ -257,7 +257,7 @@ impl ApiTable {
&self,
timeoutmilliseconds: u32,
callback: Option<&IFabricAsyncOperationCallback>,
) -> crate::Result<IFabricAsyncOperationContext> {
) -> crate::WinResult<IFabricAsyncOperationContext> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_begin_get_node_context_fn)(
Expand All @@ -273,7 +273,7 @@ impl ApiTable {
pub fn fabric_end_get_node_context<T: Interface>(
&self,
context: Option<&IFabricAsyncOperationContext>,
) -> crate::Result<T> {
) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_end_get_node_context_fn)(
Expand All @@ -285,7 +285,7 @@ impl ApiTable {
Ok(unsafe { T::from_raw(result) })
}

pub fn fabric_get_node_context<T: Interface>(&self) -> crate::Result<T> {
pub fn fabric_get_node_context<T: Interface>(&self) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe { (self.fabric_get_node_context_fn)(std::ptr::addr_of_mut!(result)) }.ok()?;
Ok(unsafe { T::from_raw(result) })
Expand All @@ -301,7 +301,7 @@ impl ApiTable {
localstorekind: FABRIC_LOCAL_STORE_KIND,
localstoresettings: *const core::ffi::c_void,
storeeventhandler: Option<&IFabricStoreEventHandler>,
) -> crate::Result<T> {
) -> crate::WinResult<T> {
let mut result = std::ptr::null_mut::<core::ffi::c_void>();
unsafe {
(self.fabric_create_key_value_store_replica_fn)(
Expand Down
12 changes: 8 additions & 4 deletions crates/libs/core/src/client/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,17 +68,21 @@ where
fn OnConnected(
&self,
gw_info: windows_core::Ref<IFabricGatewayInformationResult>,
) -> windows_core::Result<()> {
) -> crate::WinResult<()> {
let info = GatewayInformationResult::from(gw_info.unwrap());
self.inner.on_connected(&info)
self.inner
.on_connected(&info)
.map_err(crate::WinError::from)
}

fn OnDisconnected(
&self,
gw_info: windows_core::Ref<IFabricGatewayInformationResult>,
) -> windows_core::Result<()> {
) -> crate::WinResult<()> {
let info = GatewayInformationResult::from(gw_info.unwrap());
self.inner.on_disconnected(&info)
self.inner
.on_disconnected(&info)
.map_err(crate::WinError::from)
}
}

Expand Down
4 changes: 2 additions & 2 deletions crates/libs/core/src/client/health_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ impl HealthClient {
/// When a cluster is secured, the health client needs administrator permission to be able to send the reports.
/// Read more about [connecting to a cluster using the FabricClient APIs](https://learn.microsoft.com/en-us/azure/service-fabric/service-fabric-connect-to-secure-cluster).
/// For more information about health reporting, see [Service Fabric health monitoring](https://learn.microsoft.com/en-us/azure/service-fabric/service-fabric-health-introduction).
pub fn report_health(&self, health_report: &HealthReport) -> windows_core::Result<()> {
pub fn report_health(&self, health_report: &HealthReport) -> crate::Result<()> {
match health_report {
HealthReport::Invalid => {
let fabric_health_report = FABRIC_HEALTH_REPORT {
Expand Down Expand Up @@ -190,6 +190,6 @@ impl HealthClient {
};
unsafe { self.com.ReportHealth(&fabric_health_report) }
}
}
}.map_err(crate::Error::from)
}
}
2 changes: 1 addition & 1 deletion crates/libs/core/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// Licensed under the MIT License (MIT). See License.txt in the repo root for license information.
// ------------------------------------------------------------

use crate::Interface;
use connection::{ClientConnectionEventHandlerBridge, LambdaClientConnectionNotificationHandler};
use health_client::HealthClient;
use mssf_com::FabricClient::{
Expand All @@ -13,7 +14,6 @@ use notification::{
LambdaServiceNotificationHandler, ServiceNotificationEventHandler,
ServiceNotificationEventHandlerBridge,
};
use windows_core::Interface;

use crate::types::ClientRole;

Expand Down
8 changes: 5 additions & 3 deletions crates/libs/core/src/client/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl ServiceEndpointsVersion {
/// Ideally one should use mssf_core::client::svc_mgmt_client::ResolvedServicePartition instead, by
/// doing an additional FabricClient resolve call to retrieve from FabricClient cache.
pub fn compare(&self, other: &ServiceEndpointsVersion) -> crate::Result<i32> {
unsafe { self.com.Compare(&other.com) }
unsafe { self.com.Compare(&other.com) }.map_err(crate::Error::from)
}
}

Expand Down Expand Up @@ -175,10 +175,12 @@ where
fn OnNotification(
&self,
notification: windows_core::Ref<IFabricServiceNotification>,
) -> crate::Result<()> {
) -> crate::WinResult<()> {
let com = notification.unwrap();
let msg = ServiceNotification::from(com.clone());
self.inner.on_notification(&msg)
self.inner
.on_notification(&msg)
.map_err(crate::WinError::from)
}
}

Expand Down
10 changes: 5 additions & 5 deletions crates/libs/core/src/client/query_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl QueryClient {
query_description: &FABRIC_NODE_QUERY_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<::windows_core::Result<IFabricGetNodeListResult2>> {
) -> FabricReceiver2<crate::WinResult<IFabricGetNodeListResult2>> {
let com1 = &self.com;
let com2 = self.com.clone();

Expand All @@ -66,7 +66,7 @@ impl QueryClient {
desc: &FABRIC_SERVICE_PARTITION_QUERY_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<IFabricGetPartitionListResult2>> {
) -> FabricReceiver2<crate::WinResult<IFabricGetPartitionListResult2>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -83,7 +83,7 @@ impl QueryClient {
desc: &FABRIC_SERVICE_REPLICA_QUERY_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<IFabricGetReplicaListResult2>> {
) -> FabricReceiver2<crate::WinResult<IFabricGetReplicaListResult2>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -100,7 +100,7 @@ impl QueryClient {
desc: &FABRIC_PARTITION_LOAD_INFORMATION_QUERY_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<IFabricGetPartitionLoadInformationResult>> {
) -> FabricReceiver2<crate::WinResult<IFabricGetPartitionLoadInformationResult>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand Down Expand Up @@ -133,7 +133,7 @@ impl QueryClient {
desc: &NodeQueryDescription,
timeout: Duration,
cancellation_token: Option<crate::sync::CancellationToken>,
) -> windows_core::Result<NodeList> {
) -> crate::Result<NodeList> {
// Note that the SF raw structs are scoped to avoid having them across await points.
// This makes api Send. All FabricClient api should follow this pattern.
let com = {
Expand Down
23 changes: 13 additions & 10 deletions crates/libs/core/src/client/svc_mgmt_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
)]
use std::{ffi::c_void, time::Duration};

use crate::{WString, PCWSTR};
use mssf_com::{
FabricClient::{IFabricResolvedServicePartitionResult, IFabricServiceManagementClient6},
FabricTypes::{
Expand All @@ -23,7 +24,6 @@ use mssf_com::{
FABRIC_SERVICE_ROLE_STATELESS, FABRIC_URI,
},
};
use windows_core::{WString, PCWSTR};

#[cfg(feature = "tokio_async")]
use crate::sync::{fabric_begin_end_proxy2, CancellationToken, FabricReceiver2};
Expand Down Expand Up @@ -58,7 +58,7 @@ impl ServiceManagementClient {
previous_result: Option<&IFabricResolvedServicePartitionResult>, // This is different from generated code
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<IFabricResolvedServicePartitionResult>> {
) -> FabricReceiver2<crate::WinResult<IFabricResolvedServicePartitionResult>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -82,7 +82,7 @@ impl ServiceManagementClient {
desc: &FABRIC_RESTART_REPLICA_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<()>> {
) -> FabricReceiver2<crate::WinResult<()>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -99,7 +99,7 @@ impl ServiceManagementClient {
desc: &FABRIC_REMOVE_REPLICA_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<()>> {
) -> FabricReceiver2<crate::WinResult<()>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -116,7 +116,7 @@ impl ServiceManagementClient {
desc: &FABRIC_SERVICE_NOTIFICATION_FILTER_DESCRIPTION,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<i64>> {
) -> FabricReceiver2<crate::WinResult<i64>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand All @@ -133,7 +133,7 @@ impl ServiceManagementClient {
filterid: i64,
timeout_milliseconds: u32,
cancellation_token: Option<CancellationToken>,
) -> FabricReceiver2<crate::Result<()>> {
) -> FabricReceiver2<crate::WinResult<()>> {
let com1 = &self.com;
let com2 = self.com.clone();
fabric_begin_end_proxy2(
Expand Down Expand Up @@ -173,7 +173,7 @@ impl ServiceManagementClient {
prev: Option<&ResolvedServicePartition>,
timeout: Duration,
cancellation_token: Option<CancellationToken>,
) -> windows_core::Result<ResolvedServicePartition> {
) -> crate::Result<ResolvedServicePartition> {
let com = {
let uri = FABRIC_URI(name.as_ptr() as *mut u16);
// supply prev as null if not present
Expand Down Expand Up @@ -210,6 +210,7 @@ impl ServiceManagementClient {
self.restart_replica_internal(&raw, timeout.as_millis() as u32, cancellation_token)
}
.await?
.map_err(crate::Error::from)
}

/// This API gives a running replica the chance to cleanup its state and be gracefully shutdown.
Expand All @@ -228,6 +229,7 @@ impl ServiceManagementClient {
self.remove_replica_internal(&raw, timeout.as_millis() as u32, cancellation_token)
}
.await?
.map_err(crate::Error::from)
}

/// Remarks:
Expand Down Expand Up @@ -277,6 +279,7 @@ impl ServiceManagementClient {
cancellation_token,
)
.await?
.map_err(crate::Error::from)
}
}

Expand Down Expand Up @@ -420,8 +423,8 @@ impl ResolvedServicePartition {
// to enable the user to identify which RSP is more
// up-to-date. A returned value of 0 indicates that the two RSPs have the same version. 1 indicates that the other RSP has an older version.
// -1 indicates that the other RSP has a newer version.
pub fn compare_version(&self, other: &ResolvedServicePartition) -> windows_core::Result<i32> {
unsafe { self.com.CompareVersion(&other.com) }
pub fn compare_version(&self, other: &ResolvedServicePartition) -> crate::Result<i32> {
unsafe { self.com.CompareVersion(&other.com) }.map_err(crate::Error::from)
}
}

Expand Down Expand Up @@ -527,7 +530,7 @@ impl From<&FABRIC_RESOLVED_SERVICE_ENDPOINT> for ResolvedServiceEndpoint {

#[cfg(test)]
mod tests {
use windows_core::{WString, PCWSTR};
use crate::{WString, PCWSTR};

use super::{PartitionKeyType, ServicePartitionKind};

Expand Down
17 changes: 7 additions & 10 deletions crates/libs/core/src/client/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@

use std::time::Duration;

use crate::WString;
use mssf_com::FabricTypes::FABRIC_E_SERVICE_DOES_NOT_EXIST;
use tokio_util::sync::CancellationToken;
use windows_core::WString;

use crate::{
client::{svc_mgmt_client::PartitionKeyType, FabricClient},
error::FabricErrorCode,
error::ErrorCode,
types::{NodeQueryDescription, NodeStatusFilter, PagedQueryDescription},
};

Expand Down Expand Up @@ -73,7 +73,7 @@ async fn test_fabric_client() {
let list = qc.get_node_list(&desc, timeout, Some(token.clone()));
token.cancel();
let err = list.await.expect_err("request should be cancelled");
assert_eq!(err, FabricErrorCode::E_ABORT.into());
assert_eq!(err, ErrorCode::E_ABORT.into());
}

let smgr = c.get_service_manager();
Expand Down Expand Up @@ -103,17 +103,14 @@ async fn test_fabric_client() {
// FABRIC_E_SERVICE_OFFLINE is the expected result.
// TODO: Investigate the ci.
assert!(
e.code() == windows_core::HRESULT(FABRIC_E_SERVICE_DOES_NOT_EXIST.0)
|| e.code()
== windows_core::HRESULT(
e.0 == crate::HRESULT(FABRIC_E_SERVICE_DOES_NOT_EXIST.0)
|| e.0
== crate::HRESULT(
mssf_com::FabricTypes::FABRIC_E_SERVICE_OFFLINE.0
)
);
} else {
assert_eq!(
e.code(),
windows_core::HRESULT(FABRIC_E_SERVICE_DOES_NOT_EXIST.0)
);
assert_eq!(e.0, crate::HRESULT(FABRIC_E_SERVICE_DOES_NOT_EXIST.0));
println!("EchoApp not provisioned. Skip validate.")
}
}
Expand Down
Loading