From 7ebf8dd84ada4aeffcf1269e26d786d3c17fc99f Mon Sep 17 00:00:00 2001 From: Damien Elmes Date: Sat, 18 Feb 2023 23:42:20 +1000 Subject: [PATCH] Implement HttpError directly so that sources can be extracted properly When disabling the default snafu source handling, .source() doesn't work. --- rslib/src/sync/collection/meta.rs | 8 ++-- rslib/src/sync/error.rs | 52 ++++++++++++--------- rslib/src/sync/http_client/io_monitor.rs | 7 ++- rslib/src/sync/http_client/mod.rs | 6 +-- rslib/src/sync/request/header_and_stream.rs | 35 ++++++-------- 5 files changed, 50 insertions(+), 58 deletions(-) diff --git a/rslib/src/sync/collection/meta.rs b/rslib/src/sync/collection/meta.rs index 8ca35214e..4bfd61c22 100644 --- a/rslib/src/sync/collection/meta.rs +++ b/rslib/src/sync/collection/meta.rs @@ -14,7 +14,6 @@ use crate::sync::collection::normal::SyncActionRequired; use crate::sync::collection::protocol::SyncProtocol; use crate::sync::error::HttpError; use crate::sync::error::HttpResult; -use crate::sync::error::HttpSnafu; use crate::sync::error::OrHttpErr; use crate::sync::http_client::HttpSyncClient; use crate::sync::request::IntoSyncRequest; @@ -139,13 +138,12 @@ impl Collection { pub fn server_meta(req: MetaRequest, col: &mut Collection) -> HttpResult { if !matches!(req.sync_version, SYNC_VERSION_MIN..=SYNC_VERSION_MAX) { - return HttpSnafu { + return Err(HttpError { // old clients expected this code code: StatusCode::NOT_IMPLEMENTED, - context: "unsupported version", + context: "unsupported version".into(), source: None, - } - .fail(); + }); } let mut meta = col.sync_meta().or_internal_err("sync meta")?; if meta.v2_scheduler_or_later && req.sync_version < SYNC_VERSION_09_V2_SCHEDULER { diff --git a/rslib/src/sync/error.rs b/rslib/src/sync/error.rs index 00a02dff9..b26b6c392 100644 --- a/rslib/src/sync/error.rs +++ b/rslib/src/sync/error.rs @@ -1,28 +1,37 @@ // Copyright: Ankitects Pty Ltd and contributors // License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html +use std::error::Error; +use std::fmt::Display; +use std::fmt::Formatter; + use axum::http::StatusCode; use axum::response::IntoResponse; use axum::response::Redirect; use axum::response::Response; -use snafu::OptionExt; -use snafu::Snafu; -pub type HttpResult = std::result::Result; +pub type HttpResult = Result; -#[derive(Debug, Snafu)] -#[snafu(visibility(pub))] +#[derive(Debug)] pub struct HttpError { pub code: StatusCode, pub context: String, - // snafu's automatic error conversion only supports Option if - // the whatever trait is derived, and deriving whatever means we - // can't have extra fields like `code`. Even without Option, the - // error conversion requires us to manually box the error, so we end - // up having to disable the default behaviour and add the error to the - // snafu ourselves - #[snafu(source(false))] - pub source: Option>, + pub source: Option>, +} + +impl Display for HttpError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{} (code={})", self.context, self.code.as_u16()) + } +} + +impl Error for HttpError { + fn source(&self) -> Option<&(dyn Error + 'static)> { + match &self.source { + None => None, + Some(err) => Some(err.as_ref()), + } + } } impl HttpError { @@ -114,7 +123,7 @@ pub trait OrHttpErr { impl OrHttpErr for Result where - E: Into>, + E: Into>, { type Value = T; @@ -123,13 +132,10 @@ where code: StatusCode, context: impl Into, ) -> Result { - self.map_err(|err| { - HttpSnafu { - code, - context: context.into(), - source: err.into(), - } - .build() + self.map_err(|err| HttpError { + code, + context: context.into(), + source: Some(err.into()), }) } } @@ -142,9 +148,9 @@ impl OrHttpErr for Option { code: StatusCode, context: impl Into, ) -> Result { - self.context(HttpSnafu { + self.ok_or_else(|| HttpError { code, - context, + context: context.into(), source: None, }) } diff --git a/rslib/src/sync/http_client/io_monitor.rs b/rslib/src/sync/http_client/io_monitor.rs index 1ffa44d52..f6bc4423d 100644 --- a/rslib/src/sync/http_client/io_monitor.rs +++ b/rslib/src/sync/http_client/io_monitor.rs @@ -27,7 +27,6 @@ use tokio_util::io::StreamReader; use crate::error::Result; use crate::sync::error::HttpError; use crate::sync::error::HttpResult; -use crate::sync::error::HttpSnafu; use crate::sync::error::OrHttpErr; use crate::sync::request::header_and_stream::decode_zstd_body_stream_for_client; use crate::sync::request::header_and_stream::encode_zstd_body_stream; @@ -149,11 +148,11 @@ impl IoMonitor { data = response_body_stream => Ok(data?), // timeout _ = self.timeout(stall_duration) => { - HttpSnafu { + Err(HttpError { code: StatusCode::REQUEST_TIMEOUT, - context: "timeout monitor", + context: "timeout monitor".into(), source: None, - }.fail() + }) } } } diff --git a/rslib/src/sync/http_client/mod.rs b/rslib/src/sync/http_client/mod.rs index 4a3aff2de..6d9eb9e5b 100644 --- a/rslib/src/sync/http_client/mod.rs +++ b/rslib/src/sync/http_client/mod.rs @@ -18,7 +18,6 @@ use crate::sync::collection::progress::FullSyncProgressFn; use crate::sync::collection::protocol::AsSyncEndpoint; use crate::sync::error::HttpError; use crate::sync::error::HttpResult; -use crate::sync::error::HttpSnafu; use crate::sync::http_client::io_monitor::IoMonitor; use crate::sync::login::SyncAuth; use crate::sync::request::header_and_stream::SyncHeader; @@ -113,13 +112,12 @@ impl HttpSyncClient { impl From for HttpError { fn from(err: Error) -> Self { - HttpSnafu { + HttpError { // we should perhaps make this Optional instead code: err.status().unwrap_or(StatusCode::SEE_OTHER), - context: "from reqwest", + context: "from reqwest".into(), source: Some(Box::new(err) as _), } - .build() } } diff --git a/rslib/src/sync/request/header_and_stream.rs b/rslib/src/sync/request/header_and_stream.rs index b9da003a0..a4e0dd81d 100644 --- a/rslib/src/sync/request/header_and_stream.rs +++ b/rslib/src/sync/request/header_and_stream.rs @@ -21,8 +21,8 @@ use serde_derive::Serialize; use tokio::io::AsyncReadExt; use tokio_util::io::ReaderStream; +use crate::sync::error::HttpError; use crate::sync::error::HttpResult; -use crate::sync::error::HttpSnafu; use crate::sync::error::OrHttpErr; use crate::sync::request::SyncRequest; use crate::sync::request::MAXIMUM_SYNC_PAYLOAD_BYTES_UNCOMPRESSED; @@ -81,25 +81,19 @@ where data.map_err(|e| std::io::Error::new(ErrorKind::ConnectionAborted, format!("{e}"))), ); let reader = async_compression::tokio::bufread::ZstdDecoder::new(reader); - ReaderStream::new(reader).map_err(|err| { - HttpSnafu { - code: StatusCode::BAD_REQUEST, - context: "decode zstd body", - source: Some(Box::new(err) as _), - } - .build() + ReaderStream::new(reader).map_err(|err| HttpError { + code: StatusCode::BAD_REQUEST, + context: "decode zstd body".into(), + source: Some(Box::new(err) as _), }) } pub fn encode_zstd_body(data: Vec) -> impl Stream> + Unpin { let enc = async_compression::tokio::bufread::ZstdEncoder::new(Cursor::new(data)); - ReaderStream::new(enc).map_err(|err| { - HttpSnafu { - code: StatusCode::INTERNAL_SERVER_ERROR, - context: "encode zstd body", - source: Some(Box::new(err) as _), - } - .build() + ReaderStream::new(enc).map_err(|err| HttpError { + code: StatusCode::INTERNAL_SERVER_ERROR, + context: "encode zstd body".into(), + source: Some(Box::new(err) as _), }) } @@ -112,13 +106,10 @@ where data.map_err(|e| std::io::Error::new(ErrorKind::ConnectionAborted, format!("{e}"))), ); let reader = async_compression::tokio::bufread::ZstdEncoder::new(reader); - ReaderStream::new(reader).map_err(|err| { - HttpSnafu { - code: StatusCode::BAD_REQUEST, - context: "encode zstd body", - source: Some(Box::new(err) as _), - } - .build() + ReaderStream::new(reader).map_err(|err| HttpError { + code: StatusCode::BAD_REQUEST, + context: "encode zstd body".into(), + source: Some(Box::new(err) as _), }) }