Skip to content

Commit

Permalink
refine errors in uniffi
Browse files Browse the repository at this point in the history
  • Loading branch information
@octol
octol committed Feb 7, 2025
1 parent 82d4fbe commit 7651ebb
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 98 deletions.
93 changes: 33 additions & 60 deletions nym-vpn-core/crates/nym-vpn-lib/src/platform/account.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,55 +237,46 @@ pub(super) async fn get_device_id() -> Result<String, VpnError> {
.map_err(VpnError::from)
}

// Raw API that does not interact with the account controller
// Raw API that directly accesses storage without going through the account controller.
// This API places the responsibility of ensuring the account controller is not running on
// the caller.
//
// WARN: This API was added mostly as a workaround for unblocking the iOS client, and is not a
// sustainable long term solution.
pub(crate) mod raw {
use std::path::Path;

use super::*;
use crate::platform::environment;
use nym_sdk::mixnet::StoragePaths;
use nym_vpn_api_client::response::NymVpnAccountResponse;
use nym_vpn_api_client::types::{Device, DeviceStatus};
use nym_vpn_api_client::VpnApiClient;
use nym_vpn_api_client::{
response::NymVpnAccountResponse,
types::{Device, DeviceStatus},
VpnApiClient,
};

use crate::{platform::environment, storage::VpnClientOnDiskStorage};

async fn setup_account_storage(
path: &str,
) -> Result<crate::storage::VpnClientOnDiskStorage, VpnError> {
use super::*;

async fn setup_account_storage(path: &str) -> Result<VpnClientOnDiskStorage, VpnError> {
assert_account_controller_not_running().await?;
let path = PathBuf::from_str(path).map_err(|err| VpnError::InvalidAccountStoragePath {
details: err.to_string(),
})?;
Ok(crate::storage::VpnClientOnDiskStorage::new(path))
Ok(VpnClientOnDiskStorage::new(path))
}

pub(crate) async fn login_raw(mnemonic: &str, path: &str) -> Result<(), VpnError> {
let mnemonic = parse_mnemonic(mnemonic).await?;
get_account_by_mnemonic_raw(mnemonic.clone()).await?;
let storage = setup_account_storage(path).await?;
storage
.store_mnemonic(mnemonic)
.await
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})?;
storage
.init_keys(None)
.await
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})?;

storage.store_mnemonic(mnemonic).await?;
storage.init_keys(None).await?;
Ok(())
}

pub(crate) async fn is_account_mnemonic_stored_raw(path: &str) -> Result<bool, VpnError> {
let storage = setup_account_storage(path).await?;
storage
.is_mnemonic_stored()
.await
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})
storage.is_mnemonic_stored().await.map_err(Into::into)
}

pub(crate) async fn get_account_id_raw(path: &str) -> Result<String, VpnError> {
Expand All @@ -304,19 +295,13 @@ pub(crate) mod raw {
.remove_mnemonic()
.await
.map(|_| true)
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})
.map_err(Into::into)
}

async fn remove_credential_storage_raw<P: AsRef<Path>>(path: P) -> Result<(), VpnError> {
let storage_paths =
StoragePaths::new_from_dir(&path).map_err(|err| VpnError::InternalError {
details: err.to_string(),
})?;

let storage_paths = StoragePaths::new_from_dir(&path).map_err(VpnError::internal)?;
std::fs::remove_file(storage_paths.credential_database_path).map_err(|err| {
VpnError::InternalError {
VpnError::Storage {
details: err.to_string(),
}
})
Expand All @@ -325,11 +310,7 @@ pub(crate) mod raw {
async fn create_vpn_api_client() -> Result<VpnApiClient, VpnError> {
let network_env = environment::current_environment_details().await?;
let user_agent = crate::util::construct_user_agent();
VpnApiClient::new(network_env.vpn_api_url(), user_agent).map_err(|e| {
VpnError::InternalError {
details: e.to_string(),
}
})
VpnApiClient::new(network_env.vpn_api_url(), user_agent).map_err(VpnError::internal)
}

async fn load_device(path: &str) -> Result<Device, VpnError> {
Expand All @@ -355,10 +336,10 @@ pub(crate) mod raw {
vpn_api_client
.update_device(&account, &device, DeviceStatus::DeleteMe)
.await
.map(|_| ())
.map_err(|err| VpnError::UnregisterDevice {
details: err.to_string(),
})?;
Ok(())
})
}

async fn get_account_by_mnemonic_raw(
Expand All @@ -380,14 +361,11 @@ pub(crate) mod raw {
details: err.to_string(),
})?;

match unregister_device_raw(path).await {
Ok(_) => {
tracing::info!("Device has been unregistered");
}
Err(error) => {
tracing::error!("Failed to unregister device: {error:?}");
}
}
unregister_device_raw(path)
.await
.inspect(|_| tracing::info!("Device has been unregistered"))
.inspect_err(|err| tracing::error!("Failed to unregister device: {err:?}"))
.ok();

// First remove the files we own directly
remove_account_mnemonic_raw(path).await?;
Expand All @@ -396,7 +374,7 @@ pub(crate) mod raw {

// Then remove the rest of the files, that we own indirectly
nym_vpn_account_controller::util::remove_files_for_account(&path_buf).map_err(|err| {
VpnError::InternalError {
VpnError::Storage {
details: err.to_string(),
}
})?;
Expand All @@ -415,11 +393,6 @@ pub(crate) mod raw {

pub(crate) async fn remove_device_identity_raw(path: &str) -> Result<(), VpnError> {
let storage = setup_account_storage(path).await?;
storage
.remove_keys()
.await
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})
storage.remove_keys().await.map_err(VpnError::internal)
}
}
13 changes: 6 additions & 7 deletions nym-vpn-core/crates/nym-vpn-lib/src/platform/environment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use super::{error::VpnError, NETWORK_ENVIRONMENT};

pub(crate) async fn init_environment(network_name: &str) -> Result<(), VpnError> {
let network = nym_vpn_network_config::Network::fetch(network_name).map_err(|err| {
VpnError::InternalError {
VpnError::NetworkConnectionError {
details: err.to_string(),
}
})?;
Expand Down Expand Up @@ -67,9 +67,7 @@ pub(crate) async fn get_account_links(locale: &str) -> Result<AccountLinks, VpnE
network
.nym_vpn_network
.try_into_parsed_links(locale, account_id.as_deref())
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})
.map_err(VpnError::internal)
})
.map(AccountLinks::from)
}
Expand All @@ -78,16 +76,17 @@ pub(crate) async fn get_account_links_raw(
path: &str,
locale: &str,
) -> Result<AccountLinks, VpnError> {
// If the account ID is not found, we are not logged in, so we don't need to pass it to the
// API. But we can still get the links that don't require an account ID.
let account_id = super::account::raw::get_account_id_raw(path).await.ok();

current_environment_details()
.await
.and_then(|network| {
network
.nym_vpn_network
.try_into_parsed_links(locale, account_id.as_deref())
.map_err(|err| VpnError::InternalError {
details: err.to_string(),
})
.map_err(VpnError::internal)
})
.map(AccountLinks::from)
}
Expand Down
82 changes: 55 additions & 27 deletions nym-vpn-core/crates/nym-vpn-lib/src/platform/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@ use nym_vpn_lib_types::AccountCommandError;

#[derive(thiserror::Error, uniffi::Error, Debug, Clone, PartialEq)]
pub enum VpnError {
#[error("{details}")]
#[error("internal error:{details}")]
InternalError { details: String },

#[error("{details}")]
#[error("storage error: {details}")]
Storage { details: String },

#[error("network error: {details}")]
NetworkConnectionError { details: String },

#[error("{details}")]
#[error("API usage error: {details}")]
InvalidStateError { details: String },

#[error("no account stored")]
Expand All @@ -35,29 +38,54 @@ pub enum VpnError {
#[error("failed to remove device from nym vpn api: {details}")]
UnregisterDevice { details: String },

#[error("failed to store account: {0}")]
StoreAccount(super::uniffi_lib_types::StoreAccountError),

#[error("sync account failed: {0}")]
SyncAccount(super::uniffi_lib_types::SyncAccountError),
#[error("failed to store account: {details}")]
StoreAccount {
#[from]
details: super::uniffi_lib_types::StoreAccountError,
},

#[error("sync device failed: {0}")]
SyncDevice(super::uniffi_lib_types::SyncDeviceError),
#[error("sync account failed: {details}")]
SyncAccount {
#[from]
details: super::uniffi_lib_types::SyncAccountError,
},
#[error("sync device failed: {details}")]
SyncDevice {
#[from]
details: super::uniffi_lib_types::SyncDeviceError,
},

#[error("device registration failed: {0}")]
RegisterDevice(super::uniffi_lib_types::RegisterDeviceError),
#[error("device registration failed: {details}")]
RegisterDevice {
#[from]
details: super::uniffi_lib_types::RegisterDeviceError,
},

#[error("failed to request zk nym")]
RequestZkNym(super::uniffi_lib_types::RequestZkNymError),
RequestZkNym {
#[from]
details: super::uniffi_lib_types::RequestZkNymError,
},

#[error("when requesting zk nym, some were reported as failed")]
RequestZkNymBundle {
successes: Vec<super::uniffi_lib_types::RequestZkNymSuccess>,
failed: Vec<super::uniffi_lib_types::RequestZkNymError>,
},

#[error("failed to forget account: {0}")]
ForgetAccount(super::uniffi_lib_types::ForgetAccountError),
#[error("failed to forget account: {details}")]
ForgetAccount {
#[from]
details: super::uniffi_lib_types::ForgetAccountError,
},
}

impl VpnError {
pub fn internal(details: impl ToString) -> Self {
Self::InternalError {
details: details.to_string(),
}
}
}

impl From<AccountCommandError> for VpnError {
Expand All @@ -67,18 +95,18 @@ impl From<AccountCommandError> for VpnError {
AccountCommandError::Internal(err) => Self::InternalError { details: err },
AccountCommandError::NoAccountStored => Self::NoAccountStored,
AccountCommandError::NoDeviceStored => Self::NoDeviceIdentity,
AccountCommandError::StoreAccount(e) => Self::StoreAccount(e.into()),
AccountCommandError::SyncAccount(e) => Self::SyncAccount(e.into()),
AccountCommandError::SyncDevice(e) => Self::SyncDevice(e.into()),
AccountCommandError::RegisterDevice(e) => Self::RegisterDevice(e.into()),
AccountCommandError::RequestZkNym(e) => Self::RequestZkNym(e.into()),
AccountCommandError::StoreAccount(e) => Self::StoreAccount { details: e.into() },
AccountCommandError::SyncAccount(e) => Self::SyncAccount { details: e.into() },
AccountCommandError::SyncDevice(e) => Self::SyncDevice { details: e.into() },
AccountCommandError::RegisterDevice(e) => Self::RegisterDevice { details: e.into() },
AccountCommandError::RequestZkNym(e) => Self::RequestZkNym { details: e.into() },
AccountCommandError::RequestZkNymBundle { successes, failed } => {
Self::RequestZkNymBundle {
successes: successes.into_iter().map(|e| e.into()).collect(),
failed: failed.into_iter().map(|e| e.into()).collect(),
}
}
AccountCommandError::ForgetAccount(e) => Self::ForgetAccount(e.into()),
AccountCommandError::ForgetAccount(e) => Self::ForgetAccount { details: e.into() },
}
}
}
Expand All @@ -91,17 +119,17 @@ impl From<crate::Error> for VpnError {
}
}

impl From<nym_gateway_directory::Error> for VpnError {
fn from(value: nym_gateway_directory::Error) -> Self {
Self::NetworkConnectionError {
impl From<nym_vpn_store::keys::persistence::OnDiskKeysError> for VpnError {
fn from(value: nym_vpn_store::keys::persistence::OnDiskKeysError) -> Self {
Self::Storage {
details: value.to_string(),
}
}
}

impl From<nym_vpn_api_client::VpnApiClientError> for VpnError {
fn from(value: nym_vpn_api_client::VpnApiClientError) -> Self {
Self::NetworkConnectionError {
impl From<nym_vpn_store::mnemonic::on_disk::OnDiskMnemonicStorageError> for VpnError {
fn from(value: nym_vpn_store::mnemonic::on_disk::OnDiskMnemonicStorageError) -> Self {
Self::Storage {
details: value.to_string(),
}
}
Expand Down
14 changes: 10 additions & 4 deletions nym-vpn-core/crates/nym-vpn-lib/src/platform/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,11 +281,14 @@ async fn get_gateway_countries(
nym_vpn_api_url,
min_gateway_performance,
};
GatewayClient::new(directory_config, user_agent.into())?
GatewayClient::new(directory_config, user_agent.into())
.map_err(VpnError::internal)?
.lookup_countries(gw_type.into())
.await
.map(|countries| countries.into_iter().map(Location::from).collect())
.map_err(VpnError::from)
.map_err(|err| VpnError::NetworkConnectionError {
details: err.to_string(),
})
}

/// Get the list of gateways available of the given type.
Expand Down Expand Up @@ -319,7 +322,8 @@ async fn get_gateways(
nym_vpn_api_url,
min_gateway_performance,
};
GatewayClient::new(directory_config, user_agent.into())?
GatewayClient::new(directory_config, user_agent.into())
.map_err(VpnError::internal)?
.lookup_gateways(gw_type.into())
.await
.map(|gateways| {
Expand All @@ -329,7 +333,9 @@ async fn get_gateways(
.map(GatewayInfo::from)
.collect()
})
.map_err(VpnError::from)
.map_err(|err| VpnError::NetworkConnectionError {
details: err.to_string(),
})
}

/// Start the VPN by first establishing that the account is ready to connect, including requesting
Expand Down

0 comments on commit 7651ebb

Please sign in to comment.