diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e3eddc4b9..b7e8978b8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,8 +6,8 @@ env: PROPTEST_CASES: 32 jobs: - build_and_test: - name: OuiSync + check_and_test_on_linux: + name: check and test (linux) runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 @@ -46,3 +46,24 @@ jobs: - name: "Run cli integration tests" run: cargo test -p ouisync-cli --test cli + check_on_windows: + name: check (windows) + runs-on: windows-latest + steps: + - uses: actions/checkout@v3 + - uses: actions-rs/toolchain@v1 + with: + profile: minimal + toolchain: stable + components: clippy + - uses: actions/cache@v3 + with: + path: | + ~\cargo\bin + ~\cargo\registry\index + ~\cargo\egistry\cache + ~\cargo\git\db + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.toml') }} + - name: "Run clippy" + run: cargo clippy --all-targets diff --git a/bridge/src/config.rs b/bridge/src/config.rs index 883c6ddc8..d2f2cd539 100644 --- a/bridge/src/config.rs +++ b/bridge/src/config.rs @@ -103,7 +103,7 @@ impl ConfigEntry { } let content = serde_json::to_string_pretty(value) - .map_err(|error| io::Error::new(ErrorKind::Other, error))?; + .map_err(|error| io::Error::new(ErrorKind::InvalidInput, error))?; file.write_all(b"\n").await?; file.write_all(content.as_bytes()).await?; diff --git a/bridge/src/device_id.rs b/bridge/src/device_id.rs index d16d541a4..9f787eeed 100644 --- a/bridge/src/device_id.rs +++ b/bridge/src/device_id.rs @@ -1,7 +1,4 @@ -use crate::{ - config::{ConfigError, ConfigKey, ConfigStore}, - error::Result, -}; +use crate::config::{ConfigError, ConfigKey, ConfigStore}; use ouisync_lib::DeviceId; use rand::{rngs::OsRng, Rng}; @@ -21,7 +18,7 @@ const KEY: ConfigKey = ConfigKey::new( identification.", ); -pub async fn get_or_create(config: &ConfigStore) -> Result { +pub async fn get_or_create(config: &ConfigStore) -> Result { let cfg = config.entry(KEY); match cfg.get().await { @@ -31,7 +28,7 @@ pub async fn get_or_create(config: &ConfigStore) -> Result { cfg.set(&new_id).await?; Ok(new_id) } - Err(e) => Err(e.into()), + Err(ConfigError::Io(error)) => Err(error.into()), } } diff --git a/bridge/src/error.rs b/bridge/src/error.rs deleted file mode 100644 index 4c595f505..000000000 --- a/bridge/src/error.rs +++ /dev/null @@ -1,171 +0,0 @@ -use crate::config::ConfigError; -use num_enum::{IntoPrimitive, TryFromPrimitive}; -use serde::{Deserialize, Serialize}; -use std::io; -use thiserror::Error; - -/// A specialized `Result` type for convenience. -pub type Result = std::result::Result; - -#[derive(Debug, Error)] -pub enum Error { - #[error("{0}")] - Library(#[from] ouisync_lib::Error), - #[error("failed to initialize logger")] - InitializeLogger(#[source] io::Error), - #[error("failed to initialize runtime")] - InitializeRuntime(#[source] io::Error), - #[error("request is malformed")] - MalformedRequest(#[source] rmp_serde::decode::Error), - #[error("request failed: {message}")] - RequestFailed { code: ErrorCode, message: String }, - #[error("request is forbidden")] - ForbiddenRequest, - #[error("argument is not valid")] - InvalidArgument, - #[error("connection lost")] - ConnectionLost, - #[error("failed to read from or write into the config file")] - Config(#[from] ConfigError), - #[error("input/output error")] - Io(#[from] io::Error), -} - -pub trait ToErrorCode { - fn to_error_code(&self) -> ErrorCode; -} - -impl ToErrorCode for Error { - fn to_error_code(&self) -> ErrorCode { - match self { - Self::Library(error) => { - use ouisync_lib::Error::*; - - match error { - Db(_) | Store(_) => ErrorCode::Store, - PermissionDenied => ErrorCode::PermissionDenied, - MalformedData | MalformedDirectory => ErrorCode::MalformedData, - EntryExists => ErrorCode::EntryExists, - EntryNotFound => ErrorCode::EntryNotFound, - AmbiguousEntry => ErrorCode::AmbiguousEntry, - DirectoryNotEmpty => ErrorCode::DirectoryNotEmpty, - OperationNotSupported => ErrorCode::OperationNotSupported, - InvalidArgument | NonUtf8FileName | OffsetOutOfRange => { - ErrorCode::InvalidArgument - } - StorageVersionMismatch => ErrorCode::StorageVersionMismatch, - EntryIsFile | EntryIsDirectory | Writer(_) | Locked => ErrorCode::Other, - } - } - Self::InitializeLogger(_) | Self::InitializeRuntime(_) | Self::Io(_) => { - ErrorCode::Other - } - Self::Config(_) => ErrorCode::Config, - Self::MalformedRequest(_) => ErrorCode::MalformedRequest, - Self::RequestFailed { code, .. } => *code, - Self::InvalidArgument => ErrorCode::InvalidArgument, - Self::ConnectionLost => ErrorCode::ConnectionLost, - Self::ForbiddenRequest => ErrorCode::ForbiddenRequest, - } - } -} - -#[derive( - Copy, Clone, Eq, PartialEq, Serialize, Deserialize, Debug, IntoPrimitive, TryFromPrimitive, -)] -#[repr(u16)] -#[serde(into = "u16", try_from = "u16")] -pub enum ErrorCode { - /// No error - Ok = 0, - /// Store error - Store = 1, - /// Insuficient permission to perform the intended operation - PermissionDenied = 2, - /// Malformed data - MalformedData = 3, - /// Entry already exists - EntryExists = 4, - /// Entry doesn't exist - EntryNotFound = 5, - /// Multiple matching entries found - AmbiguousEntry = 6, - /// The intended operation requires the directory to be empty but it isn't - DirectoryNotEmpty = 7, - /// The indended operation is not supported - OperationNotSupported = 8, - /// Failed to read from or write into the config file - Config = 10, - /// Argument passed to a function is not valid - InvalidArgument = 11, - /// Interface request is malformed - MalformedRequest = 12, - /// Storage format version mismatch - StorageVersionMismatch = 13, - /// Connection lost - ConnectionLost = 14, - /// Request is forbidden - ForbiddenRequest = 15, - - /// Failed to parse the mount point string - VfsFailedToParseMountPoint = 2048, - - /// Mounting is not yes supported on this Operating System - VfsUnsupportedOs = 2048 + 1, - - // These are equivalents of the dokan::file_system::FileSystemMountError errors - // https://github.com/dokan-dev/dokan-rust/blob/master/dokan/src/file_system.rs - /// A general error - VfsGeneral = 2048 + 3, - /// Bad drive letter - VfsDriveLetter = 2048 + 4, - /// Can't install the Dokan driver. - VfsDriverInstall = 2048 + 5, - /// The driver responds that something is wrong. - VfsStart = 2048 + 2, - /// Can't assign a drive letter or mount point. - /// - /// This probably means that the mount point is already used by another volume. - VfsMount = 2048 + 6, - /// The mount point is invalid. - VfsMountPoint = 2048 + 7, - /// The Dokan version that this wrapper is targeting is incompatible with the loaded Dokan - /// library. - VfsVersion = 2048 + 8, - - /// Unspecified error - Other = 65535, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn error_code_serialize_deserialize() { - let origs = [ - ErrorCode::Ok, - ErrorCode::Store, - ErrorCode::PermissionDenied, - ErrorCode::MalformedData, - ErrorCode::EntryExists, - ErrorCode::EntryNotFound, - ErrorCode::AmbiguousEntry, - ErrorCode::DirectoryNotEmpty, - ErrorCode::OperationNotSupported, - ErrorCode::Config, - ErrorCode::InvalidArgument, - ErrorCode::MalformedRequest, - ErrorCode::StorageVersionMismatch, - ErrorCode::ConnectionLost, - ErrorCode::ForbiddenRequest, - ErrorCode::Other, - ]; - - for orig in origs { - let encoded = rmp_serde::to_vec(&orig).unwrap(); - let decoded: ErrorCode = rmp_serde::from_slice(&encoded).unwrap(); - assert_eq!(decoded, orig); - } - } -} diff --git a/bridge/src/lib.rs b/bridge/src/lib.rs index ef75ed074..4304ab659 100644 --- a/bridge/src/lib.rs +++ b/bridge/src/lib.rs @@ -2,7 +2,6 @@ pub mod config; pub mod constants; pub mod device_id; pub mod dht_contacts; -pub mod error; pub mod logger; pub mod network; pub mod protocol; diff --git a/bridge/src/logger/default.rs b/bridge/src/logger/default.rs index 066359eee..2ed9ac9b2 100644 --- a/bridge/src/logger/default.rs +++ b/bridge/src/logger/default.rs @@ -1,10 +1,8 @@ use super::{common, LogFormat}; use ouisync_tracing_fmt::Formatter; -use std::{ - io::{self, IsTerminal}, - path::Path, - sync::Mutex, -}; +#[cfg(not(target_os = "windows"))] +use std::io::IsTerminal; +use std::{io, path::Path, sync::Mutex}; use tracing::{ metadata::LevelFilter, span::{Attributes, Record}, diff --git a/bridge/src/logger/mod.rs b/bridge/src/logger/mod.rs index 625bddafc..2dde60466 100644 --- a/bridge/src/logger/mod.rs +++ b/bridge/src/logger/mod.rs @@ -6,11 +6,10 @@ mod default; mod common; -use crate::error::{Error, Result}; use ouisync_lib::StateMonitor; use serde::{Deserialize, Serialize}; use std::{ - fmt, fs, + fmt, fs, io, panic::{self, PanicInfo}, path::Path, str::FromStr, @@ -31,9 +30,9 @@ impl Logger { path: Option<&Path>, root_monitor: Option, format: LogFormat, - ) -> Result { + ) -> Result { if let Some(parent) = path.and_then(|path| path.parent()) { - fs::create_dir_all(parent).map_err(Error::InitializeLogger)?; + fs::create_dir_all(parent)?; } let inner = Inner::new(path, format)?; diff --git a/bridge/src/network.rs b/bridge/src/network.rs index 9b2f9a341..cfe61084a 100644 --- a/bridge/src/network.rs +++ b/bridge/src/network.rs @@ -1,7 +1,4 @@ -use crate::{ - config::{ConfigKey, ConfigStore}, - error::{Error, Result}, -}; +use crate::config::{ConfigKey, ConfigStore}; use ouisync_lib::network::{peer_addr::PeerAddr, Network}; use serde::{Deserialize, Serialize}; use std::{io, net::SocketAddr, num::ParseIntError}; @@ -167,10 +164,10 @@ pub async fn remove_user_provided_peers( /// it and don't have to wait for it to be discovered (e.g. on the DHT). /// /// NOTE: Currently this is not persisted. -pub async fn add_storage_server(network: &Network, host: &str) -> Result<()> { - let (hostname, port) = split_port(host).map_err(|_| { +pub async fn add_storage_server(network: &Network, host: &str) -> Result<(), io::Error> { + let (hostname, port) = split_port(host).map_err(|error| { tracing::error!(host, "invalid storage server host"); - Error::InvalidArgument + io::Error::new(io::ErrorKind::InvalidInput, error) })?; let port = port.unwrap_or(DEFAULT_STORAGE_SERVER_PORT); diff --git a/bridge/src/protocol/mod.rs b/bridge/src/protocol/mod.rs index 70271496c..6943a2c85 100644 --- a/bridge/src/protocol/mod.rs +++ b/bridge/src/protocol/mod.rs @@ -1,32 +1,22 @@ pub mod remote; -use crate::{ - constants::{NETWORK_EVENT_PEER_SET_CHANGE, NETWORK_EVENT_PROTOCOL_VERSION_MISMATCH}, - error::{Error, ErrorCode, Result, ToErrorCode}, -}; +use crate::constants::{NETWORK_EVENT_PEER_SET_CHANGE, NETWORK_EVENT_PROTOCOL_VERSION_MISMATCH}; use num_enum::{IntoPrimitive, TryFromPrimitive}; use serde::{Deserialize, Serialize}; #[derive(Eq, PartialEq, Debug, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] -pub(crate) enum ServerMessage { +pub(crate) enum ServerMessage { Success(T), - Failure { code: ErrorCode, message: String }, + Failure(E), Notification(Notification), } -impl ServerMessage { - pub fn response(result: Result) -> Self { +impl ServerMessage { + pub fn response(result: Result) -> Self { match result { Ok(response) => Self::Success(response), - Err(error) => Self::Failure { - code: error.to_error_code(), - // TODO: include also sources - message: match error { - Error::Io(inner) => inner.to_string(), - _ => error.to_string(), - }, - }, + Err(error) => Self::Failure(error), } } @@ -56,8 +46,6 @@ pub enum NetworkEvent { #[cfg(test)] mod tests { use super::*; - use crate::error::Error; - use std::io; #[test] fn server_message_serialize_deserialize() { @@ -67,21 +55,25 @@ mod tests { Bool(bool), } + #[derive(Eq, PartialEq, Debug, Serialize, Deserialize)] + enum TestError { + ForbiddenRequest, + Io, + } + let origs = [ ServerMessage::response(Ok(TestResponse::None)), ServerMessage::response(Ok(TestResponse::Bool(true))), ServerMessage::response(Ok(TestResponse::Bool(false))), - ServerMessage::response(Err(Error::ForbiddenRequest)), - ServerMessage::response(Err(Error::Io(io::Error::new( - io::ErrorKind::Other, - "something went wrong", - )))), + ServerMessage::response(Err(TestError::ForbiddenRequest)), + ServerMessage::response(Err(TestError::Io)), ]; for orig in origs { let encoded = rmp_serde::to_vec(&orig).unwrap(); println!("{encoded:?}"); - let decoded: ServerMessage = rmp_serde::from_slice(&encoded).unwrap(); + let decoded: ServerMessage = + rmp_serde::from_slice(&encoded).unwrap(); assert_eq!(decoded, orig); } } diff --git a/bridge/src/protocol/remote.rs b/bridge/src/protocol/remote.rs index 16f02837d..ec0f91056 100644 --- a/bridge/src/protocol/remote.rs +++ b/bridge/src/protocol/remote.rs @@ -1,5 +1,7 @@ +use crate::transport::TransportError; use ouisync_lib::ShareToken; use serde::{Deserialize, Serialize}; +use thiserror::Error; #[derive(Debug, Serialize, Deserialize)] pub enum Request { @@ -17,3 +19,15 @@ impl From<()> for Response { Self::None } } + +#[derive(Error, Debug, Serialize, Deserialize)] +pub enum ServerError { + #[error("server is shutting down")] + ShuttingDown, + #[error("invalid argument")] + InvalidArgument, + #[error("transport error")] + Transport(#[from] TransportError), + #[error("failed to create repository: {0}")] + CreateRepository(String), +} diff --git a/bridge/src/repository.rs b/bridge/src/repository.rs index 41b9bf162..c0f9a965c 100644 --- a/bridge/src/repository.rs +++ b/bridge/src/repository.rs @@ -1,8 +1,7 @@ use crate::{ config::{ConfigError, ConfigKey, ConfigStore}, device_id, - error::{Error, Result}, - protocol::remote::{Request, Response}, + protocol::remote::{Request, Response, ServerError}, transport::RemoteClient, }; use camino::Utf8PathBuf; @@ -11,7 +10,8 @@ use ouisync_lib::{ crypto::Password, Access, AccessMode, AccessSecrets, LocalSecret, ReopenToken, Repository, RepositoryParams, ShareToken, StateMonitor, StorageSize, }; -use std::{borrow::Cow, sync::Arc, time::Duration}; +use std::{borrow::Cow, io, sync::Arc, time::Duration}; +use thiserror::Error; use tokio_rustls::rustls; const DEFAULT_QUOTA_KEY: ConfigKey = ConfigKey::new("default_quota", "Default storage quota"); @@ -20,6 +20,22 @@ const DEFAULT_BLOCK_EXPIRATION_MILLIS: ConfigKey = ConfigKey::new( "Default time in seconds when blocks start to expire if not used", ); +#[derive(Debug, Error)] +pub enum OpenError { + #[error("config error")] + Config(#[from] ConfigError), + #[error("repository error")] + Repository(#[from] ouisync_lib::Error), +} + +#[derive(Debug, Error)] +pub enum MirrorError { + #[error("failed to connect to server")] + Connect(#[source] io::Error), + #[error("server responded with error")] + Server(#[source] ServerError), +} + /// Creates a new repository and set access to it based on the following table: /// /// local_read_password | local_write_password | token access | result @@ -32,13 +48,14 @@ const DEFAULT_BLOCK_EXPIRATION_MILLIS: ConfigKey = ConfigKey::new( /// None | any | write | read without password, require password for writing /// any | any | write | read with password, write with (same or different) password pub async fn create( + // TODO: does this need to be utf8? store: Utf8PathBuf, local_read_password: Option, local_write_password: Option, share_token: Option, config: &ConfigStore, repos_monitor: &StateMonitor, -) -> Result { +) -> Result { let params = RepositoryParams::new(store.into_std_path_buf()) .with_device_id(device_id::get_or_create(config).await?) .with_parent_monitor(repos_monitor.clone()); @@ -69,11 +86,12 @@ pub async fn create( /// Opens an existing repository. pub async fn open( + // TODO: does this need to be utf8? store: Utf8PathBuf, local_password: Option, config: &ConfigStore, repos_monitor: &StateMonitor, -) -> Result { +) -> Result { let params = RepositoryParams::new(store.into_std_path_buf()) .with_device_id(device_id::get_or_create(config).await?) .with_parent_monitor(repos_monitor.clone()); @@ -91,7 +109,7 @@ pub async fn reopen( store: Utf8PathBuf, token: Vec, repos_monitor: &StateMonitor, -) -> Result { +) -> Result { let params = RepositoryParams::new(store.into_std_path_buf()).with_parent_monitor(repos_monitor.clone()); let token = ReopenToken::decode(&token)?; @@ -114,7 +132,7 @@ pub async fn set_read_access( repository: &Repository, local_read_password: Option, share_token: Option, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { // If None, repository shall attempt to use the one it's currently using. let access_secrets = share_token.map(ShareToken::into_secrets); @@ -150,7 +168,7 @@ pub async fn set_read_and_write_access( local_old_rw_password: Option, local_new_rw_password: Option, share_token: Option, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { // If None, repository shall attempt to use the one it's currently using. let access_secrets = share_token.map(ShareToken::into_secrets); @@ -180,7 +198,7 @@ pub async fn create_share_token( password: Option, access_mode: AccessMode, name: Option, -) -> Result { +) -> Result { let password = password.map(Password::from); let access_secrets = if let Some(password) = password { @@ -203,7 +221,10 @@ pub async fn create_share_token( Ok(share_token.to_string()) } -pub async fn set_default_quota(config: &ConfigStore, value: Option) -> Result<()> { +pub async fn set_default_quota( + config: &ConfigStore, + value: Option, +) -> Result<(), ConfigError> { let entry = config.entry(DEFAULT_QUOTA_KEY); if let Some(value) = value { @@ -215,25 +236,25 @@ pub async fn set_default_quota(config: &ConfigStore, value: Option) Ok(()) } -pub async fn get_default_quota(config: &ConfigStore) -> Result> { +pub async fn get_default_quota(config: &ConfigStore) -> Result, ConfigError> { let entry = config.entry(DEFAULT_QUOTA_KEY); match entry.get().await { Ok(quota) => Ok(Some(StorageSize::from_bytes(quota))), Err(ConfigError::NotFound) => Ok(None), - Err(error) => Err(error.into()), + Err(error) => Err(error), } } pub async fn set_default_block_expiration( config: &ConfigStore, value: Option, -) -> Result<()> { +) -> Result<(), ConfigError> { let entry = config.entry(DEFAULT_BLOCK_EXPIRATION_MILLIS); if let Some(value) = value { entry - .set(&u64::try_from(value.as_millis()).map_err(|_| Error::InvalidArgument)?) + .set(&value.as_millis().try_into().unwrap_or(u64::MAX)) .await?; } else { entry.remove().await?; @@ -242,13 +263,15 @@ pub async fn set_default_block_expiration( Ok(()) } -pub async fn get_default_block_expiration(config: &ConfigStore) -> Result> { +pub async fn get_default_block_expiration( + config: &ConfigStore, +) -> Result, ConfigError> { let entry = config.entry::(DEFAULT_BLOCK_EXPIRATION_MILLIS); match entry.get().await { Ok(millis) => Ok(Some(Duration::from_millis(millis))), Err(ConfigError::NotFound) => Ok(None), - Err(error) => Err(error.into()), + Err(error) => Err(error), } } @@ -257,7 +280,7 @@ pub async fn mirror( repository: &Repository, client_config: Arc, hosts: &[String], -) -> Result<()> { +) -> Result<(), MirrorError> { let share_token = repository.secrets().with_mode(AccessMode::Blind); let tasks = hosts.iter().map(|host| { @@ -270,8 +293,9 @@ pub async fn mirror( async move { let client = RemoteClient::connect(host, client_config) .await + .map_err(MirrorError::Connect) .map_err(|error| { - tracing::error!(host, ?error, "failed to connect to the storage server"); + tracing::error!(host, ?error, "mirror request failed"); error })?; @@ -279,7 +303,7 @@ pub async fn mirror( share_token: share_token.into(), }; - match client.invoke(request).await { + match client.invoke(request).await.map_err(MirrorError::Server) { Ok(Response::None) => { tracing::info!(host, "mirror request successfull"); Ok(()) diff --git a/bridge/src/transport/mod.rs b/bridge/src/transport/mod.rs index c623cb401..5fff8aeb0 100644 --- a/bridge/src/transport/mod.rs +++ b/bridge/src/transport/mod.rs @@ -6,21 +6,32 @@ pub use self::{ socket::{server_connection as socket_server_connection, SocketClient}, }; -use crate::{error::Result, protocol::Notification}; +use crate::protocol::Notification; use async_trait::async_trait; -use serde::{de::DeserializeOwned, Serialize}; +use serde::{de::DeserializeOwned, Deserialize, Serialize}; +use thiserror::Error; use tokio::sync::mpsc; #[async_trait] pub trait Handler: Clone + Send + Sync + 'static { type Request: DeserializeOwned + Send; type Response: Serialize + Send; + type Error: From + Serialize + Send; async fn handle( &self, request: Self::Request, notification_tx: &NotificationSender, - ) -> Result; + ) -> Result; } pub type NotificationSender = mpsc::Sender<(u64, Notification)>; + +#[derive(Debug, Error, Serialize, Deserialize)] +#[error("malformed message")] +pub enum TransportError { + #[error("connection lost")] + ConnectionLost, + #[error("malformed message")] + MalformedMessage, +} diff --git a/bridge/src/transport/remote.rs b/bridge/src/transport/remote.rs index 4b27e17ed..292772485 100644 --- a/bridge/src/transport/remote.rs +++ b/bridge/src/transport/remote.rs @@ -1,10 +1,7 @@ //! Client and Server than run on different devices. use super::{socket_server_connection, Handler, SocketClient}; -use crate::{ - error::Result, - protocol::remote::{Request, Response}, -}; +use crate::protocol::remote::{Request, Response, ServerError}; use bytes::{Bytes, BytesMut}; use futures_util::{SinkExt, StreamExt}; use std::{ @@ -36,7 +33,7 @@ const MAX_VERSION: u64 = 0; pub fn make_server_config( cert_chain: Vec, key: rustls::PrivateKey, -) -> Result> { +) -> io::Result> { make_server_config_with_versions(cert_chain, key, MIN_VERSION..=MAX_VERSION) } @@ -44,7 +41,7 @@ fn make_server_config_with_versions( cert_chain: Vec, key: rustls::PrivateKey, versions: RangeInclusive, -) -> Result> { +) -> io::Result> { let mut config = rustls::ServerConfig::builder() .with_safe_defaults() .with_no_client_auth() @@ -64,14 +61,14 @@ fn make_server_config_with_versions( /// Shared config for `RemoteClient` pub fn make_client_config( additional_root_certs: &[rustls::Certificate], -) -> Result> { +) -> io::Result> { make_client_config_with_versions(additional_root_certs, MIN_VERSION..=MAX_VERSION) } fn make_client_config_with_versions( additional_root_certs: &[rustls::Certificate], versions: RangeInclusive, -) -> Result> { +) -> io::Result> { let mut root_cert_store = rustls::RootCertStore::empty(); // Add default root certificates @@ -130,7 +127,10 @@ impl RemoteServer { self.local_addr } - pub async fn run(self, handler: H) { + pub async fn run(self, handler: H) + where + H: Handler, + { let mut connections = JoinSet::new(); loop { @@ -175,7 +175,7 @@ async fn run_connection(stream: TcpStream, tls_acceptor: TlsAcceptor } pub struct RemoteClient { - inner: SocketClient>, Request, Response>, + inner: SocketClient>, Request, Response, ServerError>, } impl RemoteClient { @@ -199,7 +199,7 @@ impl RemoteClient { Ok(Self { inner }) } - pub async fn invoke(&self, request: Request) -> Result { + pub async fn invoke(&self, request: Request) -> Result { self.inner.invoke(request).await } } @@ -377,8 +377,13 @@ mod tests { impl Handler for TestHandler { type Request = Request; type Response = Response; + type Error = ServerError; - async fn handle(&self, _: Self::Request, _: &NotificationSender) -> Result { + async fn handle( + &self, + _: Self::Request, + _: &NotificationSender, + ) -> Result { self.received.fetch_add(1, Ordering::Relaxed); Ok(Response::None) } diff --git a/bridge/src/transport/socket.rs b/bridge/src/transport/socket.rs index 5a26db9c4..1406e1cfe 100644 --- a/bridge/src/transport/socket.rs +++ b/bridge/src/transport/socket.rs @@ -1,11 +1,8 @@ //! Low-level Client and Server tha wraps Stream/Sink of bytes. Used to implement some higher-level //! clients/servers -use super::Handler; -use crate::{ - error::{Error, ErrorCode, Result}, - protocol::ServerMessage, -}; +use super::{Handler, TransportError}; +use crate::protocol::ServerMessage; use bytes::{Bytes, BytesMut}; use futures_util::{stream::FuturesUnordered, Sink, SinkExt, Stream, StreamExt, TryStreamExt}; use serde::{de::DeserializeOwned, Serialize}; @@ -40,7 +37,7 @@ pub mod server_connection { let task = async move { let result = match result { Ok(request) => handler.handle(request, notification_tx).await, - Err(error) => Err(error), + Err(error) => Err(error.into()), }; (id, result) @@ -51,7 +48,7 @@ pub mod server_connection { notification = notification_rx.recv() => { // unwrap is OK because the sender exists at this point. let (id, notification) = notification.unwrap(); - let message = ServerMessage::::notification(notification); + let message = ServerMessage::::notification(notification); send(&mut socket, id, message).await; } Some((id, result)) = request_handlers.next() => { @@ -63,12 +60,12 @@ pub mod server_connection { } } -pub struct SocketClient { - request_tx: mpsc::Sender<(Request, oneshot::Sender>)>, +pub struct SocketClient { + request_tx: mpsc::Sender<(Request, oneshot::Sender>)>, _socket: PhantomData, } -impl SocketClient +impl SocketClient where Socket: Stream> + Sink @@ -77,6 +74,7 @@ where + 'static, Request: Serialize + Send + 'static, Response: DeserializeOwned + Send + 'static, + Error: From + DeserializeOwned + Send + 'static, { pub fn new(socket: Socket) -> Self { let (request_tx, request_rx) = mpsc::channel(1); @@ -89,37 +87,41 @@ where } } - pub async fn invoke(&self, request: Request) -> Result { + pub async fn invoke(&self, request: Request) -> Result { let (response_tx, response_rx) = oneshot::channel(); self.request_tx .send((request, response_tx)) .await - .map_err(|_| Error::ConnectionLost)?; + .map_err(|_| TransportError::ConnectionLost)?; - match response_rx.await.map_err(|_| Error::ConnectionLost) { + match response_rx + .await + .map_err(|_| TransportError::ConnectionLost) + { Ok(result) => result, - Err(error) => Err(error), + Err(error) => Err(error.into()), } } } -struct Worker { +struct Worker { running: bool, - request_rx: mpsc::Receiver<(Request, oneshot::Sender>)>, + request_rx: mpsc::Receiver<(Request, oneshot::Sender>)>, socket: Socket, - pending_requests: HashMap>>, + pending_requests: HashMap>>, next_message_id: u64, } -impl Worker +impl Worker where Socket: Stream> + Sink + Unpin + Send, Request: Serialize, Response: DeserializeOwned, + Error: From + DeserializeOwned, { fn new( - request_rx: mpsc::Receiver<(Request, oneshot::Sender>)>, + request_rx: mpsc::Receiver<(Request, oneshot::Sender>)>, socket: Socket, ) -> Self { Self { @@ -147,7 +149,7 @@ where async fn handle_request( &mut self, - request: Option<(Request, oneshot::Sender>)>, + request: Option<(Request, oneshot::Sender>)>, ) { let Some((request, response_tx)) = request else { self.running = false; @@ -165,7 +167,7 @@ where async fn handle_server_message( &mut self, - message: Option<(u64, Result>)>, + message: Option<(u64, ServerMessageResult)>, ) { let Some((message_id, message)) = message else { self.running = false; @@ -179,24 +181,22 @@ where let response = match message { Ok(ServerMessage::Success(response)) => Ok(response), - Ok(ServerMessage::Failure { code, message }) => { - Err(Error::RequestFailed { code, message }) + Ok(ServerMessage::Failure(error)) => Err(error), + Ok(ServerMessage::Notification(_)) => { + tracing::error!("notifications are not supported yet"); + return; } - Ok(ServerMessage::Notification(_)) => Err(Error::RequestFailed { - code: ErrorCode::OperationNotSupported, - message: "notifications not supported yet".to_owned(), - }), - Err(error) => Err(error), + Err(error) => Err(error.into()), }; response_tx.send(response).ok(); } } -async fn receive(reader: &mut R) -> Option<(u64, Result)> +async fn receive(reader: &mut R) -> Option<(u64, Result)> where R: Stream> + Unpin, - M: DeserializeOwned, + T: DeserializeOwned, { loop { let buffer = match reader.try_next().await { @@ -220,7 +220,7 @@ where let body = rmp_serde::from_slice(&buffer[8..]).map_err(|error| { tracing::error!(?error, "failed to decode message body"); - Error::MalformedRequest(error) + TransportError::MalformedMessage }); return Some((id, body)); @@ -249,3 +249,5 @@ where true } + +type ServerMessageResult = Result, TransportError>; diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 8344c5a07..19a5acbd2 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -35,6 +35,7 @@ pem = "2.0.1" rustls = { workspace = true } scoped_task = { path = "../scoped_task" } serde = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true, features = ["signal", "io-std"] } tokio-stream = { workspace = true } tokio-util = { workspace = true, features = ["codec", "compat"] } diff --git a/cli/src/client.rs b/cli/src/client.rs index 634d80721..b9743b2c3 100644 --- a/cli/src/client.rs +++ b/cli/src/client.rs @@ -1,7 +1,7 @@ use crate::{ handler::local::LocalHandler, options::Dirs, - protocol::{Request, Response}, + protocol::{Error, Request, Response}, state::State, transport::{local::LocalClient, native::NativeClient}, }; @@ -85,7 +85,7 @@ enum Client { } impl Client { - async fn invoke(&self, request: Request) -> ouisync_bridge::error::Result { + async fn invoke(&self, request: Request) -> Result { match self { Self::Local(client) => client.invoke(request).await, Self::Native(client) => client.invoke(request).await, diff --git a/cli/src/geo_ip.rs b/cli/src/geo_ip.rs index bf76544f4..33c772386 100644 --- a/cli/src/geo_ip.rs +++ b/cli/src/geo_ip.rs @@ -1,7 +1,6 @@ //! IP geolocation use maxminddb::{geoip2, MaxMindDBError, Reader}; -use ouisync_bridge::error::{Error, Result}; use std::{ fmt, io, net::IpAddr, @@ -24,7 +23,7 @@ impl GeoIp { } } - pub async fn refresh(&mut self) -> Result<()> { + pub async fn refresh(&mut self) -> Result<(), io::Error> { let mut file = File::open(&self.path).await?; let new_timestamp = file.metadata().await?.modified()?; @@ -89,19 +88,19 @@ impl fmt::Display for CountryCode { pub(crate) enum LookupError { NotFound, - Fatal(Error), + Io(io::Error), } impl From for LookupError { fn from(src: MaxMindDBError) -> Self { match src { MaxMindDBError::AddressNotFoundError(_) => Self::NotFound, - _ => Self::Fatal(io::Error::new(io::ErrorKind::Other, src).into()), + _ => Self::Io(io::Error::new(io::ErrorKind::Other, src)), } } } -async fn load(file: &mut File) -> Result>> { +async fn load(file: &mut File) -> Result>, io::Error> { let mut content = Vec::new(); file.read_to_end(&mut content).await?; diff --git a/cli/src/handler/local.rs b/cli/src/handler/local.rs index 9dc278b5c..89aba15c3 100644 --- a/cli/src/handler/local.rs +++ b/cli/src/handler/local.rs @@ -1,14 +1,10 @@ use crate::{ - protocol::{QuotaInfo, Request, Response}, + protocol::{Error, QuotaInfo, Request, Response}, repository::{self, RepositoryHolder, RepositoryName, OPEN_ON_START}, state::State, }; use async_trait::async_trait; -use ouisync_bridge::{ - error::{Error, Result}, - network, - transport::NotificationSender, -}; +use ouisync_bridge::{network, transport::NotificationSender}; use ouisync_lib::{PeerAddr, ShareToken}; use std::{net::SocketAddr, sync::Arc, time::Duration}; @@ -31,14 +27,15 @@ impl LocalHandler { impl ouisync_bridge::transport::Handler for LocalHandler { type Request = Request; type Response = Response; + type Error = Error; async fn handle( &self, request: Self::Request, _notification_tx: &NotificationSender, - ) -> Result { + ) -> Result { match request { - Request::Start { .. } => Err(Error::ForbiddenRequest), + Request::Start { .. } => unimplemented!(), Request::BindRpc { addrs } => Ok(self .state .rpc_servers @@ -62,7 +59,7 @@ impl ouisync_bridge::transport::Handler for LocalHandler { .as_deref() .map(str::parse::) .transpose() - .map_err(|_| Error::InvalidArgument)?; + .map_err(|error| Error::new(format!("invalid share token: {error}")))?; let name = match (name, &share_token) { (Some(name), _) => name, @@ -81,7 +78,9 @@ impl ouisync_bridge::transport::Handler for LocalHandler { let write_password = write_password.or(password); let repository = ouisync_bridge::repository::create( - store_path.try_into().map_err(|_| Error::InvalidArgument)?, + store_path.try_into().map_err(|error| { + Error::new(format!("invalid repository store path: {error}")) + })?, read_password, write_password, share_token, @@ -112,9 +111,7 @@ impl ouisync_bridge::transport::Handler for LocalHandler { Request::Delete { name } => { self.state.repositories.remove(&name); - repository::delete_store(&self.state.store_dir, &name) - .await - .map_err(Error::Io)?; + repository::delete_store(&self.state.store_dir, &name).await?; Ok(().into()) } @@ -128,7 +125,9 @@ impl ouisync_bridge::transport::Handler for LocalHandler { let store_path = self.state.store_path(&name); let repository = ouisync_bridge::repository::open( - store_path.try_into().map_err(|_| Error::InvalidArgument)?, + store_path.try_into().map_err(|error| { + Error::new(format!("invalid repository store path: {error}")) + })?, password, &self.state.config, &self.state.repositories_monitor, @@ -176,22 +175,23 @@ impl ouisync_bridge::transport::Handler for LocalHandler { password, } => { let holder = self.state.repositories.find(&name)?; - - ouisync_bridge::repository::create_share_token( + let token = ouisync_bridge::repository::create_share_token( &holder.repository, password, mode, Some(name), ) - .await - .map(Into::into) + .await?; + + Ok(token.into()) } Request::Mount { name, path, all: _ } => { if let Some(name) = name { let holder = self.state.repositories.find(&name)?; let mount_point = if let Some(path) = &path { - path.to_str().ok_or(Error::InvalidArgument)? + path.to_str() + .ok_or_else(|| Error::new("invalid mount point"))? } else { "" }; diff --git a/cli/src/handler/remote.rs b/cli/src/handler/remote.rs index d18d7cd02..637858298 100644 --- a/cli/src/handler/remote.rs +++ b/cli/src/handler/remote.rs @@ -4,8 +4,7 @@ use crate::{ }; use async_trait::async_trait; use ouisync_bridge::{ - error::{Error, Result}, - protocol::remote::{Request, Response}, + protocol::remote::{Request, Response, ServerError}, transport::NotificationSender, }; use ouisync_lib::{AccessMode, RepositoryId, ShareToken}; @@ -31,18 +30,18 @@ impl RemoteHandler { impl ouisync_bridge::transport::Handler for RemoteHandler { type Request = Request; type Response = Response; + type Error = ServerError; async fn handle( &self, request: Self::Request, _notification_tx: &NotificationSender, - ) -> Result { + ) -> Result { tracing::debug!(?request); let Some(state) = self.state.upgrade() else { tracing::error!("can't handle request - shutting down"); - // TODO: return more appropriate error (ShuttingDown or similar) - return Err(Error::ForbiddenRequest); + return Err(ServerError::ShuttingDown); }; match request { @@ -63,14 +62,17 @@ impl ouisync_bridge::transport::Handler for RemoteHandler { let store_path = state.store_path(name.as_ref()); let repository = ouisync_bridge::repository::create( - store_path.try_into().map_err(|_| Error::InvalidArgument)?, + store_path + .try_into() + .map_err(|_| ServerError::InvalidArgument)?, None, None, Some(share_token), &state.config, &state.repositories_monitor, ) - .await?; + .await + .map_err(|error| ServerError::CreateRepository(error.to_string()))?; tracing::info!(%name, "repository created"); diff --git a/cli/src/metrics.rs b/cli/src/metrics.rs index 93aabb5b7..926215548 100644 --- a/cli/src/metrics.rs +++ b/cli/src/metrics.rs @@ -2,6 +2,7 @@ use crate::{ geo_ip::{CountryCode, GeoIp}, state::State, }; +use anyhow::Result; use hyper::{ server::{conn::AddrIncoming, Server}, service::{make_service_fn, service_fn}, @@ -10,10 +11,7 @@ use hyper::{ use hyper_rustls::TlsAcceptor; use metrics::{Gauge, Key, KeyName, Label, Recorder, Unit}; use metrics_exporter_prometheus::{PrometheusBuilder, PrometheusRecorder}; -use ouisync_bridge::{ - config::{ConfigError, ConfigKey}, - error::Error, -}; +use ouisync_bridge::config::{ConfigError, ConfigKey}; use ouisync_lib::{network::PeerState, PeerInfoCollector, PublicRuntimeId}; use scoped_task::ScopedAbortHandle; use std::{ @@ -47,7 +45,7 @@ impl MetricsServer { } } - pub async fn init(&self, state: &State) -> Result<(), Error> { + pub async fn init(&self, state: &State) -> Result<()> { let entry = state.config.entry(BIND_METRICS_KEY); let addr = match entry.get().await { @@ -64,7 +62,7 @@ impl MetricsServer { Ok(()) } - pub async fn bind(&self, state: &State, addr: Option) -> Result<(), Error> { + pub async fn bind(&self, state: &State, addr: Option) -> Result<()> { let entry = state.config.entry(BIND_METRICS_KEY); if let Some(addr) = addr { @@ -84,7 +82,7 @@ impl MetricsServer { } } -async fn start(state: &State, addr: SocketAddr) -> Result { +async fn start(state: &State, addr: SocketAddr) -> Result { let recorder = PrometheusBuilder::new().build_recorder(); let recorder_handle = recorder.handle(); diff --git a/cli/src/protocol.rs b/cli/src/protocol.rs index 46681c672..a36f971e7 100644 --- a/cli/src/protocol.rs +++ b/cli/src/protocol.rs @@ -2,7 +2,9 @@ use clap::{builder::BoolishValueParser, Subcommand}; use ouisync_bridge::logger::LogFormat; use ouisync_lib::{AccessMode, PeerAddr, PeerInfo, StorageSize}; use serde::{Deserialize, Serialize}; -use std::{fmt, net::SocketAddr, path::PathBuf, time::Duration}; +use std::{fmt, io, net::SocketAddr, path::PathBuf, time::Duration}; + +use crate::repository::{FindError, InvalidRepositoryName}; #[derive(Subcommand, Debug, Serialize, Deserialize)] #[allow(clippy::large_enum_variant)] @@ -320,6 +322,42 @@ impl fmt::Display for Response { } } +#[derive(Debug, Serialize, Deserialize)] +pub struct Error(String); + +impl Error { + pub fn new(message: impl Into) -> Self { + Self(message.into()) + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::error::Error for Error {} + +macro_rules! impl_from { + ($ty:ty) => { + impl From<$ty> for Error { + fn from(src: $ty) -> Self { + Self(src.to_string()) + } + } + }; +} + +impl_from!(InvalidRepositoryName); +impl_from!(FindError); +impl_from!(ouisync_lib::Error); +impl_from!(ouisync_bridge::config::ConfigError); +impl_from!(ouisync_bridge::repository::OpenError); +impl_from!(ouisync_bridge::transport::TransportError); +impl_from!(anyhow::Error); +impl_from!(io::Error); + #[derive(Serialize, Deserialize)] pub(crate) struct QuotaInfo { pub quota: Option, diff --git a/cli/src/repository.rs b/cli/src/repository.rs index 1a15ab26a..efdcdd833 100644 --- a/cli/src/repository.rs +++ b/cli/src/repository.rs @@ -1,8 +1,8 @@ use crate::{options::Dirs, utils, DB_EXTENSION}; +use anyhow::Result; use camino::Utf8Path; use ouisync_bridge::{ config::ConfigStore, - error::{Error, Result}, protocol::remote::{Request, Response}, transport::RemoteClient, }; @@ -19,6 +19,7 @@ use std::{ path::{Path, PathBuf}, sync::{Arc, Mutex, RwLock}, }; +use thiserror::Error; use tokio::{fs, runtime, task}; use tokio_stream::StreamExt; @@ -83,15 +84,10 @@ impl Deref for RepositoryName { } } -#[derive(Debug)] +#[derive(Debug, Error)] +#[error("invalid repository name")] pub(crate) struct InvalidRepositoryName; -impl From for Error { - fn from(_: InvalidRepositoryName) -> Self { - Self::InvalidArgument - } -} - pub(crate) struct RepositoryHolder { pub repository: Arc, pub registration: Registration, @@ -116,6 +112,7 @@ impl RepositoryHolder { &self.name } + // TODO: should `mount_point` be `Option<&Path>` ? pub async fn set_mount_point(&self, mount_point: Option<&str>) { let metadata = self.repository.metadata(); @@ -393,20 +390,14 @@ impl RepositoryMap { } } +#[derive(Debug, Error)] pub(crate) enum FindError { + #[error("repository not found")] NotFound, + #[error("repository name is ambiguous")] Ambiguous, } -impl From for Error { - fn from(src: FindError) -> Self { - match src { - FindError::NotFound => Error::Library(ouisync_lib::Error::EntryNotFound), - FindError::Ambiguous => Error::Library(ouisync_lib::Error::AmbiguousEntry), - } - } -} - // Find repositories that are marked to be opened on startup and open them. pub(crate) async fn find_all( dirs: &Dirs, diff --git a/cli/src/server.rs b/cli/src/server.rs index caa2e6853..f5cc0a231 100644 --- a/cli/src/server.rs +++ b/cli/src/server.rs @@ -7,7 +7,6 @@ use crate::{ use anyhow::Result; use ouisync_bridge::{ config::{ConfigError, ConfigKey}, - error::Error, logger::{LogFormat, Logger}, transport::RemoteServer, }; @@ -75,7 +74,7 @@ impl ServerContainer { Self::default() } - pub async fn init(&self, state: Arc) -> Result<(), Error> { + pub async fn init(&self, state: Arc) -> Result<()> { let entry = state.config.entry(BIND_RPC_KEY); let addrs = match entry.get().await { Ok(addrs) => addrs, @@ -89,11 +88,7 @@ impl ServerContainer { Ok(()) } - pub async fn set( - &self, - state: Arc, - addrs: &[SocketAddr], - ) -> Result, Error> { + pub async fn set(&self, state: Arc, addrs: &[SocketAddr]) -> Result> { let entry = state.config.entry(BIND_RPC_KEY); let (handles, addrs) = start(state, addrs).await?; @@ -110,7 +105,7 @@ impl ServerContainer { async fn start( state: Arc, addrs: &[SocketAddr], -) -> Result<(Vec, Vec), Error> { +) -> Result<(Vec, Vec)> { let mut handles = Vec::with_capacity(addrs.len()); let mut local_addrs = Vec::with_capacity(addrs.len()); diff --git a/cli/src/state.rs b/cli/src/state.rs index 4af5a06fd..b0eb6b607 100644 --- a/cli/src/state.rs +++ b/cli/src/state.rs @@ -5,10 +5,10 @@ use crate::{ server::ServerContainer, transport::tls, }; +use anyhow::{format_err, Result}; use futures_util::future; use ouisync_bridge::{ config::ConfigStore, - error::Result, network::{self, NetworkDefaults}, transport, }; @@ -145,11 +145,10 @@ async fn make_server_config(config_dir: &Path) -> Result Result Result> { // Load custom root certificates (if any) let additional_root_certs = load_certificates(&config_dir.join("root_certs")).await?; - transport::make_client_config(&additional_root_certs) + Ok(transport::make_client_config(&additional_root_certs)?) } async fn load_certificates(root_dir: &Path) -> Result> { diff --git a/cli/src/transport/local.rs b/cli/src/transport/local.rs index d74642809..24791387f 100644 --- a/cli/src/transport/local.rs +++ b/cli/src/transport/local.rs @@ -2,16 +2,13 @@ use crate::{ handler::local::LocalHandler, - protocol::{Request, Response}, + protocol::{Error, Request, Response}, }; use interprocess::local_socket::{ tokio::{LocalSocketListener, LocalSocketStream}, ToLocalSocketName, }; -use ouisync_bridge::{ - error::Result, - transport::{socket_server_connection, SocketClient}, -}; +use ouisync_bridge::transport::{socket_server_connection, SocketClient}; use std::{fs, io, path::PathBuf}; use tokio::task::JoinSet; use tokio_util::{ @@ -86,7 +83,7 @@ impl Drop for LocalServer { } pub(crate) struct LocalClient { - inner: SocketClient, + inner: SocketClient, } impl LocalClient { @@ -99,7 +96,7 @@ impl LocalClient { }) } - pub async fn invoke(&self, request: Request) -> Result { + pub async fn invoke(&self, request: Request) -> Result { self.inner.invoke(request).await } } diff --git a/cli/src/transport/native.rs b/cli/src/transport/native.rs index 7e1bbf38a..29680ed7d 100644 --- a/cli/src/transport/native.rs +++ b/cli/src/transport/native.rs @@ -4,12 +4,9 @@ use crate::{ handler::local::LocalHandler, - protocol::{Request, Response}, -}; -use ouisync_bridge::{ - error::Result, - transport::{Handler as _, NotificationSender}, + protocol::{Error, Request, Response}, }; +use ouisync_bridge::transport::{Handler as _, NotificationSender}; use tokio::sync::mpsc; pub(crate) struct NativeClient { @@ -27,7 +24,7 @@ impl NativeClient { } } - pub async fn invoke(&self, request: Request) -> Result { + pub async fn invoke(&self, request: Request) -> Result { self.handler.handle(request, &self.notification_tx).await } diff --git a/cli/tests/utils.rs b/cli/tests/utils.rs index 9110f20f5..d9edd0a8d 100644 --- a/cli/tests/utils.rs +++ b/cli/tests/utils.rs @@ -401,7 +401,6 @@ where // Gracefully terminate the process, unlike `Child::kill` which sends `SIGKILL` and thus doesn't // allow destructors to run. -// TODO: windows version #[cfg(unix)] fn terminate(process: &Child) { // SAFETY: we are just sending a `SIGTERM` signal to the process, there should be no reason for @@ -411,6 +410,11 @@ fn terminate(process: &Child) { } } +#[cfg(windows)] +fn terminate(_process: &Child) { + todo!() +} + // RNG adaptor that implements `io::Read`. pub struct RngRead(pub R); diff --git a/ffi/Cargo.toml b/ffi/Cargo.toml index 45cf089ee..adbdf27c1 100644 --- a/ffi/Cargo.toml +++ b/ffi/Cargo.toml @@ -18,6 +18,7 @@ bytes = { workspace = true } camino = { workspace = true, features = ["serde1"] } futures-util = { workspace = true } hex = "0.4.3" +num_enum = { workspace = true } once_cell = { workspace = true } ouisync-bridge = { path = "../bridge" } ouisync-lib = { package = "ouisync", path = "../lib" } diff --git a/ffi/src/dart.rs b/ffi/src/dart.rs index 95f415ac8..6bcbb8e60 100644 --- a/ffi/src/dart.rs +++ b/ffi/src/dart.rs @@ -3,8 +3,8 @@ // Most of this file is ripped from [dart-sys](https://crates.io/crates/dart-sys) and // [allo-isolate](https://crates.io/crates/allo-isolate) +use crate::error::{ErrorCode, ToErrorCode}; use bytes::Bytes; -use ouisync_bridge::error::{ErrorCode, ToErrorCode}; use std::{ffi::CString, marker::PhantomData, mem, os::raw::c_char, result::Result}; #[repr(C)] diff --git a/ffi/src/directory.rs b/ffi/src/directory.rs index f3605f5fa..b95c883e0 100644 --- a/ffi/src/directory.rs +++ b/ffi/src/directory.rs @@ -4,7 +4,6 @@ use crate::{ state::State, }; use camino::Utf8PathBuf; -use ouisync_bridge::error::Result; use serde::{Deserialize, Serialize}; // Currently this is only a read-only snapshot of a directory. @@ -22,7 +21,7 @@ pub(crate) async fn create( state: &State, repo: Handle, path: Utf8PathBuf, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { state .get_repository(repo) .repository @@ -35,7 +34,7 @@ pub(crate) async fn open( state: &State, repo: Handle, path: Utf8PathBuf, -) -> Result { +) -> Result { let repo = state.get_repository(repo); let dir = repo.repository.open_directory(path).await?; @@ -57,7 +56,7 @@ pub(crate) async fn remove( repo: Handle, path: Utf8PathBuf, recursive: bool, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { let repo = &state.get_repository(repo).repository; if recursive { diff --git a/ffi/src/error.rs b/ffi/src/error.rs new file mode 100644 index 000000000..bfa8690e4 --- /dev/null +++ b/ffi/src/error.rs @@ -0,0 +1,190 @@ +use crate::SessionError; +use num_enum::{IntoPrimitive, TryFromPrimitive}; +use ouisync_bridge::{ + protocol::remote::ServerError, + repository::{MirrorError, OpenError}, + transport::TransportError, +}; +use ouisync_vfs::MountError; +use serde::{Deserialize, Serialize}; +use std::io; +use thiserror::Error; + +#[derive(Debug, Error, Serialize, Deserialize)] +#[error("{message}")] +pub struct Error { + pub message: String, + pub code: ErrorCode, +} + +#[derive( + Copy, Clone, Debug, Eq, PartialEq, Serialize, Deserialize, IntoPrimitive, TryFromPrimitive, +)] +#[repr(u16)] +#[serde(into = "u16", try_from = "u16")] +pub enum ErrorCode { + /// No error + Ok = 0, + /// Store error + Store = 1, + /// Insuficient permission to perform the intended operation + PermissionDenied = 2, + /// Malformed data + MalformedData = 3, + /// Entry already exists + EntryExists = 4, + /// Entry doesn't exist + EntryNotFound = 5, + /// Multiple matching entries found + AmbiguousEntry = 6, + /// The intended operation requires the directory to be empty but it isn't + DirectoryNotEmpty = 7, + /// The indended operation is not supported + OperationNotSupported = 8, + /// Failed to read from or write into the config file + Config = 10, + /// Argument passed to a function is not valid + InvalidArgument = 11, + /// Request or response is malformed + MalformedMessage = 12, + /// Storage format version mismatch + StorageVersionMismatch = 13, + /// Connection lost + ConnectionLost = 14, + + /// Failed to parse the mount point string + VfsFailedToParseMountPoint = 2048, + + /// Mounting is not yes supported on this Operating System + VfsUnsupportedOs = 2048 + 1, + + // These are equivalents of the dokan::file_system::FileSystemMountError errors + // https://github.com/dokan-dev/dokan-rust/blob/master/dokan/src/file_system.rs + /// A general error + VfsGeneral = 2048 + 3, + /// Bad drive letter + VfsDriveLetter = 2048 + 4, + /// Can't install the Dokan driver. + VfsDriverInstall = 2048 + 5, + /// The driver responds that something is wrong. + VfsStart = 2048 + 2, + /// Can't assign a drive letter or mount point. + /// + /// This probably means that the mount point is already used by another volume. + VfsMount = 2048 + 6, + /// The mount point is invalid. + VfsMountPoint = 2048 + 7, + /// The Dokan version that this wrapper is targeting is incompatible with the loaded Dokan + /// library. + VfsVersion = 2048 + 8, + + /// Unspecified error + Other = 65535, +} + +pub(crate) trait ToErrorCode { + fn to_error_code(&self) -> ErrorCode; +} + +impl ToErrorCode for SessionError { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::InitializeLogger(_) | Self::InitializeRuntime(_) => ErrorCode::Other, + Self::InvalidUtf8(_) => ErrorCode::InvalidArgument, + } + } +} + +impl ToErrorCode for MirrorError { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::Connect(error) => error.to_error_code(), + Self::Server(error) => error.to_error_code(), + } + } +} + +impl ToErrorCode for ouisync_lib::Error { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::Db(_) | Self::Store(_) => ErrorCode::Store, + Self::PermissionDenied => ErrorCode::PermissionDenied, + Self::MalformedData | Self::MalformedDirectory => ErrorCode::MalformedData, + Self::EntryExists => ErrorCode::EntryExists, + Self::EntryNotFound => ErrorCode::EntryNotFound, + Self::AmbiguousEntry => ErrorCode::AmbiguousEntry, + Self::DirectoryNotEmpty => ErrorCode::DirectoryNotEmpty, + Self::OperationNotSupported => ErrorCode::OperationNotSupported, + Self::InvalidArgument | Self::NonUtf8FileName | Self::OffsetOutOfRange => { + ErrorCode::InvalidArgument + } + Self::StorageVersionMismatch => ErrorCode::StorageVersionMismatch, + Self::EntryIsFile | Self::EntryIsDirectory | Self::Writer(_) | Self::Locked => { + ErrorCode::Other + } + } + } +} + +impl ToErrorCode for TransportError { + fn to_error_code(&self) -> ErrorCode { + match self { + TransportError::ConnectionLost => ErrorCode::ConnectionLost, + TransportError::MalformedMessage => ErrorCode::MalformedMessage, + } + } +} + +impl ToErrorCode for ServerError { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::ShuttingDown => ErrorCode::Other, + Self::InvalidArgument => ErrorCode::InvalidArgument, + Self::Transport(error) => error.to_error_code(), + Self::CreateRepository(_) => ErrorCode::Other, + } + } +} + +impl ToErrorCode for OpenError { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::Config(_) => ErrorCode::Config, + Self::Repository(error) => error.to_error_code(), + } + } +} + +impl ToErrorCode for MountError { + fn to_error_code(&self) -> ErrorCode { + match self { + Self::FailedToParseMountPoint => ErrorCode::VfsFailedToParseMountPoint, + Self::UnsupportedOs => ErrorCode::VfsUnsupportedOs, + Self::Start => ErrorCode::VfsStart, + Self::General => ErrorCode::VfsGeneral, + Self::DriveLetter => ErrorCode::VfsDriveLetter, + Self::DriverInstall => ErrorCode::VfsDriverInstall, + Self::Mount => ErrorCode::VfsMount, + Self::MountPoint => ErrorCode::VfsMountPoint, + Self::Version => ErrorCode::VfsVersion, + } + } +} + +impl ToErrorCode for io::Error { + fn to_error_code(&self) -> ErrorCode { + ErrorCode::Other + } +} + +impl From for Error +where + T: std::error::Error + ToErrorCode, +{ + fn from(src: T) -> Self { + Self { + message: src.to_string(), + code: src.to_error_code(), + } + } +} diff --git a/ffi/src/file.rs b/ffi/src/file.rs index d9f509369..07186cca1 100644 --- a/ffi/src/file.rs +++ b/ffi/src/file.rs @@ -1,8 +1,7 @@ use crate::{registry::Handle, repository::RepositoryHolder, state::State}; use camino::Utf8PathBuf; -use ouisync_bridge::error::{Error, Result}; use ouisync_lib::{deadlock::AsyncMutex, Branch, File}; -use std::{convert::TryInto, io::SeekFrom}; +use std::io::SeekFrom; pub struct FileHolder { pub(crate) file: AsyncMutex, @@ -13,7 +12,7 @@ pub(crate) async fn open( state: &State, repo: Handle, path: Utf8PathBuf, -) -> Result> { +) -> Result, ouisync_lib::Error> { let repo = state.get_repository(repo); let local_branch = repo.repository.local_branch().ok(); @@ -31,7 +30,7 @@ pub(crate) async fn create( state: &State, repo: Handle, path: Utf8PathBuf, -) -> Result> { +) -> Result, ouisync_lib::Error> { let repo = state.get_repository(repo); let local_branch = repo.repository.local_branch()?; @@ -50,7 +49,7 @@ pub(crate) async fn remove( state: &State, repo: Handle, path: Utf8PathBuf, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { state .get_repository(repo) .repository @@ -59,7 +58,10 @@ pub(crate) async fn remove( Ok(()) } -pub(crate) async fn close(state: &State, handle: Handle) -> Result<()> { +pub(crate) async fn close( + state: &State, + handle: Handle, +) -> Result<(), ouisync_lib::Error> { if let Some(holder) = state.files.remove(handle) { holder.file.lock().await.flush().await? } @@ -67,7 +69,10 @@ pub(crate) async fn close(state: &State, handle: Handle) -> Result<( Ok(()) } -pub(crate) async fn flush(state: &State, handle: Handle) -> Result<()> { +pub(crate) async fn flush( + state: &State, + handle: Handle, +) -> Result<(), ouisync_lib::Error> { state.files.get(handle).file.lock().await.flush().await?; Ok(()) } @@ -79,8 +84,8 @@ pub(crate) async fn read( handle: Handle, offset: u64, len: u64, -) -> Result> { - let len: usize = len.try_into().map_err(|_| Error::InvalidArgument)?; +) -> Result, ouisync_lib::Error> { + let len = len as usize; let mut buffer = vec![0; len]; let holder = state.files.get(handle); @@ -101,7 +106,7 @@ pub(crate) async fn write( handle: Handle, offset: u64, buffer: Vec, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { let holder = state.files.get(handle); let mut file = holder.file.lock().await; @@ -121,7 +126,11 @@ pub(crate) async fn write( } /// Truncate the file to `len` bytes. -pub(crate) async fn truncate(state: &State, handle: Handle, len: u64) -> Result<()> { +pub(crate) async fn truncate( + state: &State, + handle: Handle, + len: u64, +) -> Result<(), ouisync_lib::Error> { let holder = state.files.get(handle); let mut file = holder.file.lock().await; @@ -144,7 +153,10 @@ pub(crate) async fn len(state: &State, handle: Handle) -> u64 { } /// Retrieve the sync progress of the file. -pub(crate) async fn progress(state: &State, handle: Handle) -> Result { +pub(crate) async fn progress( + state: &State, + handle: Handle, +) -> Result { // Don't keep the file locked while progress is being awaited. let progress = state.files.get(handle).file.lock().await.progress(); let progress = progress.await?; diff --git a/ffi/src/handler.rs b/ffi/src/handler.rs index 1444ce394..155579bba 100644 --- a/ffi/src/handler.rs +++ b/ffi/src/handler.rs @@ -1,12 +1,14 @@ use crate::{ - directory, file, network, + directory, + error::Error, + file, network, protocol::{Request, Response}, repository, share_token, state::State, state_monitor, }; use async_trait::async_trait; -use ouisync_bridge::{error::Result, transport::NotificationSender}; +use ouisync_bridge::transport::NotificationSender; use ouisync_lib::PeerAddr; use std::{net::SocketAddr, sync::Arc}; @@ -25,12 +27,13 @@ impl Handler { impl ouisync_bridge::transport::Handler for Handler { type Request = Request; type Response = Response; + type Error = Error; async fn handle( &self, request: Self::Request, notification_tx: &NotificationSender, - ) -> Result { + ) -> Result { let response = match request { Request::RepositoryCreate { path, @@ -53,7 +56,7 @@ impl ouisync_bridge::transport::Handler for Handler { repository::close(&self.state, handle).await?.into() } Request::RepositoryCreateReopenToken(handle) => { - repository::create_reopen_token(&self.state, handle)?.into() + repository::create_reopen_token(&self.state, handle).into() } Request::RepositoryReopen { path, token } => { repository::reopen(&self.state, path, token).await?.into() diff --git a/ffi/src/lib.rs b/ffi/src/lib.rs index b21135f8d..b92f2b62b 100644 --- a/ffi/src/lib.rs +++ b/ffi/src/lib.rs @@ -4,6 +4,7 @@ mod utils; mod dart; mod directory; +mod error; mod file; mod handler; mod network; @@ -17,6 +18,7 @@ mod transport; use crate::{ dart::{Port, PortSender}, + error::{ErrorCode, ToErrorCode}, handler::Handler, state::State, transport::{ClientSender, Server}, @@ -25,23 +27,22 @@ use crate::{ #[cfg(unix)] use crate::{file::FileHolder, registry::Handle}; use bytes::Bytes; -use ouisync_bridge::{ - error::{Error, ErrorCode, Result, ToErrorCode}, - logger::{LogFormat, Logger}, -}; +use ouisync_bridge::logger::{LogFormat, Logger}; use ouisync_lib::StateMonitor; use ouisync_vfs::MountError; #[cfg(unix)] use std::os::raw::c_int; use std::{ ffi::CString, - mem, + io, mem, os::raw::{c_char, c_void}, path::PathBuf, ptr, slice, + str::Utf8Error, sync::Arc, time::Duration, }; +use thiserror::Error; use tokio::{ runtime::{self, Runtime}, time, @@ -54,8 +55,8 @@ pub struct SessionCreateResult { error_message: *const c_char, } -impl From> for SessionCreateResult { - fn from(result: Result) -> Self { +impl From> for SessionCreateResult { + fn from(result: Result) -> Self { match result { Ok(session) => Self { session: SessionHandle::new(Box::new(session)), @@ -90,12 +91,12 @@ pub unsafe extern "C" fn session_create( let configs_path = match utils::ptr_to_str(configs_path) { Ok(configs_path) => PathBuf::from(configs_path), - Err(error) => return Err(error).into(), + Err(error) => return Err(SessionError::from(error)).into(), }; let log_path = match utils::ptr_to_maybe_str(log_path) { Ok(log_path) => log_path.map(PathBuf::from), - Err(error) => return Err(error).into(), + Err(error) => return Err(SessionError::from(error)).into(), }; let (server, client_tx) = Server::new(port_sender, server_tx_port); @@ -205,7 +206,7 @@ pub unsafe extern "C" fn file_copy_to_raw_fd( session: SessionHandle, handle: Handle, fd: c_int, - port: Port>, + port: Port>, ) { use std::os::unix::io::FromRawFd; use tokio::fs; @@ -218,7 +219,7 @@ pub unsafe extern "C" fn file_copy_to_raw_fd( session.runtime.spawn(async move { let mut src = src.file.lock().await; - let result = src.copy_to_writer(&mut dst).await.map_err(Error::from); + let result = src.copy_to_writer(&mut dst).await; port_sender.send_result(port, result); }); @@ -299,7 +300,7 @@ impl Session { log_path: Option, port_sender: PortSender, client_sender: ClientSender, - ) -> Result { + ) -> Result { let root_monitor = StateMonitor::make_root(); // Init logger @@ -307,13 +308,14 @@ impl Session { log_path.as_deref(), Some(root_monitor.clone()), LogFormat::Human, - )?; + ) + .map_err(SessionError::InitializeLogger)?; // Create runtime let runtime = runtime::Builder::new_multi_thread() .enable_all() .build() - .map_err(Error::InitializeRuntime)?; + .map_err(SessionError::InitializeRuntime)?; let _enter = runtime.enter(); // runtime context is needed for some of the following calls let state = Arc::new(State::new(configs_path, root_monitor)); @@ -376,3 +378,13 @@ impl Session { } pub type SessionHandle = UniqueHandle; + +#[derive(Debug, Error)] +pub enum SessionError { + #[error("failed to initialize logger")] + InitializeLogger(#[source] io::Error), + #[error("failed to initialize runtime")] + InitializeRuntime(#[source] io::Error), + #[error("invalid utf8 string")] + InvalidUtf8(#[from] Utf8Error), +} diff --git a/ffi/src/protocol.rs b/ffi/src/protocol.rs index cb815cf29..eca511407 100644 --- a/ffi/src/protocol.rs +++ b/ffi/src/protocol.rs @@ -3,7 +3,7 @@ use crate::{ state::SubscriptionHandle, }; use camino::Utf8PathBuf; -use ouisync_bridge::{error::Result, network::NetworkDefaults}; +use ouisync_bridge::network::NetworkDefaults; use ouisync_lib::{AccessMode, MonitorId, PeerAddr, PeerInfo, Progress, ShareToken, StateMonitor}; use serde::{Deserialize, Serialize}; use std::{ diff --git a/ffi/src/repository.rs b/ffi/src/repository.rs index 8e85e04e2..84eb48f2a 100644 --- a/ffi/src/repository.rs +++ b/ffi/src/repository.rs @@ -1,11 +1,11 @@ use crate::{ + error::Error, registry::Handle, state::{State, SubscriptionHandle}, }; use camino::Utf8PathBuf; use ouisync_bridge::{ constants::{ENTRY_TYPE_DIRECTORY, ENTRY_TYPE_FILE}, - error::Result, protocol::Notification, repository, transport::NotificationSender, @@ -29,7 +29,7 @@ pub(crate) async fn create( local_read_password: Option, local_write_password: Option, share_token: Option, -) -> Result> { +) -> Result, Error> { let repository = repository::create( store_path.clone(), local_read_password, @@ -60,7 +60,7 @@ pub(crate) async fn open( state: &State, store_path: Utf8PathBuf, local_password: Option, -) -> Result> { +) -> Result, Error> { let repository = repository::open( store_path.clone(), local_password, @@ -81,7 +81,10 @@ pub(crate) async fn open( } /// Closes a repository. -pub(crate) async fn close(state: &State, handle: Handle) -> Result<()> { +pub(crate) async fn close( + state: &State, + handle: Handle, +) -> Result<(), ouisync_lib::Error> { let holder = state.remove_repository(handle); if let Some(holder) = holder { @@ -91,21 +94,19 @@ pub(crate) async fn close(state: &State, handle: Handle) -> Re Ok(()) } -pub(crate) fn create_reopen_token( - state: &State, - handle: Handle, -) -> Result> { - let holder = state.get_repository(handle); - let token = holder.repository.reopen_token().encode(); - - Ok(token) +pub(crate) fn create_reopen_token(state: &State, handle: Handle) -> Vec { + state + .get_repository(handle) + .repository + .reopen_token() + .encode() } pub(crate) async fn reopen( state: &State, store_path: Utf8PathBuf, token: Vec, -) -> Result> { +) -> Result, ouisync_lib::Error> { let repository = repository::reopen(store_path.clone(), token, &state.repos_monitor).await?; let repository = Arc::new(repository); let registration = state.network.register(repository.handle()).await; @@ -124,7 +125,7 @@ pub(crate) async fn set_read_access( handle: Handle, local_read_password: Option, share_token: Option, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { let holder = state.get_repository(handle); repository::set_read_access(&holder.repository, local_read_password, share_token).await } @@ -135,7 +136,7 @@ pub(crate) async fn set_read_and_write_access( local_old_rw_password: Option, local_new_rw_password: Option, share_token: Option, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { let holder = state.get_repository(handle); repository::set_read_and_write_access( &holder.repository, @@ -148,7 +149,10 @@ pub(crate) async fn set_read_and_write_access( /// Note that after removing read key the user may still read the repository if they previously had /// write key set up. -pub(crate) async fn remove_read_key(state: &State, handle: Handle) -> Result<()> { +pub(crate) async fn remove_read_key( + state: &State, + handle: Handle, +) -> Result<(), ouisync_lib::Error> { state .get_repository(handle) .repository @@ -161,7 +165,7 @@ pub(crate) async fn remove_read_key(state: &State, handle: Handle, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { state .get_repository(handle) .repository @@ -174,24 +178,24 @@ pub(crate) async fn remove_write_key( pub(crate) async fn requires_local_password_for_reading( state: &State, handle: Handle, -) -> Result { - Ok(state +) -> Result { + state .get_repository(handle) .repository .requires_local_password_for_reading() - .await?) + .await } /// Returns true if the repository requires a local password to be opened for writing. pub(crate) async fn requires_local_password_for_writing( state: &State, handle: Handle, -) -> Result { - Ok(state +) -> Result { + state .get_repository(handle) .repository .requires_local_password_for_writing() - .await?) + .await } /// Return the info-hash of the repository formatted as hex string. This can be used as a globally @@ -209,7 +213,7 @@ pub(crate) fn info_hash(state: &State, handle: Handle) -> Stri pub(crate) async fn database_id( state: &State, handle: Handle, -) -> Result> { +) -> Result, ouisync_lib::Error> { let holder = state.get_repository(handle); Ok(holder.repository.database_id().await?.as_ref().to_vec()) } @@ -220,13 +224,13 @@ pub(crate) async fn entry_type( state: &State, handle: Handle, path: Utf8PathBuf, -) -> Result> { +) -> Result, ouisync_lib::Error> { let holder = state.get_repository(handle); match holder.repository.lookup_type(path).await { Ok(entry_type) => Ok(Some(entry_type_to_num(entry_type))), Err(ouisync_lib::Error::EntryNotFound) => Ok(None), - Err(error) => Err(error.into()), + Err(error) => Err(error), } } @@ -236,7 +240,7 @@ pub(crate) async fn move_entry( handle: Handle, src: Utf8PathBuf, dst: Utf8PathBuf, -) -> Result<()> { +) -> Result<(), ouisync_lib::Error> { let holder = state.get_repository(handle); let (src_dir, src_name) = path::decompose(&src).ok_or(ouisync_lib::Error::EntryNotFound)?; let (dst_dir, dst_name) = path::decompose(&dst).ok_or(ouisync_lib::Error::EntryNotFound)?; @@ -320,7 +324,7 @@ pub(crate) async fn create_share_token( password: Option, access_mode: AccessMode, name: Option, -) -> Result { +) -> Result { let holder = state.get_repository(repository); repository::create_share_token(&holder.repository, password, access_mode, name).await } @@ -333,16 +337,16 @@ pub(crate) fn access_mode(state: &State, handle: Handle) -> u8 pub(crate) async fn sync_progress( state: &State, handle: Handle, -) -> Result { - Ok(state +) -> Result { + state .get_repository(handle) .repository .sync_progress() - .await?) + .await } /// Mirror the repository to the storage servers -pub(crate) async fn mirror(state: &State, handle: Handle) -> Result<()> { +pub(crate) async fn mirror(state: &State, handle: Handle) -> Result<(), Error> { let holder = state.get_repository(handle); let config = state.get_remote_client_config()?; let hosts: Vec<_> = state @@ -353,7 +357,9 @@ pub(crate) async fn mirror(state: &State, handle: Handle) -> R .cloned() .collect(); - ouisync_bridge::repository::mirror(&holder.repository, config, &hosts).await + ouisync_bridge::repository::mirror(&holder.repository, config, &hosts).await?; + + Ok(()) } pub(crate) fn entry_type_to_num(entry_type: EntryType) -> u8 { diff --git a/ffi/src/state.rs b/ffi/src/state.rs index f4f097be6..ef84305ad 100644 --- a/ffi/src/state.rs +++ b/ffi/src/state.rs @@ -4,7 +4,7 @@ use crate::{ repository::RepositoryHolder, }; use once_cell::sync::OnceCell; -use ouisync_bridge::{config::ConfigStore, error::Result, transport}; +use ouisync_bridge::{config::ConfigStore, transport}; use ouisync_lib::{ deadlock::{BlockingMutex, BlockingRwLockReadGuard}, network::Network, @@ -14,6 +14,7 @@ use ouisync_vfs::MultiRepoVFS; use scoped_task::ScopedJoinHandle; use std::{ collections::{BTreeSet, HashMap}, + io, path::PathBuf, sync::Arc, }; @@ -61,7 +62,7 @@ impl State { self.tasks.remove(handle); } - pub fn get_remote_client_config(&self) -> Result> { + pub fn get_remote_client_config(&self) -> Result, io::Error> { self.remote_client_config .get_or_try_init(|| transport::make_client_config(&[])) .cloned() diff --git a/ffi/src/state_monitor.rs b/ffi/src/state_monitor.rs index c82aa7b49..071ad9686 100644 --- a/ffi/src/state_monitor.rs +++ b/ffi/src/state_monitor.rs @@ -1,15 +1,15 @@ use crate::state::{State, SubscriptionHandle}; -use ouisync_bridge::{error::Result, protocol::Notification, transport::NotificationSender}; +use ouisync_bridge::{protocol::Notification, transport::NotificationSender}; use ouisync_lib::{MonitorId, StateMonitor}; use std::time::Duration; use tokio::time; /// Retrieve a state monitor corresponding to the `path`. -pub(crate) fn get(state: &State, path: Vec) -> Result { - Ok(state +pub(crate) fn get(state: &State, path: Vec) -> Result { + state .root_monitor .locate(path) - .ok_or(ouisync_lib::Error::EntryNotFound)?) + .ok_or(ouisync_lib::Error::EntryNotFound) } /// Subscribe to "on change" events happening inside a monitor corresponding to the `path`. @@ -17,7 +17,7 @@ pub(crate) fn subscribe( state: &State, notification_tx: &NotificationSender, path: Vec, -) -> Result { +) -> Result { let monitor = state .root_monitor .locate(path) diff --git a/ffi/src/utils.rs b/ffi/src/utils.rs index fad0f29b7..a566d28cb 100644 --- a/ffi/src/utils.rs +++ b/ffi/src/utils.rs @@ -1,9 +1,9 @@ -use ouisync_bridge::error::{Error, Result}; use std::{ ffi::{CStr, CString}, marker::PhantomData, os::raw::c_char, ptr, + str::Utf8Error, }; /// FFI handle to a resource with unique ownership. @@ -27,28 +27,22 @@ impl UniqueHandle { } } -pub(crate) unsafe fn ptr_to_str<'a>(ptr: *const c_char) -> Result<&'a str> { +pub(crate) unsafe fn ptr_to_str<'a>(ptr: *const c_char) -> Result<&'a str, Utf8Error> { Ok(ptr_to_maybe_str(ptr)?.unwrap_or("")) } -pub(crate) unsafe fn ptr_to_maybe_str<'a>(ptr: *const c_char) -> Result> { +pub(crate) unsafe fn ptr_to_maybe_str<'a>( + ptr: *const c_char, +) -> Result, Utf8Error> { if ptr.is_null() { return Ok(None); } - Ok(Some( - CStr::from_ptr(ptr) - .to_str() - .map_err(|_| Error::InvalidArgument)?, - )) -} - -pub(crate) fn str_to_c_string(s: &str) -> Result { - CString::new(s.as_bytes()).map_err(|_| Error::InvalidArgument) + CStr::from_ptr(ptr).to_str().map(Some) } pub(crate) fn str_to_ptr(s: &str) -> *mut c_char { - str_to_c_string(s) + CString::new(s.as_bytes()) .map(CString::into_raw) .unwrap_or(ptr::null_mut()) } diff --git a/vfs/Cargo.toml b/vfs/Cargo.toml index 941449f5a..68298150c 100644 --- a/vfs/Cargo.toml +++ b/vfs/Cargo.toml @@ -16,7 +16,6 @@ harness = false [dependencies] camino = "1.0.9" ouisync-lib = { package = "ouisync", path = "../lib" } -ouisync-bridge = { path = "../bridge" } slab = "0.4.6" tokio = { workspace = true } tracing = { workspace = true } diff --git a/vfs/src/dokan/mod.rs b/vfs/src/dokan/mod.rs index cf3088972..05b33216d 100644 --- a/vfs/src/dokan/mod.rs +++ b/vfs/src/dokan/mod.rs @@ -133,15 +133,13 @@ impl VirtualFilesystem { shared, ) } + } else if create_directory { + self.repo.create_directory(&path).await?; + Entry::new_dir(self.repo.clone(), path.clone(), shared).await? } else { - if create_directory { - self.repo.create_directory(&path).await?; - Entry::new_dir(self.repo.clone(), path.clone(), shared).await? - } else { - let mut file = self.repo.create_file(path).await?; - file.flush().await?; - Entry::new_file(OpenState::Open(file), shared) - } + let mut file = self.repo.create_file(path).await?; + file.flush().await?; + Entry::new_file(OpenState::Open(file), shared) }; return Ok((entry, true /* is new */)); @@ -233,7 +231,7 @@ impl VirtualFilesystem { match handles.entry(lock.path.clone()) { hash_map::Entry::Occupied(occupied) => { - assert_eq!(Arc::as_ptr(occupied.get()), Arc::as_ptr(&shared)); + assert_eq!(Arc::as_ptr(occupied.get()), Arc::as_ptr(shared)); lock.handle_count -= 1; @@ -250,7 +248,7 @@ impl VirtualFilesystem { return to_delete; } - return None; + None } // This FileEntry exists, so it must be in `handles`. hash_map::Entry::Vacant(_) => { @@ -314,7 +312,8 @@ impl VirtualFilesystem { // https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea // https://learn.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntcreatefile - #[instrument(skip(self, file_name, _security_context, access_mask, _file_attributes, _share_access, create_disposition, create_options), fields(file_name = ?to_path(file_name)), err(Debug))] + #[instrument(skip_all, fields(file_name = ?to_path(file_name)), err(Debug))] + #[allow(clippy::too_many_arguments)] async fn async_create_file<'c, 'h: 'c>( &'h self, file_name: &U16CStr, @@ -353,6 +352,7 @@ impl VirtualFilesystem { }) } + #[allow(clippy::too_many_arguments)] fn create_file( &self, file_name: &U16CStr, @@ -817,7 +817,7 @@ impl VirtualFilesystem { // preserve the seek offset. *file = OpenState::Lazy { path: src_path.clone(), - create_disposition: CreateDisposition::FileOpen, + create_disposition: CreateDisposition::Open, } } OpenState::Lazy { .. } | OpenState::Closed => {} @@ -829,7 +829,7 @@ impl VirtualFilesystem { } self.repo - .move_entry(&src_dir, &src_name, &dst_dir, &dst_name) + .move_entry(&src_dir, src_name, &dst_dir, dst_name) .await?; Ok(()) @@ -1117,7 +1117,7 @@ impl From for i32 { E::EntryIsFile => STATUS_INVALID_DEVICE_REQUEST, E::EntryIsDirectory => STATUS_INVALID_DEVICE_REQUEST, E::NonUtf8FileName => STATUS_OBJECT_NAME_INVALID, - E::OffsetOutOfRange => STATUS_INVALID_PARAMETER, + E::InvalidArgument | E::OffsetOutOfRange => STATUS_INVALID_PARAMETER, E::DirectoryNotEmpty => STATUS_DIRECTORY_NOT_EMPTY, E::OperationNotSupported => STATUS_NOT_IMPLEMENTED, E::Writer(_) => STATUS_IO_DEVICE_ERROR, @@ -1300,28 +1300,28 @@ impl OpenState { #[derive(Debug)] enum CreateDisposition { // If the file already exists, replace it with the given file. If it does not, create the given file. - FileSupersede, + Supersede, // If the file already exists, fail the request and do not create or open the given file. If it does not, create the given file. - FileCreate, + Create, // If the file already exists, open it instead of creating a new file. If it does not, fail the request and do not create a new file. - FileOpen, + Open, // If the file already exists, open it. If it does not, create the given file. - FileOpenIf, + OpenIf, // If the file already exists, open it and overwrite it. If it does not, fail the request. - FileOverwrite, + Overwrite, // If the file already exists, open it and overwrite it. If it does not, create the given file. - FileOverwriteIf, + OverwriteIf, } impl CreateDisposition { fn should_create(&self) -> bool { match self { - Self::FileSupersede => true, - Self::FileCreate => true, - Self::FileOpen => false, - Self::FileOpenIf => true, - Self::FileOverwrite => false, - Self::FileOverwriteIf => true, + Self::Supersede => true, + Self::Create => true, + Self::Open => false, + Self::OpenIf => true, + Self::Overwrite => false, + Self::OverwriteIf => true, } } } @@ -1331,12 +1331,12 @@ impl TryFrom for CreateDisposition { fn try_from(n: u32) -> Result { match n { - FILE_SUPERSEDE => Ok(Self::FileSupersede), - FILE_CREATE => Ok(Self::FileCreate), - FILE_OPEN => Ok(Self::FileOpen), - FILE_OPEN_IF => Ok(Self::FileOpenIf), - FILE_OVERWRITE => Ok(Self::FileOverwrite), - FILE_OVERWRITE_IF => Ok(Self::FileOverwriteIf), + FILE_SUPERSEDE => Ok(Self::Supersede), + FILE_CREATE => Ok(Self::Create), + FILE_OPEN => Ok(Self::Open), + FILE_OPEN_IF => Ok(Self::OpenIf), + FILE_OVERWRITE => Ok(Self::Overwrite), + FILE_OVERWRITE_IF => Ok(Self::OverwriteIf), _ => Err(STATUS_INVALID_PARAMETER.into()), } } @@ -1418,9 +1418,8 @@ impl fmt::Debug for AccessMask { pub(crate) fn default_mount_flags() -> MountFlags { // TODO: Check these flags. - let flags = MountFlags::empty(); //flags |= ALT_STREAM; //flags |= MountFlags::DEBUG | MountFlags::STDERR; //flags |= MountFlags::REMOVABLE; - flags + MountFlags::empty() } diff --git a/vfs/src/dokan/multi_repo_mount.rs b/vfs/src/dokan/multi_repo_mount.rs index 7513019ce..2491c573b 100644 --- a/vfs/src/dokan/multi_repo_mount.rs +++ b/vfs/src/dokan/multi_repo_mount.rs @@ -234,9 +234,9 @@ struct Handler { } impl Handler { - fn get_repo_and_path<'a>( + fn get_repo_and_path( &self, - path: &'a U16CStr, + path: &U16CStr, ) -> OperationResult<(Option>, U16CString)> { let (repo_name, path) = match decompose_path(path)? { (Some(repo_name), path) => (repo_name, path), @@ -284,6 +284,7 @@ impl MultiRepoEntryHandle { } impl Handler { + #[allow(clippy::too_many_arguments)] fn create_file_<'c, 'h: 'c>( &'h self, file_name: &U16CStr, @@ -1460,10 +1461,10 @@ impl<'c, 'h: 'c> FileSystemHandler<'c, 'h> for Handler { // Input looks like "\", "\desktop.ini", "\reponame\desktop.ini",... // Returns (Some(repository name), path in repository) if there is at least one subdirectory, and // (None, path to element in root) if no repository is used. -fn decompose_path<'a>(path: &'a U16CStr) -> OperationResult<(Option, U16CString)> { +fn decompose_path(path: &U16CStr) -> OperationResult<(Option, U16CString)> { let slice = path.as_slice(); - if slice.len() < 1 { + if slice.is_empty() { tracing::error!("MultiRepoVFS path is too short, should start with '\\' {path:?}"); return Err(STATUS_INVALID_PARAMETER); } diff --git a/vfs/src/dokan/single_repo_mount.rs b/vfs/src/dokan/single_repo_mount.rs index edccb6377..b08398889 100644 --- a/vfs/src/dokan/single_repo_mount.rs +++ b/vfs/src/dokan/single_repo_mount.rs @@ -299,9 +299,7 @@ pub fn mount( shutdown(); }); - if let Err(error) = on_mount_rx.recv().unwrap() { - return Err(error); - } + on_mount_rx.recv().unwrap()?; Ok(MountGuard { unmount_tx, diff --git a/vfs/src/lib.rs b/vfs/src/lib.rs index 2aac6cb6a..2491c9baf 100644 --- a/vfs/src/lib.rs +++ b/vfs/src/lib.rs @@ -22,7 +22,6 @@ pub use fuse::{mount, MountGuard}; #[cfg(any(target_os = "linux", target_os = "android"))] pub use dummy_multi_repo_mount::MultiRepoVFS; -use ouisync_bridge::error::{ErrorCode, ToErrorCode}; use thiserror::Error; #[derive(Copy, Clone, Debug, Error)] @@ -46,19 +45,3 @@ pub enum MountError { #[error("The Dokan version that this wrapper is targeting is incompatible with the loaded Dokan library")] Version, } - -impl ToErrorCode for MountError { - fn to_error_code(&self) -> ErrorCode { - match self { - MountError::FailedToParseMountPoint => ErrorCode::VfsFailedToParseMountPoint, - MountError::UnsupportedOs => ErrorCode::VfsUnsupportedOs, - MountError::Start => ErrorCode::VfsStart, - MountError::General => ErrorCode::VfsGeneral, - MountError::DriveLetter => ErrorCode::VfsDriveLetter, - MountError::DriverInstall => ErrorCode::VfsDriverInstall, - MountError::Mount => ErrorCode::VfsMount, - MountError::MountPoint => ErrorCode::VfsMountPoint, - MountError::Version => ErrorCode::VfsVersion, - } - } -}