diff --git a/syncserver-db-common/src/error.rs b/syncserver-db-common/src/error.rs index 46f52f282d..b3fd78e4fc 100644 --- a/syncserver-db-common/src/error.rs +++ b/syncserver-db-common/src/error.rs @@ -41,7 +41,11 @@ impl From for MysqlError { impl ReportableError for MysqlError { fn is_sentry_event(&self) -> bool { - true + #[allow(clippy::match_like_matches_macro)] + match &self.kind { + MysqlErrorKind::Pool(_) => false, + _ => true, + } } fn metric_label(&self) -> Option { diff --git a/syncserver/src/tokenserver/extractors.rs b/syncserver/src/tokenserver/extractors.rs index 12bd024b10..15bcfd6075 100644 --- a/syncserver/src/tokenserver/extractors.rs +++ b/syncserver/src/tokenserver/extractors.rs @@ -320,6 +320,7 @@ impl FromRequest for DbWrapper { .map(Self) .map_err(|e| TokenserverError { context: format!("Couldn't acquire a database connection: {}", e), + source: Some(Box::new(e)), ..TokenserverError::internal_error() }) }) diff --git a/syncserver/src/tokenserver/mod.rs b/syncserver/src/tokenserver/mod.rs index 50d5a1772f..9de40b0347 100644 --- a/syncserver/src/tokenserver/mod.rs +++ b/syncserver/src/tokenserver/mod.rs @@ -1,4 +1,6 @@ +#[allow(clippy::result_large_err)] pub mod extractors; +#[allow(clippy::result_large_err)] pub mod handlers; pub mod logging; diff --git a/tokenserver-auth/src/lib.rs b/tokenserver-auth/src/lib.rs index 636eb7512e..6291571f32 100644 --- a/tokenserver-auth/src/lib.rs +++ b/tokenserver-auth/src/lib.rs @@ -3,7 +3,9 @@ mod crypto; #[cfg(not(feature = "py"))] pub use crypto::{JWTVerifier, JWTVerifierImpl}; +#[allow(clippy::result_large_err)] pub mod oauth; +#[allow(clippy::result_large_err)] mod token; use syncserver_common::Metrics; pub use token::Tokenlib; diff --git a/tokenserver-common/src/error.rs b/tokenserver-common/src/error.rs index b2e933cb50..c321bd2bc8 100644 --- a/tokenserver-common/src/error.rs +++ b/tokenserver-common/src/error.rs @@ -11,7 +11,7 @@ use syncserver_common::{InternalError, ReportableError}; /// An error type that represents application-specific errors to Tokenserver. This error is not /// used to represent database-related errors; database-related errors have their own type. -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct TokenserverError { pub status: &'static str, pub location: ErrorLocation, @@ -24,6 +24,10 @@ pub struct TokenserverError { pub backtrace: Box, pub token_type: TokenType, pub tags: Option>>, + /// TODO: refactor TokenserverError to include a TokenserverErrorKind, w/ + /// variants for sources (currently just DbError). May require moving + /// TokenserverError out of common (into syncserver) + pub source: Option>, } #[derive(Clone, Debug)] @@ -64,6 +68,7 @@ impl Default for TokenserverError { backtrace: Box::new(Backtrace::new()), token_type: TokenType::Oauth, tags: None, + source: None, } } } @@ -275,14 +280,23 @@ impl From for HttpResponse { impl ReportableError for TokenserverError { fn backtrace(&self) -> Option<&Backtrace> { + if let Some(source) = &self.source { + return source.backtrace(); + } Some(&self.backtrace) } fn is_sentry_event(&self) -> bool { + if let Some(source) = &self.source { + return source.is_sentry_event(); + } self.http_status.is_server_error() && self.metric_label().is_none() } fn metric_label(&self) -> Option { + if let Some(source) = &self.source { + return source.metric_label(); + } if self.http_status.is_client_error() { match self.token_type { TokenType::Oauth => Some("request.error.oauth".to_owned()), diff --git a/tokenserver-db/src/error.rs b/tokenserver-db/src/error.rs index a7e30a12d2..b9efcbd50b 100644 --- a/tokenserver-db/src/error.rs +++ b/tokenserver-db/src/error.rs @@ -2,7 +2,7 @@ use std::fmt; use backtrace::Backtrace; use http::StatusCode; -use syncserver_common::{from_error, impl_fmt_display, InternalError}; +use syncserver_common::{from_error, impl_fmt_display, InternalError, ReportableError}; use syncserver_db_common::error::MysqlError; use thiserror::Error; use tokenserver_common::TokenserverError; @@ -25,6 +25,29 @@ impl DbError { } } +impl ReportableError for DbError { + fn backtrace(&self) -> Option<&Backtrace> { + match &self.kind { + DbErrorKind::Mysql(e) => e.backtrace(), + _ => Some(&self.backtrace), + } + } + + fn is_sentry_event(&self) -> bool { + match &self.kind { + DbErrorKind::Mysql(e) => e.is_sentry_event(), + _ => true, + } + } + + fn metric_label(&self) -> Option { + match &self.kind { + DbErrorKind::Mysql(e) => e.metric_label(), + _ => None, + } + } +} + #[derive(Debug, Error)] enum DbErrorKind { #[error("{}", _0)] @@ -56,7 +79,7 @@ impl From for TokenserverError { TokenserverError { description: db_error.to_string(), context: db_error.to_string(), - backtrace: db_error.backtrace, + backtrace: db_error.backtrace.clone(), http_status: if db_error.status.is_server_error() { // Use the status code from the DbError if it already suggests an internal error; // it might be more specific than `StatusCode::SERVICE_UNAVAILABLE` @@ -64,6 +87,7 @@ impl From for TokenserverError { } else { StatusCode::SERVICE_UNAVAILABLE }, + source: Some(Box::new(db_error)), // An unhandled DbError in the Tokenserver code is an internal error ..TokenserverError::internal_error() }