diff options
35 files changed, 719 insertions, 519 deletions
diff --git a/src/boot/app.rs b/src/boot/app.rs index 1ca8adb..c2766cd 100644 --- a/src/boot/app.rs +++ b/src/boot/app.rs @@ -4,10 +4,10 @@ use sqlx::sqlite::SqlitePool; use super::Snapshot; use crate::{ conversation::{self, repo::Provider as _}, - db::NotFound, + db::{self, NotFound as _}, + error::failed::{Failed, ResultExt as _}, event::{Event, Sequence, repo::Provider as _}, message::{self, repo::Provider as _}, - name, user::{self, repo::Provider as _}, vapid::{self, repo::Provider as _}, }; @@ -21,16 +21,37 @@ impl Boot { Self { db } } - pub async fn snapshot(&self) -> Result<Snapshot, Error> { - let mut tx = self.db.begin().await?; - let resume_point = tx.sequence().current().await?; - - let users = tx.users().all(resume_point).await?; - let conversations = tx.conversations().all(resume_point).await?; - let messages = tx.messages().all(resume_point).await?; - let vapid = tx.vapid().current().await.optional()?; - - tx.commit().await?; + pub async fn snapshot(&self) -> Result<Snapshot, Failed> { + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + let resume_point = tx + .sequence() + .current() + .await + .fail("Failed to load resume point")?; + + let users = tx + .users() + .all(resume_point) + .await + .fail("Failed to load user events")?; + let conversations = tx + .conversations() + .all(resume_point) + .await + .fail("Failed to load conversation events")?; + let messages = tx + .messages() + .all(resume_point) + .await + .fail("Failed to load message events")?; + let vapid = tx + .vapid() + .current() + .await + .optional() + .fail("Failed to load VAPID key events")?; + + tx.commit().await.fail(db::failed::COMMIT)?; let user_events = users .iter() @@ -71,45 +92,3 @@ impl Boot { }) } } - -#[derive(Debug, thiserror::Error)] -#[error(transparent)] -pub enum Error { - Database(#[from] sqlx::Error), - Name(#[from] name::Error), - Ecdsa(#[from] p256::ecdsa::Error), - Pkcs8(#[from] p256::pkcs8::Error), - WebPush(#[from] web_push::WebPushError), -} - -impl From<user::repo::LoadError> for Error { - fn from(error: user::repo::LoadError) -> Self { - use user::repo::LoadError; - match error { - LoadError::Name(error) => error.into(), - LoadError::Database(error) => error.into(), - } - } -} - -impl From<conversation::repo::LoadError> for Error { - fn from(error: conversation::repo::LoadError) -> Self { - use conversation::repo::LoadError; - match error { - LoadError::Name(error) => error.into(), - LoadError::Database(error) => error.into(), - } - } -} - -impl From<vapid::repo::Error> for Error { - fn from(error: vapid::repo::Error) -> Self { - use vapid::repo::Error; - match error { - Error::Database(error) => error.into(), - Error::Ecdsa(error) => error.into(), - Error::Pkcs8(error) => error.into(), - Error::WebPush(error) => error.into(), - } - } -} @@ -3,7 +3,7 @@ //! This module supports running `pilcrow` as a freestanding program, via the //! [`Args`] struct. -use std::{future, io, str}; +use std::{future, str}; use axum::{ http::header, @@ -15,10 +15,13 @@ use sqlx::sqlite::SqlitePool; use tokio::net; use web_push::{IsahcWebPushClient, WebPushClient}; +pub use crate::exit::Exit; use crate::{ app::App, - clock, db, routes, - umask::{self, Umask}, + clock, db, + error::failed::{Failed, ResultExt as _}, + routes, + umask::Umask, }; /// Command-line entry point for running the `pilcrow` server. @@ -96,20 +99,24 @@ impl Args { /// of the failure. pub async fn run(self) -> Result<(), impl std::error::Error> { self.umask.set(); - let pool = self.pool().await?; + let pool = self.pool().await.fail("Failed to create database pool")?; - let webpush = IsahcWebPushClient::new()?; + let webpush = IsahcWebPushClient::new().fail("Failed to create web push publisher")?; let app = App::from(pool, webpush); match self.command { None => self.serve(app).await?, - Some(Command::RotateVapidKey) => app.vapid().rotate_key().await?, + Some(Command::RotateVapidKey) => app + .vapid() + .rotate_key() + .await + .fail("Failed to rotate VAPID key")?, } - Result::<_, Error>::Ok(()) + Result::<_, Failed>::Ok(()) } - async fn serve<P>(self, app: App<P>) -> Result<(), Error> + async fn serve<P>(self, app: App<P>) -> Result<(), Failed> where P: WebPushClient + Clone + Send + Sync + 'static, { @@ -118,24 +125,25 @@ impl Args { .route_layer(middleware::map_response(Self::server_info())) .with_state(app); - let listener = self.listener().await?; - let started_msg = started_msg(&listener)?; + let listen_addr = self.listen_addr(); + let listener = net::TcpListener::bind(&listen_addr).await.fail_with(|| { + format!( + "Failed to bind to {host}:{port}", + host = listen_addr.0, + port = listen_addr.1 + ) + })?; + let started_msg = started_msg(&listen_addr); let serve = axum::serve(listener, app); println!("{started_msg}"); - serve.await?; + serve.await.fail("Failed to serve application")?; Ok(()) } - async fn listener(&self) -> io::Result<net::TcpListener> { - let listen_addr = self.listen_addr(); - let listener = tokio::net::TcpListener::bind(listen_addr).await?; - Ok(listener) - } - - fn listen_addr(&self) -> impl net::ToSocketAddrs + '_ { - (self.address.as_str(), self.port) + fn listen_addr(&self) -> (String, u16) { + (self.address.clone(), self.port) } async fn pool(&self) -> Result<SqlitePool, db::Error> { @@ -154,17 +162,7 @@ impl Args { } } -fn started_msg(listener: &net::TcpListener) -> io::Result<String> { - let local_addr = listener.local_addr()?; - Ok(format!("listening on http://{local_addr}/")) -} - -#[derive(Debug, thiserror::Error)] -#[error(transparent)] -enum Error { - Io(#[from] io::Error), - Database(#[from] db::Error), - Sqlx(#[from] sqlx::Error), - Umask(#[from] umask::Error), - Webpush(#[from] web_push::WebPushError), +fn started_msg(listen_addr: &(String, u16)) -> String { + let (host, port) = listen_addr; + format!("Listening on http://{host}:{port}/") } diff --git a/src/conversation/app.rs b/src/conversation/app.rs index 2b62e77..73c655b 100644 --- a/src/conversation/app.rs +++ b/src/conversation/app.rs @@ -2,17 +2,14 @@ use chrono::TimeDelta; use itertools::Itertools; use sqlx::sqlite::SqlitePool; -use super::{ - Conversation, History, Id, history, - repo::{LoadError, Provider as _}, - validate, -}; +use super::{Conversation, History, Id, history, repo::Provider as _, validate}; use crate::{ clock::DateTime, - db::{Duplicate as _, NotFound as _}, + db::{self, NotFound as _}, + error::failed::{ErrorExt as _, Failed, ResultExt as _}, event::{Broadcaster, Sequence, repo::Provider as _}, message::repo::Provider as _, - name::{self, Name}, + name::Name, }; pub struct Conversations { @@ -34,8 +31,12 @@ impl Conversations { return Err(CreateError::InvalidName(name.clone())); } - let mut tx = self.db.begin().await?; - let created = tx.sequence().next(created_at).await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + let created = tx + .sequence() + .next(created_at) + .await + .fail("Failed to find event sequence number")?; let conversation = History::begin(name, created); // This filter technically includes every event in the history, but it's easier to follow if @@ -45,9 +46,12 @@ impl Conversations { tx.conversations() .record_events(events.clone()) .await - .duplicate(|| CreateError::DuplicateName(name.clone()))?; + .map_err(|err| match err.as_database_error() { + Some(err) if err.is_unique_violation() => CreateError::DuplicateName(name.clone()), + _ => err.fail("Failed to store events"), + })?; - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; self.events.broadcast_from(events); @@ -56,17 +60,21 @@ impl Conversations { // This function is careless with respect to time, and gets you the // conversation as it exists in the specific moment when you call it. - pub async fn get(&self, conversation: &Id) -> Result<Conversation, Error> { - let to_not_found = || Error::NotFound(conversation.clone()); - let to_deleted = || Error::Deleted(conversation.clone()); + pub async fn get(&self, conversation: &Id) -> Result<Conversation, GetError> { + let to_not_found = || GetError::NotFound(conversation.clone()); + let to_deleted = || GetError::Deleted(conversation.clone()); + + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; - let mut tx = self.db.begin().await?; let conversation = tx .conversations() .by_id(conversation) .await - .not_found(to_not_found)?; - tx.commit().await?; + .optional() + .fail("Failed to load conversation")? + .ok_or_else(to_not_found)?; + + tx.commit().await.fail(db::failed::COMMIT)?; conversation.as_snapshot().ok_or_else(to_deleted) } @@ -76,16 +84,28 @@ impl Conversations { conversation: &Id, deleted_at: &DateTime, ) -> Result<(), DeleteError> { - let mut tx = self.db.begin().await?; + let to_not_found = || DeleteError::NotFound(conversation.clone()); + + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let conversation = tx .conversations() .by_id(conversation) .await - .not_found(|| DeleteError::NotFound(conversation.clone()))?; + .optional() + .fail("Failed to load conversation")? + .ok_or_else(to_not_found)?; - let messages = tx.messages().live(&conversation).await?; - let deleted_at = tx.sequence().next(deleted_at).await?; + let messages = tx + .messages() + .live(&conversation) + .await + .fail("Failed to load messages")?; + let deleted_at = tx + .sequence() + .next(deleted_at) + .await + .fail("Failed to find event sequence number")?; let has_messages = messages .iter() @@ -100,9 +120,12 @@ impl Conversations { let events = conversation .events() .filter(Sequence::start_from(deleted_at)); - tx.conversations().record_events(events.clone()).await?; + tx.conversations() + .record_events(events.clone()) + .await + .fail("Failed to store events")?; - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; self.events.broadcast_from(events); @@ -114,23 +137,33 @@ impl Conversations { // expired until their messages expire. let expire_at = relative_to.to_owned() - TimeDelta::days(7); - let mut tx = self.db.begin().await?; - let expired = tx.conversations().expired(&expire_at).await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + + let expired = tx + .conversations() + .expired(&expire_at) + .await + .fail("Failed to load expirable conversations")?; let mut events = Vec::with_capacity(expired.len()); for conversation in expired { - let deleted = tx.sequence().next(relative_to).await?; + let deleted = tx + .sequence() + .next(relative_to) + .await + .fail("Failed to find event sequence number")?; let conversation = conversation.delete(deleted)?; let conversation_events = conversation.events().filter(Sequence::start_from(deleted)); tx.conversations() .record_events(conversation_events.clone()) - .await?; + .await + .fail("Failed to store events")?; events.push(conversation_events); } - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; self.events .broadcast_from(events.into_iter().kmerge_by(Sequence::merge)); @@ -157,39 +190,17 @@ pub enum CreateError { #[error("invalid conversation name: {0}")] InvalidName(Name), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<LoadError> for CreateError { - fn from(error: LoadError) -> Self { - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } #[derive(Debug, thiserror::Error)] -pub enum Error { +pub enum GetError { #[error("conversation {0} not found")] NotFound(Id), #[error("conversation {0} deleted")] Deleted(Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<LoadError> for Error { - fn from(error: LoadError) -> Self { - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } #[derive(Debug, thiserror::Error)] @@ -201,51 +212,27 @@ pub enum DeleteError { #[error("conversation {0} not empty")] NotEmpty(Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<LoadError> for DeleteError { - fn from(error: LoadError) -> Self { - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } impl From<history::DeleteError> for DeleteError { fn from(error: history::DeleteError) -> Self { - match error { - history::DeleteError::Deleted(conversation) => Self::Deleted(conversation.id().clone()), - } + let history::DeleteError::Deleted(conversation) = error; + Self::Deleted(conversation.id().clone()) } } #[derive(Debug, thiserror::Error)] pub enum ExpireError { - #[error("tried to expire already-deleted conversation: {0}")] + #[error("tried to expire already-deleted conversation {0}")] Deleted(Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<LoadError> for ExpireError { - fn from(error: LoadError) -> Self { - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } impl From<history::DeleteError> for ExpireError { fn from(error: history::DeleteError) -> Self { - match error { - history::DeleteError::Deleted(conversation) => Self::Deleted(conversation.id().clone()), - } + let history::DeleteError::Deleted(conversation) = error; + Self::Deleted(conversation.id().clone()) } } diff --git a/src/conversation/handlers/create/mod.rs b/src/conversation/handlers/create/mod.rs index 2b7fa39..aa77411 100644 --- a/src/conversation/handlers/create/mod.rs +++ b/src/conversation/handlers/create/mod.rs @@ -57,9 +57,7 @@ impl IntoResponse for Error { app::CreateError::InvalidName(_) => { (StatusCode::BAD_REQUEST, error.to_string()).into_response() } - app::CreateError::Name(_) | app::CreateError::Database(_) => { - Internal::from(error).into_response() - } + app::CreateError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/conversation/handlers/delete/mod.rs b/src/conversation/handlers/delete/mod.rs index 231e433..2365303 100644 --- a/src/conversation/handlers/delete/mod.rs +++ b/src/conversation/handlers/delete/mod.rs @@ -50,9 +50,7 @@ impl IntoResponse for Error { app::DeleteError::NotEmpty(_) => { (StatusCode::CONFLICT, error.to_string()).into_response() } - app::DeleteError::Name(_) | app::DeleteError::Database(_) => { - Internal::from(error).into_response() - } + app::DeleteError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/conversation/handlers/send/mod.rs b/src/conversation/handlers/send/mod.rs index ff63652..979dd24 100644 --- a/src/conversation/handlers/send/mod.rs +++ b/src/conversation/handlers/send/mod.rs @@ -58,10 +58,9 @@ impl IntoResponse for Error { SendError::ConversationNotFound(_) | SendError::ConversationDeleted(_) => { NotFound(error).into_response() } - SendError::SenderNotFound(_) - | SendError::SenderDeleted(_) - | SendError::Name(_) - | SendError::Database(_) => Internal::from(error).into_response(), + SendError::SenderNotFound(_) | SendError::SenderDeleted(_) | SendError::Failed(_) => { + Internal::from(error).into_response() + } } } } diff --git a/src/db/failed.rs b/src/db/failed.rs new file mode 100644 index 0000000..f917ed1 --- /dev/null +++ b/src/db/failed.rs @@ -0,0 +1,2 @@ +pub const BEGIN: &str = "Failed to start transaction"; +pub const COMMIT: &str = "Failed to commit transaction"; diff --git a/src/db/mod.rs b/src/db/mod.rs index 9a74dcb..98ec618 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -1,9 +1,9 @@ mod backup; +pub mod failed; use std::str::FromStr; use sqlx::{ - error::{DatabaseError, ErrorKind}, migrate::MigrateDatabase as _, sqlite::{Sqlite, SqliteConnectOptions, SqlitePool, SqlitePoolOptions}, }; @@ -66,14 +66,6 @@ pub trait NotFound: Sized { type Ok; type Error; - fn not_found<E, F>(self, map: F) -> Result<Self::Ok, E> - where - E: From<Self::Error>, - F: FnOnce() -> E, - { - self.optional()?.ok_or_else(map) - } - fn optional(self) -> Result<Option<Self::Ok>, Self::Error>; } @@ -89,32 +81,3 @@ impl<T> NotFound for Result<T, sqlx::Error> { } } } - -pub trait Duplicate { - type Ok; - type Error; - - fn duplicate<E, F>(self, map: F) -> Result<Self::Ok, E> - where - E: From<Self::Error>, - F: FnOnce() -> E; -} - -impl<T> Duplicate for Result<T, sqlx::Error> { - type Ok = T; - type Error = sqlx::Error; - - fn duplicate<E, F>(self, map: F) -> Result<T, E> - where - E: From<sqlx::Error>, - F: FnOnce() -> E, - { - match self { - Ok(value) => Ok(value), - Err(error) => match error.as_database_error().map(DatabaseError::kind) { - Some(ErrorKind::UniqueViolation) => Err(map()), - _ => Err(error.into()), - }, - } - } -} diff --git a/src/error/chain.rs b/src/error/chain.rs new file mode 100644 index 0000000..d78fe67 --- /dev/null +++ b/src/error/chain.rs @@ -0,0 +1,72 @@ +use std::{error::Error, io}; + +use super::Id; + +pub fn format<W, E>(out: &mut W, error: E) -> Result<(), io::Error> +where + W: io::Write, + E: Error, +{ + writeln!(out, "{error}")?; + format_sources(out, error)?; + + Ok(()) +} + +pub fn format_with_id<W, E>(out: &mut W, id: &Id, error: E) -> Result<(), io::Error> +where + W: io::Write, + E: Error, +{ + writeln!(out, "[{id}] {error}")?; + format_sources(out, error)?; + + Ok(()) +} + +fn format_sources<W, E>(out: &mut W, error: E) -> Result<(), io::Error> +where + W: io::Write, + E: Error, +{ + let mut sources = Sources::from(&error); + if let Some(source) = sources.next() { + writeln!(out)?; + writeln!(out, "Caused by:")?; + writeln!(out, " {source}")?; + for source in sources { + writeln!(out, " {source}")?; + } + writeln!(out)?; + } + Ok(()) +} + +struct Sources<'e> { + next: Option<&'e dyn Error>, +} + +impl<'e, E> From<&'e E> for Sources<'e> +where + E: Error, +{ + fn from(error: &'e E) -> Self { + Self { + next: error.source(), + } + } +} + +// See also: <https://doc.rust-lang.org/std/error/trait.Error.html#method.sources> However, we only +// want to iterate the sources, and not the error itself. Personally, I find the `skip(1)` +// suggestion untidy, since the error itself is non-optional while the sources are optional. +impl<'a> Iterator for Sources<'a> { + type Item = &'a dyn Error; + + fn next(&mut self) -> Option<Self::Item> { + let source = self.next; + self.next = self.next.and_then(|err| err.source()); + + source + } +} diff --git a/src/error/failed.rs b/src/error/failed.rs new file mode 100644 index 0000000..4d55552 --- /dev/null +++ b/src/error/failed.rs @@ -0,0 +1,155 @@ +use std::{borrow::Cow, error::Error, fmt}; + +use super::BoxedError; + +// Don't bother allocating a String for each error if we have a static error message, which is +// a fairly common use case. +pub type Message = Cow<'static, str>; + +// Intended to represent a generic failure caused by some underlying cause, tagged with a +// context-appropriate message to help operators and developers diagnose and repair the issue. +// Unlike `Internal`, this error type has no default HTTP representation - it's not meant for that +// use case: the intended audience for a `Failed` and its underlying cause is the service operator, +// not the conversational users or client developers. +// +// The underlying cause will be used as the `Failed` error's `source()`, and the provided message +// will be used as its `Display` implementation. +// +// Ways to make a `Failed`: +// +// * Call `ResultExt::fail` or `ResultExt::fail_with` on a result to convert its error type (most +// preferred). +// * Call `ErrorExt::fail` to convert an existing error into `Failed`. +// * Call `Result::map_err` with `Failed::with_message` or `Failed::with_message_from` to convert +// its error type to `Failed`. +// * Call `Failed::new` with a message and an existing error (least preferred). +// +// Ways to use `Failed`: +// +// * As the error type of a `Result` return value, when all errors that can arise in a function +// are of interest to the operator only but there are multiple failure types to contend with. +// * As the error type of a `Result` return value, when there's only one error that can arise but +// you want to annotate that error with a contextual message, like what the function was doing at +// the time. +// * As a variant in am error enum, when _some_ errors are only of operator interest but other +// errors may need to be broken out for matching, or returned to clients. +// +// When not to use `Failed`: +// +// * When the caller needs to match on the specific error type. `Failed`, by design, assumes that +// nothing up the call stack will care about the structure of the error, and while you _can_ get +// at the underlying error to check what you ahve by going through the `source()` chain, it's +// tricky to do well. If the specific error is relevant to the caller, surface it through the +// error type, instead. +#[derive(Debug)] +pub struct Failed { + source: BoxedError, + message: Message, +} + +impl Failed { + pub fn new<M, E>(message: M, source: E) -> Self + where + M: Into<Message>, + E: Into<BoxedError>, + { + Self { + source: source.into(), + message: message.into(), + } + } +} + +// Adapters for use with `Result::map_err`. +impl Failed { + // The returned adaptor wraps an error with `Failed`, using the provided message. + pub fn with_message<M, E>(message: M) -> impl FnOnce(E) -> Self + where + M: Into<Message>, + E: Into<BoxedError>, + { + |source| Self::new(message, source) + } + + // The returned adaptor wraps an error with `Failed`, using a message from the provided message + // callback. + pub fn with_message_from<F, M, E>(message_fn: F) -> impl FnOnce(E) -> Self + where + F: FnOnce() -> M, + M: Into<Message>, + E: Into<BoxedError>, + { + |source| Self::new(message_fn(), source) + } +} + +impl Error for Failed { + fn source(&self) -> Option<&(dyn Error + 'static)> { + Some(self.source.as_ref()) + } +} + +impl fmt::Display for Failed { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.message.fmt(f) + } +} + +pub trait ErrorExt { + // Convenience for `Failed::new(message, err).into()`, when wrapping an existing error into + // `Failed`. + fn fail<M, T>(self, message: M) -> T + where + M: Into<Message>, + T: From<Failed>; +} + +impl<E> ErrorExt for E +where + E: Into<BoxedError>, +{ + fn fail<M, T>(self, message: M) -> T + where + M: Into<Message>, + T: From<Failed>, + { + Failed::new(message, self).into() + } +} + +pub trait ResultExt { + type Ok; + + // Convenience for `.map_err(Failed::with_message(message))`, when a result's error type can be + // collapsed into `Failed`. + fn fail<M>(self, message: M) -> Result<Self::Ok, Failed> + where + M: Into<Message>; + + fn fail_with<F, M>(self, message_fn: F) -> Result<Self::Ok, Failed> + where + F: FnOnce() -> M, + M: Into<Message>; +} + +impl<T, E> ResultExt for Result<T, E> +where + E: Into<BoxedError>, +{ + type Ok = T; + + fn fail<M>(self, message: M) -> Result<T, Failed> + where + M: Into<Message>, + { + self.map_err(Failed::with_message(message)) + } + + fn fail_with<F, M>(self, message_fn: F) -> Result<T, Failed> + where + F: FnOnce() -> M, + M: Into<Message>, + { + self.map_err(Failed::with_message_from(message_fn)) + } +} diff --git a/src/error.rs b/src/error/mod.rs index 3c46097..606e344 100644 --- a/src/error.rs +++ b/src/error/mod.rs @@ -1,10 +1,13 @@ -use std::{error, fmt}; +use std::{error, fmt, io}; use axum::{ http::StatusCode, response::{IntoResponse, Response}, }; +pub mod chain; +pub mod failed; + // I'm making an effort to avoid `anyhow` here, as that crate is _enormously_ // complex (though very usable). We don't need to be overly careful about // allocations on errors in this app, so this is fine for most "general @@ -38,7 +41,8 @@ impl fmt::Display for Internal { impl IntoResponse for Internal { fn into_response(self) -> Response { let Self(id, error) = &self; - eprintln!("pilcrow: [{id}] {error}"); + chain::format_with_id(&mut io::stderr().lock(), id, error.as_ref()) + .expect("write to stderr"); (StatusCode::INTERNAL_SERVER_ERROR, self.to_string()).into_response() } } diff --git a/src/event/app.rs b/src/event/app.rs index e422de9..386d749 100644 --- a/src/event/app.rs +++ b/src/event/app.rs @@ -8,9 +8,9 @@ use sqlx::sqlite::SqlitePool; use super::{Event, Sequence, Sequenced, broadcaster::Broadcaster}; use crate::{ conversation::{self, repo::Provider as _}, - db::NotFound, + db::{self, NotFound as _}, + error::failed::{Failed, ResultExt as _}, message::{self, repo::Provider as _}, - name, user::{self, repo::Provider as _}, vapid, vapid::repo::Provider as _, @@ -29,14 +29,18 @@ impl Events { pub async fn subscribe( &self, resume_at: Sequence, - ) -> Result<impl Stream<Item = Event> + std::fmt::Debug + use<>, Error> { + ) -> Result<impl Stream<Item = Event> + std::fmt::Debug + use<>, Failed> { // Subscribe before retrieving, to catch messages broadcast while we're // querying the DB. We'll prune out duplicates later. let live_messages = self.events.subscribe(); - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; - let users = tx.users().replay(resume_at).await?; + let users = tx + .users() + .replay(resume_at) + .await + .fail("Failed to load user events")?; let user_events = users .iter() .map(user::History::events) @@ -44,7 +48,11 @@ impl Events { .filter(Sequence::after(resume_at)) .map(Event::from); - let conversations = tx.conversations().replay(resume_at).await?; + let conversations = tx + .conversations() + .replay(resume_at) + .await + .fail("Failed to load conversation events")?; let conversation_events = conversations .iter() .map(conversation::History::events) @@ -52,7 +60,11 @@ impl Events { .filter(Sequence::after(resume_at)) .map(Event::from); - let messages = tx.messages().replay(resume_at).await?; + let messages = tx + .messages() + .replay(resume_at) + .await + .fail("Failed to load message events")?; let message_events = messages .iter() .map(message::History::events) @@ -60,7 +72,12 @@ impl Events { .filter(Sequence::after(resume_at)) .map(Event::from); - let vapid = tx.vapid().current().await.optional()?; + let vapid = tx + .vapid() + .current() + .await + .optional() + .fail("Failed to load VAPID key events")?; let vapid_events = vapid .iter() .flat_map(vapid::History::events) @@ -91,45 +108,3 @@ impl Events { move |event| future::ready(filter(event)) } } - -#[derive(Debug, thiserror::Error)] -#[error(transparent)] -pub enum Error { - Database(#[from] sqlx::Error), - Name(#[from] name::Error), - Ecdsa(#[from] p256::ecdsa::Error), - Pkcs8(#[from] p256::pkcs8::Error), - WebPush(#[from] web_push::WebPushError), -} - -impl From<user::repo::LoadError> for Error { - fn from(error: user::repo::LoadError) -> Self { - use user::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } -} - -impl From<conversation::repo::LoadError> for Error { - fn from(error: conversation::repo::LoadError) -> Self { - use conversation::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } -} - -impl From<vapid::repo::Error> for Error { - fn from(error: vapid::repo::Error) -> Self { - use vapid::repo::Error; - match error { - Error::Database(error) => error.into(), - Error::Ecdsa(error) => error.into(), - Error::Pkcs8(error) => error.into(), - Error::WebPush(error) => error.into(), - } - } -} diff --git a/src/event/handlers/stream/mod.rs b/src/event/handlers/stream/mod.rs index 8b89c31..dde4fae 100644 --- a/src/event/handlers/stream/mod.rs +++ b/src/event/handlers/stream/mod.rs @@ -10,8 +10,8 @@ use futures::stream::{Stream, StreamExt as _}; use crate::{ app::App, - error::{Internal, Unauthorized}, - event::{Event, Heartbeat::Heartbeat, Sequence, Sequenced as _, app, extract::LastEventId}, + error::{Internal, Unauthorized, failed::Failed}, + event::{Event, Heartbeat::Heartbeat, Sequence, Sequenced as _, extract::LastEventId}, token::{app::ValidateError, extract::Identity}, }; @@ -71,7 +71,7 @@ impl TryFrom<Event> for sse::Event { #[derive(Debug, thiserror::Error)] #[error(transparent)] pub enum Error { - Subscribe(#[from] app::Error), + Subscribe(#[from] Failed), Validate(#[from] ValidateError), } @@ -79,7 +79,9 @@ impl IntoResponse for Error { fn into_response(self) -> response::Response { match self { Self::Validate(ValidateError::InvalidToken) => Unauthorized.into_response(), - other => Internal::from(other).into_response(), + Self::Validate(ValidateError::Failed(_)) | Self::Subscribe(_) => { + Internal::from(self).into_response() + } } } } diff --git a/src/exit.rs b/src/exit.rs new file mode 100644 index 0000000..3c7b724 --- /dev/null +++ b/src/exit.rs @@ -0,0 +1,54 @@ +use std::{ + error::Error, + io, + process::{ExitCode, Termination}, +}; + +use crate::error::chain; + +/// Formats errors for display before exiting the program. +/// +/// When this is used as the return type of a program's `main`, it can be used to capture any errors +/// during the execution of the program and to display them in a readable format at exit time: +/// +/// ```no_run +/// # use std::{error::Error, io}; +/// fn main() -> pilcrow::cli::Exit<impl Error> { +/// my_complicated_task().into() +/// } +/// +/// fn my_complicated_task() -> Result<(), impl Error> { +/// Err(io::Error::other("stand-in for a real failure")) +/// } +/// ``` +/// +/// If constructed with an `Ok(())`, the resulting `Exit` indicates successful execution, and, when +/// returned from `main`, will cause the program to exit with a successful exit status. If constructed +/// with any `Err(…)` value, the resulting `Exit` indicates unsuccessful execution, and, when +/// returned from `main`, will cause the program to print the error (along with its `source()` +/// chain, if any) before exiting with an unsuccessful exit status. +pub struct Exit<E>(pub Result<(), E>); + +impl<E> From<Result<(), E>> for Exit<E> { + fn from(result: Result<(), E>) -> Self { + Self(result) + } +} + +impl<E> Termination for Exit<E> +where + E: Error, +{ + fn report(self) -> ExitCode { + let Self(result) = self; + match result { + Ok(()) => ExitCode::SUCCESS, + Err(err) => { + // if we can't write the error message to stderr, there's nothing else we can do + // instead, and we're about to exit with a failure anyway. + let _ = chain::format(&mut io::stderr().lock(), &err); + ExitCode::FAILURE + } + } + } +} diff --git a/src/invite/app.rs b/src/invite/app.rs index 6f58d0a..54272bc 100644 --- a/src/invite/app.rs +++ b/src/invite/app.rs @@ -4,17 +4,14 @@ use sqlx::sqlite::SqlitePool; use super::{Id, Invite, Summary, repo::Provider as _}; use crate::{ clock::DateTime, - db::{Duplicate as _, NotFound as _}, + db::{self, NotFound as _}, + error::failed::{ErrorExt as _, Failed, ResultExt as _}, event::{Broadcaster, repo::Provider as _}, login::Login, - name::{self, Name}, + name::Name, password::Password, token::{Secret, Token, repo::Provider as _}, - user::{ - self, - create::{self, Create}, - repo::{LoadError, Provider as _}, - }, + user::{self, create, create::Create, repo::Provider as _}, }; pub struct Invites { @@ -27,20 +24,30 @@ impl Invites { Self { db, events } } - pub async fn issue(&self, issuer: &Login, issued_at: &DateTime) -> Result<Invite, Error> { - let issuer_not_found = || Error::IssuerNotFound(issuer.id.clone().into()); - let issuer_deleted = || Error::IssuerDeleted(issuer.id.clone().into()); + pub async fn issue(&self, issuer: &Login, issued_at: &DateTime) -> Result<Invite, IssueError> { + let issuer_not_found = || IssueError::IssuerNotFound(issuer.id.clone().into()); + let issuer_deleted = || IssueError::IssuerDeleted(issuer.id.clone().into()); - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let issuer = tx .users() .by_login(issuer) .await - .not_found(issuer_not_found)?; - let now = tx.sequence().current().await?; + .optional() + .fail("Failed to load issuing user")? + .ok_or_else(issuer_not_found)?; + let now = tx + .sequence() + .current() + .await + .fail("Failed to find event sequence number")?; let issuer = issuer.as_of(now).ok_or_else(issuer_deleted)?; - let invite = tx.invites().create(&issuer, issued_at).await?; - tx.commit().await?; + let invite = tx + .invites() + .create(&issuer, issued_at) + .await + .fail("Failed to store new invitation")?; + tx.commit().await.fail(db::failed::COMMIT)?; Ok(invite) } @@ -60,33 +67,52 @@ impl Invites { password: &Password, accepted_at: &DateTime, ) -> Result<Secret, AcceptError> { + let to_not_found = || AcceptError::NotFound(invite.clone()); + let create = Create::begin(name, password, accepted_at); - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let invite = tx .invites() .by_id(invite) .await - .not_found(|| AcceptError::NotFound(invite.clone()))?; + .optional() + .fail("Failed to load invitation")? + .ok_or_else(to_not_found)?; // Split the tx here so we don't block writes while we deal with the password, // and don't deal with the password until we're fairly confident we can accept // the invite. Final validation is in the next tx. - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; - let validated = create.validate()?; + let validated = create.validate().map_err(|err| match err { + create::Error::InvalidName(name) => AcceptError::InvalidName(name), + create::Error::Failed(_) => err.fail("Failed to validate invited user"), + })?; - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; // If the invite has been deleted or accepted in the interim, this step will // catch it. - tx.invites().accept(&invite).await?; - let stored = validated - .store(&mut tx) + tx.invites() + .accept(&invite) .await - .duplicate(|| AcceptError::DuplicateLogin(name.clone()))?; + .fail("Failed to remove accepted invitation")?; + let stored = + validated + .store(&mut tx) + .await + .map_err(|err| match err.as_database_error() { + Some(err) if err.is_unique_violation() => { + AcceptError::DuplicateLogin(name.clone()) + } + _ => err.fail("Failed to store invited user"), + })?; let login = stored.login(); let (token, secret) = Token::generate(login, accepted_at); - tx.tokens().create(&token, &secret).await?; - tx.commit().await?; + tx.tokens() + .create(&token, &secret) + .await + .fail("Failed to issue token for invited user")?; + tx.commit().await.fail(db::failed::COMMIT)?; stored.publish(&self.events); @@ -106,25 +132,13 @@ impl Invites { } #[derive(Debug, thiserror::Error)] -pub enum Error { +pub enum IssueError { #[error("issuing user {0} not found")] IssuerNotFound(user::Id), #[error("issuing user {0} deleted")] IssuerDeleted(user::Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<user::repo::LoadError> for Error { - fn from(error: LoadError) -> Self { - use user::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } #[derive(Debug, thiserror::Error)] @@ -136,16 +150,5 @@ pub enum AcceptError { #[error("name in use: {0}")] DuplicateLogin(Name), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - PasswordHash(#[from] password_hash::Error), -} - -impl From<create::Error> for AcceptError { - fn from(error: create::Error) -> Self { - match error { - create::Error::InvalidName(name) => Self::InvalidName(name), - create::Error::PasswordHash(error) => Self::PasswordHash(error), - } - } + Failed(#[from] Failed), } diff --git a/src/invite/handlers/accept/mod.rs b/src/invite/handlers/accept/mod.rs index 8bdaa51..34f1e94 100644 --- a/src/invite/handlers/accept/mod.rs +++ b/src/invite/handlers/accept/mod.rs @@ -53,7 +53,7 @@ impl IntoResponse for Error { app::AcceptError::DuplicateLogin(_) => { (StatusCode::CONFLICT, error.to_string()).into_response() } - other => Internal::from(other).into_response(), + app::AcceptError::Failed(_) => Internal::from(error).into_response(), } } } @@ -12,6 +12,7 @@ mod db; mod empty; mod error; mod event; +pub mod exit; mod expire; mod id; mod invite; diff --git a/src/login/app.rs b/src/login/app.rs index 8cc8cd0..5c012c4 100644 --- a/src/login/app.rs +++ b/src/login/app.rs @@ -2,9 +2,11 @@ use sqlx::sqlite::SqlitePool; use crate::{ clock::DateTime, + db, db::NotFound as _, - login::{self, Login, repo::Provider as _}, - name::{self, Name}, + error::failed::{Failed, ResultExt as _}, + login::{Login, repo::Provider as _}, + name::Name, password::Password, push::repo::Provider as _, token::{Broadcaster, Event as TokenEvent, Secret, Token, repo::Provider as _}, @@ -26,24 +28,32 @@ impl Logins { candidate: &Password, login_at: &DateTime, ) -> Result<Secret, LoginError> { - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let (login, password) = tx .logins() .by_name(name) .await - .not_found(|| LoginError::Rejected)?; + .optional() + .fail("Failed to load login")? + .ok_or_else(|| LoginError::Rejected)?; // Split the transaction here to avoid holding the tx open (potentially blocking // other writes) while we do the fairly expensive task of verifying the // password. It's okay if the token issuance transaction happens some notional // amount of time after retrieving the login, as inserting the token will fail // if the account is deleted during that time. - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; - if password.verify(candidate)? { - let mut tx = self.db.begin().await?; + if password + .verify(candidate) + .fail("Failed to verify password")? + { + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let (token, secret) = Token::generate(&login, login_at); - tx.tokens().create(&token, &secret).await?; - tx.commit().await?; + tx.tokens() + .create(&token, &secret) + .await + .fail("Failed to create token")?; + tx.commit().await.fail(db::failed::COMMIT)?; Ok(secret) } else { Err(LoginError::Rejected) @@ -57,30 +67,47 @@ impl Logins { to: &Password, changed_at: &DateTime, ) -> Result<Secret, LoginError> { - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let (login, password) = tx .logins() .by_id(&login.id) .await - .not_found(|| LoginError::Rejected)?; + .optional() + .fail("Failed to load login")? + .ok_or_else(|| LoginError::Rejected)?; // Split the transaction here to avoid holding the tx open (potentially blocking // other writes) while we do the fairly expensive task of verifying the // password. It's okay if the token issuance transaction happens some notional // amount of time after retrieving the login, as inserting the token will fail // if the account is deleted during that time. - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; - if password.verify(from)? { - let to_hash = to.hash()?; + if password + .verify(from) + .fail("Failed to verify current password")? + { + let to_hash = to.hash().fail("Failed to digest new password")?; let (token, secret) = Token::generate(&login, changed_at); - let mut tx = self.db.begin().await?; - tx.logins().set_password(&login, &to_hash).await?; - - tx.push().unsubscribe_login(&login).await?; - let revoked = tx.tokens().revoke_all(&login).await?; - tx.tokens().create(&token, &secret).await?; - tx.commit().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + tx.logins() + .set_password(&login, &to_hash) + .await + .fail("Failed to store new password")?; + tx.push() + .unsubscribe_login(&login) + .await + .fail("Failed to remove push notification subscriptions")?; + let revoked = tx + .tokens() + .revoke_all(&login) + .await + .fail("Failed to revoke existing tokens")?; + tx.tokens() + .create(&token, &secret) + .await + .fail("Failed to create new token")?; + tx.commit().await.fail(db::failed::COMMIT)?; revoked .into_iter() @@ -99,19 +126,5 @@ pub enum LoginError { #[error("invalid login")] Rejected, #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), - #[error(transparent)] - PasswordHash(#[from] password_hash::Error), -} - -impl From<login::repo::LoadError> for LoginError { - fn from(error: login::repo::LoadError) -> Self { - use login::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } diff --git a/src/login/handlers/login/mod.rs b/src/login/handlers/login/mod.rs index 2ce8a67..d42cff5 100644 --- a/src/login/handlers/login/mod.rs +++ b/src/login/handlers/login/mod.rs @@ -49,7 +49,7 @@ impl IntoResponse for Error { // not error::Unauthorized due to differing messaging (StatusCode::UNAUTHORIZED, "invalid name or password").into_response() } - other => Internal::from(other).into_response(), + app::LoginError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/login/handlers/logout/mod.rs b/src/login/handlers/logout/mod.rs index ce4cb1a..4bd7a89 100644 --- a/src/login/handlers/logout/mod.rs +++ b/src/login/handlers/logout/mod.rs @@ -42,9 +42,7 @@ impl IntoResponse for Error { let Self(error) = self; match error { app::ValidateError::InvalidToken => Unauthorized.into_response(), - app::ValidateError::Name(_) | app::ValidateError::Database(_) => { - Internal::from(error).into_response() - } + app::ValidateError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/login/handlers/password/mod.rs b/src/login/handlers/password/mod.rs index 8b82605..7eed24a 100644 --- a/src/login/handlers/password/mod.rs +++ b/src/login/handlers/password/mod.rs @@ -48,7 +48,7 @@ impl IntoResponse for Error { app::LoginError::Rejected => { (StatusCode::BAD_REQUEST, "invalid name or password").into_response() } - other => Internal::from(other).into_response(), + app::LoginError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/main.rs b/src/main.rs index f140da1..c7a3602 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,8 +1,8 @@ use clap::Parser; -use pilcrow::cli; +use pilcrow::cli::{self, Exit}; #[tokio::main] -async fn main() -> Result<(), impl std::error::Error> { +async fn main() -> Exit<impl std::error::Error> { let args = cli::Args::parse(); - args.run().await + args.run().await.into() } diff --git a/src/message/app.rs b/src/message/app.rs index cbcbff9..b82fa83 100644 --- a/src/message/app.rs +++ b/src/message/app.rs @@ -6,10 +6,11 @@ use super::{Body, History, Id, Message, history, repo::Provider as _}; use crate::{ clock::DateTime, conversation::{self, repo::Provider as _}, + db, db::NotFound as _, + error::failed::{Failed, ResultExt as _}, event::{Broadcaster, Sequence, repo::Provider as _}, login::Login, - name, user::{self, repo::Provider as _}, }; @@ -35,21 +36,29 @@ impl Messages { let sender_not_found = || SendError::SenderNotFound(sender.id.clone().into()); let sender_deleted = || SendError::SenderDeleted(sender.id.clone().into()); - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let conversation = tx .conversations() .by_id(conversation) .await - .not_found(conversation_not_found)?; + .optional() + .fail("Failed to load conversation")? + .ok_or_else(conversation_not_found)?; let sender = tx .users() .by_login(sender) .await - .not_found(sender_not_found)?; + .optional() + .fail("Failed to load sending user")? + .ok_or_else(sender_not_found)?; // Ordering: don't bother allocating a sequence number before we know the channel might // exist. - let sent = tx.sequence().next(sent_at).await?; + let sent = tx + .sequence() + .next(sent_at) + .await + .fail("Failed to find event sequence number")?; let conversation = conversation.as_of(sent).ok_or_else(conversation_deleted)?; let sender = sender.as_of(sent).ok_or_else(sender_deleted)?; let message = History::begin(&conversation, &sender, body, sent); @@ -58,9 +67,12 @@ impl Messages { // the various event-manipulating app methods are consistent, and it's harmless to have an // always-satisfied filter. let events = message.events().filter(Sequence::start_from(sent)); - tx.messages().record_events(events.clone()).await?; + tx.messages() + .record_events(events.clone()) + .await + .fail("Failed to store events")?; - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; self.events.broadcast_from(events); @@ -76,20 +88,29 @@ impl Messages { let message_not_found = || DeleteError::MessageNotFound(message.clone()); let not_sender = || DeleteError::NotSender(deleted_by.id.clone().into()); - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let message = tx .messages() .by_id(message) .await - .not_found(message_not_found)?; + .optional() + .fail("Failed to load message")? + .ok_or_else(message_not_found)?; if message.sender() == &deleted_by.id { - let deleted_at = tx.sequence().next(deleted_at).await?; + let deleted_at = tx + .sequence() + .next(deleted_at) + .await + .fail("Failed to find event sequence number")?; let message = message.delete(deleted_at)?; let events = message.events().filter(Sequence::start_from(deleted_at)); - tx.messages().record_events(events.clone()).await?; - tx.commit().await?; + tx.messages() + .record_events(events.clone()) + .await + .fail("Failed to store events")?; + tx.commit().await.fail(db::failed::COMMIT)?; self.events.broadcast_from(events); @@ -153,29 +174,7 @@ pub enum SendError { #[error("user {0} deleted")] SenderDeleted(user::Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<conversation::repo::LoadError> for SendError { - fn from(error: conversation::repo::LoadError) -> Self { - use conversation::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } -} - -impl From<user::repo::LoadError> for SendError { - fn from(error: user::repo::LoadError) -> Self { - use user::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } #[derive(Debug, thiserror::Error)] @@ -187,19 +186,7 @@ pub enum DeleteError { #[error("message {0} deleted")] Deleted(Id), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<user::repo::LoadError> for DeleteError { - fn from(error: user::repo::LoadError) -> Self { - use user::repo::LoadError; - match error { - LoadError::Database(error) => error.into(), - LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } impl From<history::DeleteError> for DeleteError { diff --git a/src/message/handlers/delete/mod.rs b/src/message/handlers/delete/mod.rs index c09a752..c680db1 100644 --- a/src/message/handlers/delete/mod.rs +++ b/src/message/handlers/delete/mod.rs @@ -53,9 +53,7 @@ impl IntoResponse for Error { DeleteError::MessageNotFound(_) | DeleteError::Deleted(_) => { NotFound(error).into_response() } - DeleteError::Database(_) | DeleteError::Name(_) => { - Internal::from(error).into_response() - } + DeleteError::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/push/app.rs b/src/push/app.rs index 56b9a02..5e57800 100644 --- a/src/push/app.rs +++ b/src/push/app.rs @@ -8,7 +8,14 @@ use web_push::{ }; use super::repo::Provider as _; -use crate::{login::Login, token::extract::Identity, vapid, vapid::repo::Provider as _}; +use crate::{ + db, + error::failed::{ErrorExt, Failed, ResultExt as _}, + login::Login, + token::extract::Identity, + vapid, + vapid::repo::Provider as _, +}; pub struct Push<P> { db: SqlitePool, @@ -26,9 +33,13 @@ impl<P> Push<P> { subscription: &SubscriptionInfo, vapid: &VerifyingKey, ) -> Result<(), SubscribeError> { - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; - let current = tx.vapid().current().await?; + let current = tx + .vapid() + .current() + .await + .fail("Failed to load current VAPID key")?; if vapid != ¤t.key { return Err(SubscribeError::StaleVapidKey(current.key)); } @@ -42,7 +53,8 @@ impl<P> Push<P> { let current = tx .push() .by_endpoint(&subscriber.login, &subscription.endpoint) - .await?; + .await + .fail("Failed to load existing subscriptions for endpoint")?; // If we already have a subscription for this endpoint, with _different_ // parameters, then this is a client error. They shouldn't reuse endpoint URLs, // per the various RFCs. @@ -55,12 +67,12 @@ impl<P> Push<P> { return Err(SubscribeError::Duplicate); } } else { - return Err(SubscribeError::Database(err)); + return Err(err.fail("Failed to create push subscription")); } } } - tx.commit().await?; + tx.commit().await.fail(db::failed::COMMIT)?; Ok(()) } @@ -138,10 +150,6 @@ where #[derive(Debug, thiserror::Error)] pub enum SubscribeError { - #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Vapid(#[from] vapid::repo::Error), #[error("subscription created with stale VAPID key")] StaleVapidKey(VerifyingKey), #[error("subscription already exists for endpoint")] @@ -149,6 +157,8 @@ pub enum SubscribeError { // and we want to limit its proliferation. The only intended recipient of this message is the // client, which already knows the endpoint anyways and doesn't need us to tell them. Duplicate, + #[error(transparent)] + Failed(#[from] Failed), } #[derive(Debug, thiserror::Error)] diff --git a/src/push/handlers/subscribe/mod.rs b/src/push/handlers/subscribe/mod.rs index a1a5899..6dcab06 100644 --- a/src/push/handlers/subscribe/mod.rs +++ b/src/push/handlers/subscribe/mod.rs @@ -81,7 +81,7 @@ impl IntoResponse for Error { app::SubscribeError::Duplicate => { (StatusCode::CONFLICT, err.to_string()).into_response() } - other => Internal::from(other).into_response(), + app::SubscribeError::Failed(_) => Internal::from(err).into_response(), } } } diff --git a/src/setup/app.rs b/src/setup/app.rs index 9539406..a0245f7 100644 --- a/src/setup/app.rs +++ b/src/setup/app.rs @@ -3,11 +3,13 @@ use sqlx::sqlite::SqlitePool; use super::repo::Provider as _; use crate::{ clock::DateTime, + db, + error::failed::{ErrorExt, Failed, ResultExt as _}, event::Broadcaster, name::Name, password::Password, token::{Secret, Token, repo::Provider as _}, - user::create::{self, Create}, + user::{create, create::Create}, }; #[derive(Clone)] @@ -29,18 +31,32 @@ impl Setup { ) -> Result<Secret, Error> { let create = Create::begin(name, password, created_at); - let validated = create.validate()?; + let validated = create.validate().map_err(|err| match err { + create::Error::InvalidName(name) => Error::InvalidName(name), + create::Error::Failed(_) => err.fail("Failed to validate new user"), + })?; - let mut tx = self.db.begin().await?; - if tx.setup().completed().await? { + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + if tx + .setup() + .completed() + .await + .fail("Failed to check for prior setup completion")? + { Err(Error::SetupCompleted) } else { - let stored = validated.store(&mut tx).await?; + let stored = validated + .store(&mut tx) + .await + .fail("Failed to create initial user")?; let login = stored.login(); let (token, secret) = Token::generate(login, created_at); - tx.tokens().create(&token, &secret).await?; - tx.commit().await?; + tx.tokens() + .create(&token, &secret) + .await + .fail("Failed to create token")?; + tx.commit().await.fail(db::failed::COMMIT)?; stored.publish(&self.events); @@ -64,16 +80,5 @@ pub enum Error { #[error("invalid user name: {0}")] InvalidName(Name), #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - PasswordHash(#[from] password_hash::Error), -} - -impl From<create::Error> for Error { - fn from(error: create::Error) -> Self { - match error { - create::Error::InvalidName(name) => Self::InvalidName(name), - create::Error::PasswordHash(error) => Self::PasswordHash(error), - } - } + Failed(#[from] Failed), } diff --git a/src/setup/handlers/setup/mod.rs b/src/setup/handlers/setup/mod.rs index 2977da8..ce57e17 100644 --- a/src/setup/handlers/setup/mod.rs +++ b/src/setup/handlers/setup/mod.rs @@ -48,7 +48,7 @@ impl IntoResponse for Error { (StatusCode::BAD_REQUEST, error.to_string()).into_response() } app::Error::SetupCompleted => (StatusCode::CONFLICT, error.to_string()).into_response(), - other => Internal::from(other).into_response(), + app::Error::Failed(_) => Internal::from(error).into_response(), } } } diff --git a/src/token/app.rs b/src/token/app.rs index 4a08877..5916e53 100644 --- a/src/token/app.rs +++ b/src/token/app.rs @@ -6,11 +6,15 @@ use futures::{ use sqlx::sqlite::SqlitePool; use super::{ - Broadcaster, Event as TokenEvent, Secret, Token, - extract::Identity, - repo::{self, Provider as _}, + Broadcaster, Event as TokenEvent, Secret, Token, extract::Identity, repo::Provider as _, +}; +use crate::{ + clock::DateTime, + db, + db::NotFound as _, + error::failed::{Failed, ResultExt as _}, + push::repo::Provider as _, }; -use crate::{clock::DateTime, db::NotFound as _, name, push::repo::Provider as _}; pub struct Tokens { db: SqlitePool, @@ -27,13 +31,15 @@ impl Tokens { secret: &Secret, used_at: &DateTime, ) -> Result<Identity, ValidateError> { - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; let (token, login) = tx .tokens() .validate(secret, used_at) .await - .not_found(|| ValidateError::InvalidToken)?; - tx.commit().await?; + .optional() + .fail("Failed to load token")? + .ok_or(ValidateError::InvalidToken)?; + tx.commit().await.fail(db::failed::COMMIT)?; Ok(Identity { token, login }) } @@ -66,12 +72,14 @@ impl Tokens { // matter. Supervising a stream, on the other hand, will run for a // _long_ time; if we miss the race here, we'll never actually carry out the // supervision. - let mut tx = self.db.begin().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; tx.tokens() .require(&token) .await - .not_found(|| ValidateError::InvalidToken)?; - tx.commit().await?; + .optional() + .fail("Failed to load token")? + .ok_or(ValidateError::InvalidToken)?; + tx.commit().await.fail(db::failed::COMMIT)?; // Then construct the guarded stream. First, project both streams into // `GuardedEvent`. @@ -111,10 +119,16 @@ impl Tokens { } pub async fn logout(&self, token: &Token) -> Result<(), ValidateError> { - let mut tx = self.db.begin().await?; - tx.push().unsubscribe_token(token).await?; - tx.tokens().revoke(token).await?; - tx.commit().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + tx.push() + .unsubscribe_token(token) + .await + .fail("Failed to remove push subscriptions")?; + tx.tokens() + .revoke(token) + .await + .fail("Failed to revoke token")?; + tx.commit().await.fail(db::failed::COMMIT)?; self.token_events .broadcast(TokenEvent::Revoked(token.id.clone())); @@ -128,18 +142,7 @@ pub enum ValidateError { #[error("invalid token")] InvalidToken, #[error(transparent)] - Database(#[from] sqlx::Error), - #[error(transparent)] - Name(#[from] name::Error), -} - -impl From<repo::LoadError> for ValidateError { - fn from(error: repo::LoadError) -> Self { - match error { - repo::LoadError::Database(error) => error.into(), - repo::LoadError::Name(error) => error.into(), - } - } + Failed(#[from] Failed), } #[derive(Debug)] diff --git a/src/token/extract/identity.rs b/src/token/extract/identity.rs index 5c004ef..59f1dfb 100644 --- a/src/token/extract/identity.rs +++ b/src/token/extract/identity.rs @@ -40,7 +40,7 @@ where .await .map_err(|err| match err { ValidateError::InvalidToken => LoginError::Unauthorized, - other => other.into(), + ValidateError::Failed(failed) => failed.into(), }) } } @@ -59,7 +59,7 @@ where match <Self as FromRequestParts<App>>::from_request_parts(parts, state).await { Ok(identity) => Ok(Some(identity)), Err(LoginError::Unauthorized) => Ok(None), - Err(other) => Err(other), + Err(LoginError::Failure(other)) => Err(LoginError::Failure(other)), } } } diff --git a/src/token/repo/mod.rs b/src/token/repo/mod.rs index 9df5bbb..50d9147 100644 --- a/src/token/repo/mod.rs +++ b/src/token/repo/mod.rs @@ -1,3 +1,3 @@ mod token; -pub use self::token::{LoadError, Provider}; +pub use self::token::Provider; diff --git a/src/ui/assets.rs b/src/ui/assets.rs index 0ca9593..ede1f7c 100644 --- a/src/ui/assets.rs +++ b/src/ui/assets.rs @@ -1,4 +1,4 @@ -use ::mime::{FromStrError, Mime}; +use ::mime::Mime; use axum::{ http::{StatusCode, header}, response::{IntoResponse, Response}, @@ -6,7 +6,10 @@ use axum::{ use rust_embed::EmbeddedFile; use super::{error::NotFound, mime}; -use crate::error::Internal; +use crate::error::{ + Internal, + failed::{Failed, ResultExt}, +}; #[derive(rust_embed::Embed)] #[folder = "$OUT_DIR/ui"] @@ -15,7 +18,7 @@ pub struct Assets; impl Assets { pub fn load(path: impl AsRef<str>) -> Result<Asset, Error> { let path = path.as_ref(); - let mime = mime::from_path(path)?; + let mime = mime::from_path(path).fail("Failed to determine MIME type from asset path")?; Self::get(path) .map(|file| Asset(mime, file)) @@ -49,14 +52,14 @@ pub enum Error { #[error("not found: {0}")] NotFound(String), #[error(transparent)] - Mime(#[from] FromStrError), + Failed(#[from] Failed), } impl IntoResponse for Error { fn into_response(self) -> Response { match self { Self::NotFound(_) => NotFound(self.to_string()).into_response(), - Self::Mime(_) => Internal::from(self).into_response(), + Self::Failed(_) => Internal::from(self).into_response(), } } } diff --git a/src/ui/handlers/conversation.rs b/src/ui/handlers/conversation.rs index 2ff090c..102efc6 100644 --- a/src/ui/handlers/conversation.rs +++ b/src/ui/handlers/conversation.rs @@ -37,11 +37,11 @@ pub enum Error { Internal(Internal), } -impl From<app::Error> for Error { - fn from(error: app::Error) -> Self { +impl From<app::GetError> for Error { + fn from(error: app::GetError) -> Self { match error { - app::Error::NotFound(_) | app::Error::Deleted(_) => Self::NotFound, - other => Self::Internal(other.into()), + app::GetError::NotFound(_) | app::GetError::Deleted(_) => Self::NotFound, + app::GetError::Failed(_) => Self::Internal(error.into()), } } } diff --git a/src/user/app.rs b/src/user/app.rs index 891e3b9..d993654 100644 --- a/src/user/app.rs +++ b/src/user/app.rs @@ -1,7 +1,15 @@ use sqlx::sqlite::SqlitePool; -use super::create::{self, Create}; -use crate::{clock::DateTime, event::Broadcaster, login::Login, name::Name, password::Password}; +use super::create::Create; +use crate::{ + clock::DateTime, + db, + error::failed::{Failed, ResultExt as _}, + event::Broadcaster, + login::Login, + name::Name, + password::Password, +}; pub struct Users { db: SqlitePool, @@ -18,13 +26,16 @@ impl Users { name: &Name, password: &Password, created_at: &DateTime, - ) -> Result<Login, CreateError> { + ) -> Result<Login, Failed> { let create = Create::begin(name, password, created_at); - let validated = create.validate()?; + let validated = create.validate().fail("Failed to validate new user")?; - let mut tx = self.db.begin().await?; - let stored = validated.store(&mut tx).await?; - tx.commit().await?; + let mut tx = self.db.begin().await.fail(db::failed::BEGIN)?; + let stored = validated + .store(&mut tx) + .await + .fail("Failed to store new user")?; + tx.commit().await.fail(db::failed::COMMIT)?; let login = stored.login().to_owned(); stored.publish(&self.events); @@ -32,22 +43,3 @@ impl Users { Ok(login) } } - -#[derive(Debug, thiserror::Error)] -pub enum CreateError { - #[error("invalid user name: {0}")] - InvalidName(Name), - #[error(transparent)] - PasswordHash(#[from] password_hash::Error), - #[error(transparent)] - Database(#[from] sqlx::Error), -} - -impl From<create::Error> for CreateError { - fn from(error: create::Error) -> Self { - match error { - create::Error::InvalidName(name) => Self::InvalidName(name), - create::Error::PasswordHash(error) => Self::PasswordHash(error), - } - } -} diff --git a/src/user/create.rs b/src/user/create.rs index d6656e5..cb52c5d 100644 --- a/src/user/create.rs +++ b/src/user/create.rs @@ -3,6 +3,7 @@ use sqlx::{Transaction, sqlite::Sqlite}; use super::{History, repo::Provider as _, validate}; use crate::{ clock::DateTime, + error::failed::{Failed, ResultExt as _}, event::{Broadcaster, Event, Sequence, repo::Provider as _}, login::{self, Login, repo::Provider as _}, name::Name, @@ -36,7 +37,7 @@ impl<'a> Create<'a> { return Err(Error::InvalidName(name.clone())); } - let password_hash = password.hash()?; + let password_hash = password.hash().fail("Failed to digest new password")?; Ok(Validated { name, @@ -112,5 +113,5 @@ pub enum Error { #[error("invalid user name: {0}")] InvalidName(Name), #[error(transparent)] - PasswordHash(#[from] password_hash::Error), + Failed(#[from] Failed), } |