feat: wire MysqlError's ReportableError impl into TokenserverError (#1611)

* feat: wire MysqlError's ReportableError impl into TokenserverError

and quiet Pool errors

Closes SYNC-4424

* kluge: allow(clippy::result_large_err) for TokenserverError

(for now) we can get rid of this in a later refactoring
This commit is contained in:
Philip Jenvey 2024-10-17 14:23:45 -07:00 committed by GitHub
parent d23133101f
commit c535e5ae52
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 51 additions and 4 deletions

View File

@ -41,7 +41,11 @@ impl From<MysqlErrorKind> 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<String> {

View File

@ -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()
})
})

View File

@ -1,4 +1,6 @@
#[allow(clippy::result_large_err)]
pub mod extractors;
#[allow(clippy::result_large_err)]
pub mod handlers;
pub mod logging;

View File

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

View File

@ -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<Backtrace>,
pub token_type: TokenType,
pub tags: Option<Box<Vec<(&'static str, String)>>>,
/// 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<Box<dyn ReportableError + Send>>,
}
#[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<TokenserverError> 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<String> {
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()),

View File

@ -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<String> {
match &self.kind {
DbErrorKind::Mysql(e) => e.metric_label(),
_ => None,
}
}
}
#[derive(Debug, Error)]
enum DbErrorKind {
#[error("{}", _0)]
@ -56,7 +79,7 @@ impl From<DbError> 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<DbError> 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()
}