From 05efd9b04471fc6e2dd9f373bc8ef505d65cf572 Mon Sep 17 00:00:00 2001 From: Jason Volk Date: Sat, 13 Jul 2024 21:11:05 +0000 Subject: [PATCH] elaborate error macro and apply at various callsites Signed-off-by: Jason Volk --- src/admin/server/commands.rs | 11 +- src/admin/utils.rs | 10 +- src/api/client/directory.rs | 7 +- src/api/client/sync.rs | 5 +- src/core/error/err.rs | 103 +++++++++++++----- src/core/error/mod.rs | 23 ++-- src/core/server.rs | 14 +-- src/core/utils/hash/argon.rs | 4 +- src/database/util.rs | 7 +- src/router/serve/unix.rs | 9 +- src/service/appservice/mod.rs | 4 +- src/service/manager.rs | 7 +- src/service/mod.rs | 2 +- src/service/rooms/alias/mod.rs | 2 +- src/service/rooms/auth_chain/mod.rs | 11 +- src/service/rooms/event_handler/mod.rs | 9 +- .../rooms/event_handler/parse_incoming_pdu.rs | 9 +- src/service/rooms/spaces/mod.rs | 10 +- src/service/rooms/state_accessor/mod.rs | 18 +-- src/service/rooms/state_cache/mod.rs | 20 ++-- src/service/sending/mod.rs | 4 +- src/service/sending/resolve.rs | 10 +- src/service/updates/mod.rs | 2 +- 23 files changed, 161 insertions(+), 140 deletions(-) diff --git a/src/admin/server/commands.rs b/src/admin/server/commands.rs index 229f05e1..e4503736 100644 --- a/src/admin/server/commands.rs +++ b/src/admin/server/commands.rs @@ -1,4 +1,4 @@ -use conduit::{utils::time, warn, Error, Result}; +use conduit::{utils::time, warn, Err, Result}; use ruma::events::room::message::RoomMessageEventContent; use crate::services; @@ -88,11 +88,10 @@ pub(super) async fn restart(_body: Vec<&str>, force: bool) -> Result (OwnedRoomId, u64, String) { /// Parses user ID pub(crate) fn parse_user_id(user_id: &str) -> Result { UserId::parse_with_server_name(user_id.to_lowercase(), services().globals.server_name()) - .map_err(|e| Error::Err(format!("The supplied username is not a valid username: {e}"))) + .map_err(|e| err!("The supplied username is not a valid username: {e}")) } /// Parses user ID as our local user @@ -41,7 +41,7 @@ pub(crate) fn parse_local_user_id(user_id: &str) -> Result { let user_id = parse_user_id(user_id)?; if !user_is_local(&user_id) { - return Err(Error::Err(String::from("User does not belong to our server."))); + return Err!("User {user_id:?} does not belong to our server."); } Ok(user_id) @@ -52,11 +52,11 @@ pub(crate) fn parse_active_local_user_id(user_id: &str) -> Result { let user_id = parse_local_user_id(user_id)?; if !services().users.exists(&user_id)? { - return Err(Error::Err(String::from("User does not exist on this server."))); + return Err!("User {user_id:?} does not exist on this server."); } if services().users.is_deactivated(&user_id)? { - return Err(Error::Err(String::from("User is deactivated."))); + return Err!("User {user_id:?} is deactivated."); } Ok(user_id) diff --git a/src/api/client/directory.rs b/src/api/client/directory.rs index 7d2aff0d..68bd0dff 100644 --- a/src/api/client/directory.rs +++ b/src/api/client/directory.rs @@ -1,4 +1,5 @@ use axum_client_ip::InsecureClientIp; +use conduit::{err, info, warn, Error, Result}; use ruma::{ api::{ client::{ @@ -18,9 +19,8 @@ use ruma::{ }, uint, RoomId, ServerName, UInt, UserId, }; -use tracing::{error, info, warn}; -use crate::{service::server_is_ours, services, Error, Result, Ruma}; +use crate::{service::server_is_ours, services, Ruma}; /// # `POST /_matrix/client/v3/publicRooms` /// @@ -271,8 +271,7 @@ pub(crate) async fn get_public_rooms_filtered_helper( _ => None, }) .map_err(|e| { - error!("Invalid room join rule event in database: {}", e); - Error::BadDatabase("Invalid room join rule event in database.") + err!(Database(error!("Invalid room join rule event in database: {e}"))) }) }) .transpose()? diff --git a/src/api/client/sync.rs b/src/api/client/sync.rs index dd1f3401..e425616b 100644 --- a/src/api/client/sync.rs +++ b/src/api/client/sync.rs @@ -8,7 +8,7 @@ use std::{ use conduit::{ error, utils::math::{ruma_from_u64, ruma_from_usize, usize_from_ruma, usize_from_u64_truncated}, - PduCount, + Err, PduCount, }; use ruma::{ api::client::{ @@ -545,8 +545,7 @@ async fn load_joined_room( // Database queries: let Some(current_shortstatehash) = services().rooms.state.get_room_shortstatehash(room_id)? else { - error!("Room {} has no state", room_id); - return Err(Error::BadDatabase("Room has no state")); + return Err!(Database(error!("Room {room_id} has no state"))); }; let since_shortstatehash = services() diff --git a/src/core/error/err.rs b/src/core/error/err.rs index 12dd4f3b..ea596644 100644 --- a/src/core/error/err.rs +++ b/src/core/error/err.rs @@ -1,3 +1,36 @@ +//! Error construction macros +//! +//! These are specialized macros specific to this project's patterns for +//! throwing Errors; they make Error construction succinct and reduce clutter. +//! They are developed from folding existing patterns into the macro while +//! fixing several anti-patterns in the codebase. +//! +//! - The primary macros `Err!` and `err!` are provided. `Err!` simply wraps +//! `err!` in the Result variant to reduce `Err(err!(...))` boilerplate, thus +//! `err!` can be used in any case. +//! +//! 1. The macro makes the general Error construction easy: `return +//! Err!("something went wrong")` replaces the prior `return +//! Err(Error::Err("something went wrong".to_owned()))`. +//! +//! 2. The macro integrates format strings automatically: `return +//! Err!("something bad: {msg}")` replaces the prior `return +//! Err(Error::Err(format!("something bad: {msg}")))`. +//! +//! 3. The macro scopes variants of Error: `return Err!(Database("problem with +//! bad database."))` replaces the prior `return Err(Error::Database("problem +//! with bad database."))`. +//! +//! 4. The macro matches and scopes some special-case sub-variants, for example +//! with ruma ErrorKind: `return Err!(Request(MissingToken("you must provide +//! an access token")))`. +//! +//! 5. The macro fixes the anti-pattern of repeating messages in an error! log +//! and then again in an Error construction, often slightly different due to +//! the Error variant not supporting a format string. Instead `return +//! Err(Database(error!("problem with db: {msg}")))` logs the error at the +//! callsite and then returns the error with the same string. Caller has the +//! option of replacing `error!` with `debug_error!`. #[macro_export] macro_rules! Err { ($($args:tt)*) => { @@ -7,36 +40,56 @@ macro_rules! Err { #[macro_export] macro_rules! err { - (error!($($args:tt),+)) => {{ - $crate::error!($($args),+); - $crate::error::Error::Err(std::format!($($args),+)) + (Config($item:literal, $($args:expr),*)) => {{ + $crate::error!(config = %$item, $($args),*); + $crate::error::Error::Config($item, $crate::format_maybe!($($args),*)) }}; - (debug_error!($($args:tt),+)) => {{ - $crate::debug_error!($($args),+); - $crate::error::Error::Err(std::format!($($args),+)) + (Request(Forbidden($level:ident!($($args:expr),*)))) => {{ + $crate::$level!($($args),*); + $crate::error::Error::Request( + ::ruma::api::client::error::ErrorKind::forbidden(), + $crate::format_maybe!($($args),*) + ) }}; - ($variant:ident(error!($($args:tt),+))) => {{ - $crate::error!($($args),+); - $crate::error::Error::$variant(std::format!($($args),+)) - }}; - - ($variant:ident(debug_error!($($args:tt),+))) => {{ - $crate::debug_error!($($args),+); - $crate::error::Error::$variant(std::format!($($args),+)) - }}; - - (Config($item:literal, $($args:tt),+)) => {{ - $crate::error!(config = %$item, $($args),+); - $crate::error::Error::Config($item, std::format!($($args),+)) - }}; - - ($variant:ident($($args:tt),+)) => { - $crate::error::Error::$variant(std::format!($($args),+)) + (Request(Forbidden($($args:expr),*))) => { + $crate::error::Error::Request( + ::ruma::api::client::error::ErrorKind::forbidden(), + $crate::format_maybe!($($args),*) + ) }; - ($string:literal$(,)? $($args:tt),*) => { - $crate::error::Error::Err(std::format!($string, $($args),*)) + (Request($variant:ident($level:ident!($($args:expr),*)))) => {{ + $crate::$level!($($args),*); + $crate::error::Error::Request( + ::ruma::api::client::error::ErrorKind::$variant, + $crate::format_maybe!($($args),*) + ) + }}; + + (Request($variant:ident($($args:expr),*))) => { + $crate::error::Error::Request( + ::ruma::api::client::error::ErrorKind::$variant, + $crate::format_maybe!($($args),*) + ) + }; + + ($variant:ident($level:ident!($($args:expr),*))) => {{ + $crate::$level!($($args),*); + $crate::error::Error::$variant($crate::format_maybe!($($args),*)) + }}; + + ($variant:ident($($args:expr),*)) => { + $crate::error::Error::$variant($crate::format_maybe!($($args),*)) + }; + + ($level:ident!($($args:expr),*)) => {{ + $crate::$level!($($args),*); + $crate::error::Error::Err($crate::format_maybe!($($args),*)) + }}; + + ($($args:expr),*) => { + $crate::error::Error::Err($crate::format_maybe!($($args),*)) }; } diff --git a/src/core/error/mod.rs b/src/core/error/mod.rs index cbc22f3e..3adfde97 100644 --- a/src/core/error/mod.rs +++ b/src/core/error/mod.rs @@ -3,7 +3,7 @@ mod log; mod panic; mod response; -use std::{any::Any, convert::Infallible, fmt}; +use std::{any::Any, borrow::Cow, convert::Infallible, fmt}; pub use log::*; @@ -64,7 +64,9 @@ pub enum Error { #[error("{0}")] Mxid(#[from] ruma::IdParseError), #[error("{0}: {1}")] - BadRequest(ruma::api::client::error::ErrorKind, &'static str), + BadRequest(ruma::api::client::error::ErrorKind, &'static str), //TODO: remove + #[error("{0}: {1}")] + Request(ruma::api::client::error::ErrorKind, Cow<'static, str>), #[error("from {0}: {1}")] Redaction(ruma::OwnedServerName, ruma::canonical_json::RedactionError), #[error("Remote server {0} responded with: {1}")] @@ -76,11 +78,9 @@ pub enum Error { #[error("Arithmetic operation failed: {0}")] Arithmetic(&'static str), #[error("There was a problem with the '{0}' directive in your configuration: {1}")] - Config(&'static str, String), + Config(&'static str, Cow<'static, str>), #[error("{0}")] - BadDatabase(&'static str), - #[error("{0}")] - Database(String), + Database(Cow<'static, str>), #[error("{0}")] BadServerResponse(&'static str), #[error("{0}")] @@ -88,14 +88,11 @@ pub enum Error { // unique / untyped #[error("{0}")] - Err(String), + Err(Cow<'static, str>), } impl Error { - pub fn bad_database(message: &'static str) -> Self { - error!("BadDatabase: {}", message); - Self::BadDatabase(message) - } + pub fn bad_database(message: &'static str) -> Self { crate::err!(Database(error!("{message}"))) } /// Sanitizes public-facing errors that can leak sensitive information. pub fn sanitized_string(&self) -> String { @@ -121,7 +118,7 @@ impl Error { match self { Self::Federation(_, error) => response::ruma_error_kind(error).clone(), - Self::BadRequest(kind, _) => kind.clone(), + Self::BadRequest(kind, _) | Self::Request(kind, _) => kind.clone(), _ => Unknown, } } @@ -129,7 +126,7 @@ impl Error { pub fn status_code(&self) -> http::StatusCode { match self { Self::Federation(_, ref error) | Self::RumaError(ref error) => error.status_code, - Self::BadRequest(ref kind, _) => response::bad_request_code(kind), + Self::BadRequest(ref kind, _) | Self::Request(ref kind, _) => response::bad_request_code(kind), Self::Conflict(_) => http::StatusCode::CONFLICT, _ => http::StatusCode::INTERNAL_SERVER_ERROR, } diff --git a/src/core/server.rs b/src/core/server.rs index 575924d3..752680ad 100644 --- a/src/core/server.rs +++ b/src/core/server.rs @@ -5,7 +5,7 @@ use std::{ use tokio::{runtime, sync::broadcast}; -use crate::{config::Config, log, Error, Result}; +use crate::{config::Config, log, Err, Result}; /// Server runtime state; public portion pub struct Server { @@ -65,15 +65,15 @@ impl Server { pub fn reload(&self) -> Result<()> { if cfg!(not(conduit_mods)) { - return Err(Error::Err("Reloading not enabled".into())); + return Err!("Reloading not enabled"); } if self.reloading.swap(true, Ordering::AcqRel) { - return Err(Error::Err("Reloading already in progress".into())); + return Err!("Reloading already in progress"); } if self.stopping.swap(true, Ordering::AcqRel) { - return Err(Error::Err("Shutdown already in progress".into())); + return Err!("Shutdown already in progress"); } self.signal("SIGINT").inspect_err(|_| { @@ -84,7 +84,7 @@ impl Server { pub fn restart(&self) -> Result<()> { if self.restarting.swap(true, Ordering::AcqRel) { - return Err(Error::Err("Restart already in progress".into())); + return Err!("Restart already in progress"); } self.shutdown() @@ -93,7 +93,7 @@ impl Server { pub fn shutdown(&self) -> Result<()> { if self.stopping.swap(true, Ordering::AcqRel) { - return Err(Error::Err("Shutdown already in progress".into())); + return Err!("Shutdown already in progress"); } self.signal("SIGTERM") @@ -102,7 +102,7 @@ impl Server { pub fn signal(&self, sig: &'static str) -> Result<()> { if let Err(e) = self.signal.send(sig) { - return Err(Error::Err(format!("Failed to send signal: {e}"))); + return Err!("Failed to send signal: {e}"); } Ok(()) diff --git a/src/core/utils/hash/argon.rs b/src/core/utils/hash/argon.rs index 98cef00e..0a1e1e14 100644 --- a/src/core/utils/hash/argon.rs +++ b/src/core/utils/hash/argon.rs @@ -5,7 +5,7 @@ use argon2::{ PasswordVerifier, Version, }; -use crate::{Error, Result}; +use crate::{err, Error, Result}; const M_COST: u32 = Params::DEFAULT_M_COST; // memory size in 1 KiB blocks const T_COST: u32 = Params::DEFAULT_T_COST; // nr of iterations @@ -44,7 +44,7 @@ pub(super) fn verify_password(password: &str, password_hash: &str) -> Result<()> .map_err(map_err) } -fn map_err(e: password_hash::Error) -> Error { Error::Err(e.to_string()) } +fn map_err(e: password_hash::Error) -> Error { err!("{e}") } #[cfg(test)] mod tests { diff --git a/src/database/util.rs b/src/database/util.rs index 513cedc8..f0ccbcbe 100644 --- a/src/database/util.rs +++ b/src/database/util.rs @@ -1,4 +1,4 @@ -use conduit::Result; +use conduit::{err, Result}; #[inline] pub(crate) fn result(r: std::result::Result) -> Result { @@ -10,4 +10,7 @@ pub(crate) fn and_then(t: T) -> Result { Ok(t) } pub(crate) fn or_else(e: rocksdb::Error) -> Result { Err(map_err(e)) } -pub(crate) fn map_err(e: rocksdb::Error) -> conduit::Error { conduit::Error::Database(e.into_string()) } +pub(crate) fn map_err(e: rocksdb::Error) -> conduit::Error { + let string = e.into_string(); + err!(Database(error!("{string}"))) +} diff --git a/src/router/serve/unix.rs b/src/router/serve/unix.rs index e876b2ac..266936dd 100644 --- a/src/router/serve/unix.rs +++ b/src/router/serve/unix.rs @@ -10,7 +10,7 @@ use axum::{ extract::{connect_info::IntoMakeServiceWithConnectInfo, Request}, Router, }; -use conduit::{debug_error, error::infallible, trace, Error, Result, Server}; +use conduit::{debug, debug_error, error::infallible, info, trace, warn, Err, Result, Server}; use hyper::{body::Incoming, service::service_fn}; use hyper_util::{ rt::{TokioExecutor, TokioIo}, @@ -23,7 +23,6 @@ use tokio::{ task::JoinSet, }; use tower::{Service, ServiceExt}; -use tracing::{debug, info, warn}; type MakeService = IntoMakeServiceWithConnectInfo; @@ -97,19 +96,19 @@ async fn init(server: &Arc) -> Result { let dir = path.parent().unwrap_or_else(|| Path::new("/")); if let Err(e) = fs::create_dir_all(dir).await { - return Err(Error::Err(format!("Failed to create {dir:?} for socket {path:?}: {e}"))); + return Err!("Failed to create {dir:?} for socket {path:?}: {e}"); } let listener = UnixListener::bind(path); if let Err(e) = listener { - return Err(Error::Err(format!("Failed to bind listener {path:?}: {e}"))); + return Err!("Failed to bind listener {path:?}: {e}"); } let socket_perms = config.unix_socket_perms.to_string(); let octal_perms = u32::from_str_radix(&socket_perms, 8).expect("failed to convert octal permissions"); let perms = std::fs::Permissions::from_mode(octal_perms); if let Err(e) = fs::set_permissions(&path, perms).await { - return Err(Error::Err(format!("Failed to set socket {path:?} permissions: {e}"))); + return Err!("Failed to set socket {path:?} permissions: {e}"); } info!("Listening at {:?}", path); diff --git a/src/service/appservice/mod.rs b/src/service/appservice/mod.rs index f7cf0b6b..24c9b8b0 100644 --- a/src/service/appservice/mod.rs +++ b/src/service/appservice/mod.rs @@ -2,7 +2,7 @@ mod data; use std::{collections::BTreeMap, sync::Arc}; -use conduit::Result; +use conduit::{err, Result}; use data::Data; use futures_util::Future; use regex::RegexSet; @@ -171,7 +171,7 @@ impl Service { .write() .await .remove(service_name) - .ok_or_else(|| crate::Error::Err("Appservice not found".to_owned()))?; + .ok_or(err!("Appservice not found"))?; // remove the appservice from the database self.db.unregister_appservice(service_name)?; diff --git a/src/service/manager.rs b/src/service/manager.rs index 84a28a76..af59b4a4 100644 --- a/src/service/manager.rs +++ b/src/service/manager.rs @@ -1,6 +1,6 @@ use std::{panic::AssertUnwindSafe, sync::Arc, time::Duration}; -use conduit::{debug, debug_warn, error, trace, utils::time, warn, Error, Result, Server}; +use conduit::{debug, debug_warn, error, trace, utils::time, warn, Err, Error, Result, Server}; use futures_util::FutureExt; use tokio::{ sync::{Mutex, MutexGuard}, @@ -129,10 +129,7 @@ impl Manager { /// Start the worker in a task for the service. async fn start_worker(&self, workers: &mut WorkersLocked<'_>, service: &Arc) -> Result<()> { if !self.server.running() { - return Err(Error::Err(format!( - "Service {:?} worker not starting during server shutdown.", - service.name() - ))); + return Err!("Service {:?} worker not starting during server shutdown.", service.name()); } debug!("Service {:?} worker starting...", service.name()); diff --git a/src/service/mod.rs b/src/service/mod.rs index 6f2f4ee5..81e0be3b 100644 --- a/src/service/mod.rs +++ b/src/service/mod.rs @@ -24,7 +24,7 @@ extern crate conduit_database as database; use std::sync::{Arc, RwLock}; -pub(crate) use conduit::{config, debug_error, debug_info, debug_warn, utils, Config, Error, Result, Server}; +pub(crate) use conduit::{config, debug_error, debug_warn, utils, Config, Error, Result, Server}; pub use conduit::{pdu, PduBuilder, PduCount, PduEvent}; use database::Database; pub(crate) use service::{Args, Service}; diff --git a/src/service/rooms/alias/mod.rs b/src/service/rooms/alias/mod.rs index 97eb0f48..792f5c98 100644 --- a/src/service/rooms/alias/mod.rs +++ b/src/service/rooms/alias/mod.rs @@ -171,7 +171,7 @@ impl Service { .rooms .alias .resolve_local_alias(room_alias)? - .ok_or_else(|| err!(BadConfig("Room does not exist.")))?, + .ok_or_else(|| err!(Request(NotFound("Room does not exist."))))?, )); } } diff --git a/src/service/rooms/auth_chain/mod.rs b/src/service/rooms/auth_chain/mod.rs index 2919ea7e..4e8c7bb2 100644 --- a/src/service/rooms/auth_chain/mod.rs +++ b/src/service/rooms/auth_chain/mod.rs @@ -5,9 +5,9 @@ use std::{ sync::Arc, }; -use conduit::{debug, error, trace, validated, warn, Error, Result}; +use conduit::{debug, error, trace, validated, warn, Err, Result}; use data::Data; -use ruma::{api::client::error::ErrorKind, EventId, RoomId}; +use ruma::{EventId, RoomId}; use crate::services; @@ -143,8 +143,11 @@ impl Service { match services().rooms.timeline.get_pdu(&event_id) { Ok(Some(pdu)) => { if pdu.room_id != room_id { - error!(?event_id, ?pdu, "auth event for incorrect room_id"); - return Err(Error::BadRequest(ErrorKind::forbidden(), "Evil event in db")); + return Err!(Request(Forbidden( + "auth event {event_id:?} for incorrect room {} which is not {}", + pdu.room_id, + room_id + ))); } for auth_event in &pdu.auth_events { let sauthevent = services() diff --git a/src/service/rooms/event_handler/mod.rs b/src/service/rooms/event_handler/mod.rs index 33c00643..6cb23b9f 100644 --- a/src/service/rooms/event_handler/mod.rs +++ b/src/service/rooms/event_handler/mod.rs @@ -10,7 +10,7 @@ use std::{ }; use conduit::{ - debug, debug_error, debug_info, error, info, trace, + debug, debug_error, debug_info, err, error, info, trace, utils::{math::continue_exponential_backoff_secs, MutexMap}, warn, Error, Result, }; @@ -1382,11 +1382,8 @@ impl Service { } fn get_room_version_id(create_event: &PduEvent) -> Result { - let create_event_content: RoomCreateEventContent = - serde_json::from_str(create_event.content.get()).map_err(|e| { - error!("Invalid create event: {}", e); - Error::BadDatabase("Invalid create event in db") - })?; + let create_event_content: RoomCreateEventContent = serde_json::from_str(create_event.content.get()) + .map_err(|e| err!(Database("Invalid create event: {e}")))?; Ok(create_event_content.room_version) } diff --git a/src/service/rooms/event_handler/parse_incoming_pdu.rs b/src/service/rooms/event_handler/parse_incoming_pdu.rs index 4c907e51..8fcd8549 100644 --- a/src/service/rooms/event_handler/parse_incoming_pdu.rs +++ b/src/service/rooms/event_handler/parse_incoming_pdu.rs @@ -1,4 +1,4 @@ -use conduit::{Error, Result}; +use conduit::{Err, Error, Result}; use ruma::{api::client::error::ErrorKind, CanonicalJsonObject, OwnedEventId, OwnedRoomId, RoomId}; use serde_json::value::RawValue as RawJsonValue; use tracing::warn; @@ -17,15 +17,12 @@ pub fn parse_incoming_pdu(pdu: &RawJsonValue) -> Result<(OwnedEventId, Canonical .ok_or(Error::BadRequest(ErrorKind::InvalidParam, "Invalid room id in pdu"))?; let Ok(room_version_id) = services().rooms.state.get_room_version(&room_id) else { - return Err(Error::Err(format!("Server is not in room {room_id}"))); + return Err!("Server is not in room {room_id}"); }; let Ok((event_id, value)) = gen_event_id_canonical_json(pdu, &room_version_id) else { // Event could not be converted to canonical json - return Err(Error::BadRequest( - ErrorKind::InvalidParam, - "Could not convert event to canonical json.", - )); + return Err!(Request(InvalidParam("Could not convert event to canonical json."))); }; Ok((event_id, value, room_id)) diff --git a/src/service/rooms/spaces/mod.rs b/src/service/rooms/spaces/mod.rs index 03d0d43f..18133fc1 100644 --- a/src/service/rooms/spaces/mod.rs +++ b/src/service/rooms/spaces/mod.rs @@ -7,7 +7,7 @@ use std::{ sync::Arc, }; -use conduit::{checked, debug_info, utils::math::usize_from_f64}; +use conduit::{checked, debug, debug_info, err, utils::math::usize_from_f64, warn, Error, Result}; use lru_cache::LruCache; use ruma::{ api::{ @@ -27,9 +27,8 @@ use ruma::{ OwnedRoomId, OwnedServerName, RoomId, ServerName, UInt, UserId, }; use tokio::sync::Mutex; -use tracing::{debug, error, warn}; -use crate::{services, Error, Result}; +use crate::services; pub struct CachedSpaceHierarchySummary { summary: SpaceHierarchyParentSummary, @@ -380,10 +379,7 @@ impl Service { .map(|s| { serde_json::from_str(s.content.get()) .map(|c: RoomJoinRulesEventContent| c.join_rule) - .map_err(|e| { - error!("Invalid room join rule event in database: {}", e); - Error::BadDatabase("Invalid room join rule event in database.") - }) + .map_err(|e| err!(Database(error!("Invalid room join rule event in database: {e}")))) }) .transpose()? .unwrap_or(JoinRule::Invite); diff --git a/src/service/rooms/state_accessor/mod.rs b/src/service/rooms/state_accessor/mod.rs index 389f12c3..7abe5e0f 100644 --- a/src/service/rooms/state_accessor/mod.rs +++ b/src/service/rooms/state_accessor/mod.rs @@ -6,7 +6,7 @@ use std::{ sync::{Arc, Mutex as StdMutex, Mutex}, }; -use conduit::{error, utils::math::usize_from_f64, warn, Error, Result}; +use conduit::{err, error, utils::math::usize_from_f64, warn, Error, Result}; use data::Data; use lru_cache::LruCache; use ruma::{ @@ -454,10 +454,7 @@ impl Service { .map(|c: RoomJoinRulesEventContent| { (c.join_rule.clone().into(), self.allowed_room_ids(c.join_rule)) }) - .map_err(|e| { - error!("Invalid room join rule event in database: {e}"); - Error::BadDatabase("Invalid room join rule event in database.") - }) + .map_err(|e| err!(Database(error!("Invalid room join rule event in database: {e}")))) }) .transpose()? .unwrap_or((SpaceRoomJoinRule::Invite, vec![]))) @@ -483,10 +480,8 @@ impl Service { Ok(self .room_state_get(room_id, &StateEventType::RoomCreate, "")? .map(|s| { - serde_json::from_str::(s.content.get()).map_err(|e| { - error!("Invalid room create event in database: {e}"); - Error::BadDatabase("Invalid room create event in database.") - }) + serde_json::from_str::(s.content.get()) + .map_err(|e| err!(Database(error!("Invalid room create event in database: {e}")))) }) .transpose()? .and_then(|e| e.room_type)) @@ -499,10 +494,7 @@ impl Service { .map_or(Ok(None), |s| { serde_json::from_str::(s.content.get()) .map(|content| Some(content.algorithm)) - .map_err(|e| { - error!("Invalid room encryption event in database: {e}"); - Error::BadDatabase("Invalid room encryption event in database.") - }) + .map_err(|e| err!(Database(error!("Invalid room encryption event in database: {e}")))) }) } } diff --git a/src/service/rooms/state_cache/mod.rs b/src/service/rooms/state_cache/mod.rs index 30fd7bf4..48215817 100644 --- a/src/service/rooms/state_cache/mod.rs +++ b/src/service/rooms/state_cache/mod.rs @@ -2,7 +2,7 @@ mod data; use std::sync::Arc; -use conduit::{error, warn, Error, Result}; +use conduit::{err, error, warn, Error, Result}; use data::Data; use itertools::Itertools; use ruma::{ @@ -128,10 +128,8 @@ impl Service { .account_data .get(Some(&predecessor.room_id), user_id, RoomAccountDataEventType::Tag)? .map(|event| { - serde_json::from_str(event.get()).map_err(|e| { - warn!("Invalid account data event in db: {e:?}"); - Error::BadDatabase("Invalid account data event in db.") - }) + serde_json::from_str(event.get()) + .map_err(|e| err!(Database(warn!("Invalid account data event in db: {e:?}")))) }) { services() .account_data @@ -144,10 +142,8 @@ impl Service { .account_data .get(None, user_id, GlobalAccountDataEventType::Direct.to_string().into())? .map(|event| { - serde_json::from_str::(event.get()).map_err(|e| { - warn!("Invalid account data event in db: {e:?}"); - Error::BadDatabase("Invalid account data event in db.") - }) + serde_json::from_str::(event.get()) + .map_err(|e| err!(Database(warn!("Invalid account data event in db: {e:?}")))) }) { let mut direct_event = direct_event?; let mut room_ids_updated = false; @@ -185,10 +181,8 @@ impl Service { .into(), )? .map(|event| { - serde_json::from_str::(event.get()).map_err(|e| { - warn!("Invalid account data event in db: {e:?}"); - Error::BadDatabase("Invalid account data event in db.") - }) + serde_json::from_str::(event.get()) + .map_err(|e| err!(Database(warn!("Invalid account data event in db: {e:?}")))) }) .transpose()? .map_or(false, |ignored| { diff --git a/src/service/sending/mod.rs b/src/service/sending/mod.rs index eb708fcf..88b8b189 100644 --- a/src/service/sending/mod.rs +++ b/src/service/sending/mod.rs @@ -6,7 +6,7 @@ mod sender; use std::fmt::Debug; -use conduit::{Error, Result}; +use conduit::{err, Result}; pub use resolve::{resolve_actual_dest, CachedDest, CachedOverride, FedDest}; use ruma::{ api::{appservice::Registration, OutgoingRequest}, @@ -224,7 +224,7 @@ impl Service { fn dispatch(&self, msg: Msg) -> Result<()> { debug_assert!(!self.sender.is_full(), "channel full"); debug_assert!(!self.sender.is_closed(), "channel closed"); - self.sender.send(msg).map_err(|e| Error::Err(e.to_string())) + self.sender.send(msg).map_err(|e| err!("{e}")) } } diff --git a/src/service/sending/resolve.rs b/src/service/sending/resolve.rs index 91041a2f..8b6bfc95 100644 --- a/src/service/sending/resolve.rs +++ b/src/service/sending/resolve.rs @@ -5,13 +5,12 @@ use std::{ time::SystemTime, }; -use conduit::Err; +use conduit::{debug, debug_error, debug_info, debug_warn, trace, utils::rand, Err, Error, Result}; use hickory_resolver::{error::ResolveError, lookup::SrvLookup}; use ipaddress::IPAddress; use ruma::{OwnedServerName, ServerName}; -use tracing::{debug, error, trace}; -use crate::{debug_error, debug_info, debug_warn, services, utils::rand, Error, Result}; +use crate::services; /// Wraps either an literal IP address plus port, or a hostname plus complement /// (colon-plus-port if it was specified). @@ -346,10 +345,7 @@ fn handle_resolve_error(e: &ResolveError) -> Result<()> { debug!("{e}"); Ok(()) }, - _ => { - error!("DNS {e}"); - Err(Error::Err(e.to_string())) - }, + _ => Err!(error!("DNS {e}")), } } diff --git a/src/service/updates/mod.rs b/src/service/updates/mod.rs index a7088aba..3fb680d6 100644 --- a/src/service/updates/mod.rs +++ b/src/service/updates/mod.rs @@ -72,7 +72,7 @@ impl Service { .await?; let response = serde_json::from_str::(&response.text().await?) - .map_err(|e| Error::Err(format!("Bad check for updates response: {e}")))?; + .map_err(|e| err!("Bad check for updates response: {e}"))?; let mut last_update_id = self.last_check_for_updates_id()?; for update in response.updates {