From 87914cbcb3dd6e501b0f9770559cc854c7286744 Mon Sep 17 00:00:00 2001 From: Quentin Gliech Date: Tue, 31 Jan 2023 15:42:54 +0100 Subject: [PATCH] Capture better errors in Sentry --- Cargo.lock | 2 ++ crates/axum-utils/Cargo.toml | 1 + crates/axum-utils/src/fancy_error.rs | 1 + crates/cli/src/main.rs | 25 ++++++++++++++++--- crates/handlers/Cargo.toml | 1 + crates/handlers/src/compat/login.rs | 1 + .../handlers/src/compat/login_sso_redirect.rs | 1 + crates/handlers/src/compat/logout.rs | 1 + crates/handlers/src/compat/refresh.rs | 1 + .../src/oauth2/authorization/complete.rs | 1 + .../handlers/src/oauth2/authorization/mod.rs | 1 + crates/handlers/src/oauth2/consent.rs | 1 + crates/handlers/src/oauth2/introspection.rs | 1 + crates/handlers/src/oauth2/registration.rs | 1 + crates/handlers/src/oauth2/token.rs | 1 + crates/handlers/src/oauth2/userinfo.rs | 1 + .../handlers/src/upstream_oauth2/authorize.rs | 1 + .../handlers/src/upstream_oauth2/callback.rs | 1 + crates/handlers/src/upstream_oauth2/link.rs | 1 + 19 files changed, 41 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1bfdcea7..1e74b956 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2734,6 +2734,7 @@ dependencies = [ "mas-templates", "mime", "rand 0.8.5", + "sentry", "serde", "serde_json", "serde_urlencoded", @@ -2919,6 +2920,7 @@ dependencies = [ "pbkdf2", "rand 0.8.5", "rand_chacha 0.3.1", + "sentry", "serde", "serde_json", "serde_urlencoded", diff --git a/crates/axum-utils/Cargo.toml b/crates/axum-utils/Cargo.toml index 7a6296be..a36b967f 100644 --- a/crates/axum-utils/Cargo.toml +++ b/crates/axum-utils/Cargo.toml @@ -17,6 +17,7 @@ http = "0.2.8" http-body = "0.4.5" mime = "0.3.16" rand = "0.8.5" +sentry = { version = "0.29.2", default-features = false } serde = "1.0.152" serde_with = "2.2.0" serde_urlencoded = "0.7.1" diff --git a/crates/axum-utils/src/fancy_error.rs b/crates/axum-utils/src/fancy_error.rs index ed9cbba4..363f423e 100644 --- a/crates/axum-utils/src/fancy_error.rs +++ b/crates/axum-utils/src/fancy_error.rs @@ -52,6 +52,7 @@ impl From for FancyError { impl IntoResponse for FancyError { fn into_response(self) -> Response { let error = format!("{:?}", self.context); + sentry::capture_message(&error, sentry::Level::Error); ( StatusCode::INTERNAL_SERVER_ERROR, Extension(self.context), diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 7252a78f..93a96bb6 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -20,6 +20,7 @@ use anyhow::Context; use clap::Parser; use mas_config::TelemetryConfig; +use sentry_tracing::EventFilter; use tracing_subscriber::{ filter::LevelFilter, layer::SubscriberExt, reload, util::SubscriberInitExt, EnvFilter, Layer, Registry, @@ -69,8 +70,8 @@ async fn try_main() -> anyhow::Result<()> { let (sentry_layer, sentry_handle) = reload::Layer::new(None); let subscriber = Registry::default() - .with(telemetry_layer) .with(sentry_layer) + .with(telemetry_layer) .with(filter_layer) .with(fmt_layer); subscriber @@ -94,9 +95,27 @@ async fn try_main() -> anyhow::Result<()> { let telemetry_config: TelemetryConfig = opts.load_config().unwrap_or_default(); // Setup Sentry - let sentry = sentry::init(telemetry_config.sentry.dsn.as_deref()); + let sentry = sentry::init(( + telemetry_config.sentry.dsn.as_deref(), + sentry::ClientOptions { + traces_sample_rate: 1.0, + auto_session_tracking: true, + session_mode: sentry::SessionMode::Request, + ..Default::default() + }, + )); if sentry.is_enabled() { - sentry_handle.reload(sentry_tracing::layer())?; + let layer = sentry_tracing::layer().event_filter(|md| { + // All the spans in the handlers module send their data to Sentry themselves, so + // we only create breadcrumbs for them, instead of full events + if md.target().starts_with("mas_handlers::") { + EventFilter::Breadcrumb + } else { + sentry_tracing::default_event_filter(md) + } + }); + + sentry_handle.reload(layer)?; } // Setup OpenTelemtry tracing and metrics diff --git a/crates/handlers/Cargo.toml b/crates/handlers/Cargo.toml index 230d82fc..b0b232c0 100644 --- a/crates/handlers/Cargo.toml +++ b/crates/handlers/Cargo.toml @@ -16,6 +16,7 @@ tracing = "0.1.37" # Error management thiserror = "1.0.38" anyhow = "1.0.68" +sentry = { version = "0.29.2", default-features = false } # Web server hyper = { version = "0.14.23", features = ["full"] } diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 62dc5a43..1cc18c8c 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -158,6 +158,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) | Self::SessionNotFound => MatrixError { errcode: "M_UNKNOWN", diff --git a/crates/handlers/src/compat/login_sso_redirect.rs b/crates/handlers/src/compat/login_sso_redirect.rs index 48d0d561..ba0072e4 100644 --- a/crates/handlers/src/compat/login_sso_redirect.rs +++ b/crates/handlers/src/compat/login_sso_redirect.rs @@ -51,6 +51,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); (StatusCode::INTERNAL_SERVER_ERROR, format!("{self}")).into_response() } } diff --git a/crates/handlers/src/compat/logout.rs b/crates/handlers/src/compat/logout.rs index 0a52eddc..7875e48b 100644 --- a/crates/handlers/src/compat/logout.rs +++ b/crates/handlers/src/compat/logout.rs @@ -44,6 +44,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) => MatrixError { errcode: "M_UNKNOWN", diff --git a/crates/handlers/src/compat/refresh.rs b/crates/handlers/src/compat/refresh.rs index f18a6ecb..a0ac4f63 100644 --- a/crates/handlers/src/compat/refresh.rs +++ b/crates/handlers/src/compat/refresh.rs @@ -52,6 +52,7 @@ pub enum RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) | Self::UnknownSession => MatrixError { errcode: "M_UNKNOWN", diff --git a/crates/handlers/src/oauth2/authorization/complete.rs b/crates/handlers/src/oauth2/authorization/complete.rs index 100bb32c..b51b2d5f 100644 --- a/crates/handlers/src/oauth2/authorization/complete.rs +++ b/crates/handlers/src/oauth2/authorization/complete.rs @@ -51,6 +51,7 @@ pub enum RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); // TODO: better error pages match self { RouteError::NotFound => { diff --git a/crates/handlers/src/oauth2/authorization/mod.rs b/crates/handlers/src/oauth2/authorization/mod.rs index f1bbfd06..2e56f927 100644 --- a/crates/handlers/src/oauth2/authorization/mod.rs +++ b/crates/handlers/src/oauth2/authorization/mod.rs @@ -66,6 +66,7 @@ pub enum RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); // TODO: better error pages match self { RouteError::Internal(e) => { diff --git a/crates/handlers/src/oauth2/consent.rs b/crates/handlers/src/oauth2/consent.rs index 8fd9bf7b..bbf98cf7 100644 --- a/crates/handlers/src/oauth2/consent.rs +++ b/crates/handlers/src/oauth2/consent.rs @@ -67,6 +67,7 @@ impl_from_error_for_route!(mas_policy::EvaluationError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); StatusCode::INTERNAL_SERVER_ERROR.into_response() } } diff --git a/crates/handlers/src/oauth2/introspection.rs b/crates/handlers/src/oauth2/introspection.rs index 44a0ad05..24b611d2 100644 --- a/crates/handlers/src/oauth2/introspection.rs +++ b/crates/handlers/src/oauth2/introspection.rs @@ -59,6 +59,7 @@ pub enum RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(e) => ( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/crates/handlers/src/oauth2/registration.rs b/crates/handlers/src/oauth2/registration.rs index 35fdf3e1..b8505b2e 100644 --- a/crates/handlers/src/oauth2/registration.rs +++ b/crates/handlers/src/oauth2/registration.rs @@ -67,6 +67,7 @@ impl From for RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) => ( StatusCode::INTERNAL_SERVER_ERROR, diff --git a/crates/handlers/src/oauth2/token.rs b/crates/handlers/src/oauth2/token.rs index 5fddcea9..29ebd1ac 100644 --- a/crates/handlers/src/oauth2/token.rs +++ b/crates/handlers/src/oauth2/token.rs @@ -113,6 +113,7 @@ pub(crate) enum RouteError { impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) | Self::InvalidSigningKey diff --git a/crates/handlers/src/oauth2/userinfo.rs b/crates/handlers/src/oauth2/userinfo.rs index 8394d16c..bb14cedb 100644 --- a/crates/handlers/src/oauth2/userinfo.rs +++ b/crates/handlers/src/oauth2/userinfo.rs @@ -83,6 +83,7 @@ impl_from_error_for_route!(mas_jose::jwt::JwtSignatureError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::Internal(_) | Self::InvalidSigningKey diff --git a/crates/handlers/src/upstream_oauth2/authorize.rs b/crates/handlers/src/upstream_oauth2/authorize.rs index b56bae88..b80bd338 100644 --- a/crates/handlers/src/upstream_oauth2/authorize.rs +++ b/crates/handlers/src/upstream_oauth2/authorize.rs @@ -48,6 +48,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::ProviderNotFound => (StatusCode::NOT_FOUND, "Provider not found").into_response(), Self::Internal(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(), diff --git a/crates/handlers/src/upstream_oauth2/callback.rs b/crates/handlers/src/upstream_oauth2/callback.rs index 03846dfb..601f2bfa 100644 --- a/crates/handlers/src/upstream_oauth2/callback.rs +++ b/crates/handlers/src/upstream_oauth2/callback.rs @@ -108,6 +108,7 @@ impl_from_error_for_route!(super::cookie::UpstreamSessionNotFound); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::ProviderNotFound => (StatusCode::NOT_FOUND, "Provider not found").into_response(), Self::SessionNotFound => (StatusCode::NOT_FOUND, "Session not found").into_response(), diff --git a/crates/handlers/src/upstream_oauth2/link.rs b/crates/handlers/src/upstream_oauth2/link.rs index 2959517d..567b1746 100644 --- a/crates/handlers/src/upstream_oauth2/link.rs +++ b/crates/handlers/src/upstream_oauth2/link.rs @@ -75,6 +75,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { + sentry::capture_error(&self); match self { Self::LinkNotFound => (StatusCode::NOT_FOUND, "Link not found").into_response(), Self::Internal(e) => (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()).into_response(),