Merge branch 'error-overhaul'

.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

bridge/src/config.rs

@@ -103,7 +103,7 @@ impl<T> ConfigEntry<T> {
         }
 
         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?;

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<DeviceId> = ConfigKey::new(
      identification.",
 );
 
-pub async fn get_or_create(config: &ConfigStore) -> Result<DeviceId> {
+pub async fn get_or_create(config: &ConfigStore) -> Result<DeviceId, ConfigError> {
     let cfg = config.entry(KEY);
 
     match cfg.get().await {
@@ -31,7 +28,7 @@ pub async fn get_or_create(config: &ConfigStore) -> Result<DeviceId> {
             cfg.set(&new_id).await?;
             Ok(new_id)
         }
-        Err(e) => Err(e.into()),
+        Err(ConfigError::Io(error)) => Err(error.into()),
     }
 }
 

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;

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},

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<StateMonitor>,
         format: LogFormat,
-    ) -> Result<Self> {
+    ) -> Result<Self, io::Error> {
         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)?;

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);
 

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<T> {
+pub(crate) enum ServerMessage<T, E> {
     Success(T),
-    Failure { code: ErrorCode, message: String },
+    Failure(E),
     Notification(Notification),
 }
 
-impl<T> ServerMessage<T> {
-    pub fn response(result: Result<T>) -> Self {
+impl<T, E> ServerMessage<T, E> {
+    pub fn response(result: Result<T, E>) -> 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<TestResponse> = rmp_serde::from_slice(&encoded).unwrap();
+            let decoded: ServerMessage<TestResponse, TestError> =
+                rmp_serde::from_slice(&encoded).unwrap();
             assert_eq!(decoded, orig);
         }
     }