diff --git a/crates/handlers/src/compat/login.rs b/crates/handlers/src/compat/login.rs index 9c9312855..d13b50d3c 100644 --- a/crates/handlers/src/compat/login.rs +++ b/crates/handlers/src/compat/login.rs @@ -10,7 +10,7 @@ use axum::{Json, extract::State, response::IntoResponse}; use axum_extra::typed_header::TypedHeader; use chrono::Duration; use hyper::StatusCode; -use mas_axum_utils::sentry::SentryEventID; +use mas_axum_utils::record_error; use mas_data_model::{ CompatSession, CompatSsoLoginState, Device, SiteConfig, TokenType, User, UserAgent, }; @@ -210,7 +210,8 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { - let event_id = sentry::capture_error(&self); + let sentry_event_id = + record_error!(self, Self::Internal(_) | Self::ProvisionDeviceFailed(_)); LOGIN_COUNTER.add(1, &[KeyValue::new(RESULT, "error")]); let response = match self { Self::Internal(_) | Self::ProvisionDeviceFailed(_) => MatrixError { @@ -257,11 +258,11 @@ impl IntoResponse for RouteError { }, }; - (SentryEventID::from(event_id), response).into_response() + (sentry_event_id, response).into_response() } } -#[tracing::instrument(name = "handlers.compat.login.post", skip_all, err)] +#[tracing::instrument(name = "handlers.compat.login.post", skip_all)] pub(crate) async fn post( mut rng: BoxRng, clock: BoxClock, diff --git a/crates/handlers/src/compat/login_sso_complete.rs b/crates/handlers/src/compat/login_sso_complete.rs index 8da507d70..1a5d1504c 100644 --- a/crates/handlers/src/compat/login_sso_complete.rs +++ b/crates/handlers/src/compat/login_sso_complete.rs @@ -48,7 +48,6 @@ pub struct Params { name = "handlers.compat.login_sso_complete.get", fields(compat_sso_login.id = %id), skip_all, - err, )] pub async fn get( PreferredLanguage(locale): PreferredLanguage, @@ -121,7 +120,6 @@ pub async fn get( name = "handlers.compat.login_sso_complete.post", fields(compat_sso_login.id = %id), skip_all, - err, )] pub async fn post( mut rng: BoxRng, diff --git a/crates/handlers/src/compat/login_sso_redirect.rs b/crates/handlers/src/compat/login_sso_redirect.rs index 36b065b39..583f24e9b 100644 --- a/crates/handlers/src/compat/login_sso_redirect.rs +++ b/crates/handlers/src/compat/login_sso_redirect.rs @@ -9,7 +9,7 @@ use axum::{ response::IntoResponse, }; use hyper::StatusCode; -use mas_axum_utils::sentry::SentryEventID; +use mas_axum_utils::record_error; use mas_router::{CompatLoginSsoAction, CompatLoginSsoComplete, UrlBuilder}; use mas_storage::{BoxClock, BoxRepository, BoxRng, compat::CompatSsoLoginRepository}; use rand::distributions::{Alphanumeric, DistString}; @@ -43,17 +43,16 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { - let event_id = sentry::capture_error(&self); - ( - StatusCode::INTERNAL_SERVER_ERROR, - SentryEventID::from(event_id), - format!("{self}"), - ) - .into_response() + let sentry_event_id = record_error!(self, Self::Internal(_)); + let status_code = match &self { + Self::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR, + Self::MissingRedirectUrl | Self::InvalidRedirectUrl => StatusCode::BAD_REQUEST, + }; + (status_code, sentry_event_id, format!("{self}")).into_response() } } -#[tracing::instrument(name = "handlers.compat.login_sso_redirect.get", skip_all, err)] +#[tracing::instrument(name = "handlers.compat.login_sso_redirect.get", skip_all)] pub async fn get( mut rng: BoxRng, clock: BoxClock, diff --git a/crates/handlers/src/compat/logout.rs b/crates/handlers/src/compat/logout.rs index 557dbaef9..7b2ea7d52 100644 --- a/crates/handlers/src/compat/logout.rs +++ b/crates/handlers/src/compat/logout.rs @@ -10,7 +10,7 @@ use axum::{Json, response::IntoResponse}; use axum_extra::typed_header::TypedHeader; use headers::{Authorization, authorization::Bearer}; use hyper::StatusCode; -use mas_axum_utils::sentry::SentryEventID; +use mas_axum_utils::record_error; use mas_data_model::TokenType; use mas_storage::{ BoxClock, BoxRepository, BoxRng, Clock, RepositoryAccess, @@ -51,7 +51,7 @@ impl_from_error_for_route!(mas_storage::RepositoryError); impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { - let event_id = sentry::capture_error(&self); + let sentry_event_id = record_error!(self, Self::Internal(_)); LOGOUT_COUNTER.add(1, &[KeyValue::new(RESULT, "error")]); let response = match self { Self::Internal(_) => MatrixError { @@ -71,11 +71,11 @@ impl IntoResponse for RouteError { }, }; - (SentryEventID::from(event_id), response).into_response() + (sentry_event_id, response).into_response() } } -#[tracing::instrument(name = "handlers.compat.logout.post", skip_all, err)] +#[tracing::instrument(name = "handlers.compat.logout.post", skip_all)] pub(crate) async fn post( clock: BoxClock, mut rng: BoxRng, diff --git a/crates/handlers/src/compat/mod.rs b/crates/handlers/src/compat/mod.rs index fcb45d68c..304499471 100644 --- a/crates/handlers/src/compat/mod.rs +++ b/crates/handlers/src/compat/mod.rs @@ -14,7 +14,7 @@ use axum::{ response::IntoResponse, }; use hyper::{StatusCode, header}; -use mas_axum_utils::sentry::SentryEventID; +use mas_axum_utils::record_error; use serde::{Serialize, de::DeserializeOwned}; use thiserror::Error; @@ -59,7 +59,7 @@ pub enum MatrixJsonBodyRejection { impl IntoResponse for MatrixJsonBodyRejection { fn into_response(self) -> axum::response::Response { - let event_id = sentry::capture_error(&self); + let sentry_event_id = record_error!(self, !); let response = match self { Self::InvalidContentType | Self::ContentTypeNotJson(_) => MatrixError { errcode: "M_NOT_JSON", @@ -102,7 +102,7 @@ impl IntoResponse for MatrixJsonBodyRejection { }, }; - (SentryEventID::from(event_id), response).into_response() + (sentry_event_id, response).into_response() } } diff --git a/crates/handlers/src/compat/refresh.rs b/crates/handlers/src/compat/refresh.rs index 9e3675910..7511e0e99 100644 --- a/crates/handlers/src/compat/refresh.rs +++ b/crates/handlers/src/compat/refresh.rs @@ -7,7 +7,7 @@ use axum::{Json, extract::State, response::IntoResponse}; use chrono::Duration; use hyper::StatusCode; -use mas_axum_utils::sentry::SentryEventID; +use mas_axum_utils::record_error; use mas_data_model::{SiteConfig, TokenFormatError, TokenType}; use mas_storage::{ BoxClock, BoxRepository, BoxRng, Clock, @@ -16,6 +16,7 @@ use mas_storage::{ use serde::{Deserialize, Serialize}; use serde_with::{DurationMilliSeconds, serde_as}; use thiserror::Error; +use ulid::Ulid; use super::MatrixError; use crate::{BoundActivityTracker, impl_from_error_for_route}; @@ -31,46 +32,50 @@ pub enum RouteError { Internal(Box), #[error("invalid token")] - InvalidToken, + InvalidToken(#[from] TokenFormatError), - #[error("refresh token already consumed")] - RefreshTokenConsumed, + #[error("unknown token")] + UnknownToken, - #[error("invalid session")] - InvalidSession, + #[error("invalid token type {0}, expected a compat refresh token")] + InvalidTokenType(TokenType), - #[error("unknown session")] - UnknownSession, + #[error("refresh token already consumed {0}")] + RefreshTokenConsumed(Ulid), + + #[error("invalid compat session {0}")] + InvalidSession(Ulid), + + #[error("unknown comapt session {0}")] + UnknownSession(Ulid), } impl IntoResponse for RouteError { fn into_response(self) -> axum::response::Response { - let event_id = sentry::capture_error(&self); + let sentry_event_id = record_error!(self, Self::Internal(_) | Self::UnknownSession(_)); let response = match self { - Self::Internal(_) | Self::UnknownSession => MatrixError { + Self::Internal(_) | Self::UnknownSession(_) => MatrixError { errcode: "M_UNKNOWN", error: "Internal error", status: StatusCode::INTERNAL_SERVER_ERROR, }, - Self::InvalidToken | Self::InvalidSession | Self::RefreshTokenConsumed => MatrixError { + Self::InvalidToken(_) + | Self::UnknownToken + | Self::InvalidTokenType(_) + | Self::InvalidSession(_) + | Self::RefreshTokenConsumed(_) => MatrixError { errcode: "M_UNKNOWN_TOKEN", error: "Invalid refresh token", status: StatusCode::UNAUTHORIZED, }, }; - (SentryEventID::from(event_id), response).into_response() + (sentry_event_id, response).into_response() } } impl_from_error_for_route!(mas_storage::RepositoryError); -impl From for RouteError { - fn from(_e: TokenFormatError) -> Self { - Self::InvalidToken - } -} - #[serde_as] #[derive(Debug, Serialize)] pub struct ResponseBody { @@ -80,7 +85,7 @@ pub struct ResponseBody { expires_in_ms: Duration, } -#[tracing::instrument(name = "handlers.compat.refresh.post", skip_all, err)] +#[tracing::instrument(name = "handlers.compat.refresh.post", skip_all)] pub(crate) async fn post( mut rng: BoxRng, clock: BoxClock, @@ -92,27 +97,27 @@ pub(crate) async fn post( let token_type = TokenType::check(&input.refresh_token)?; if token_type != TokenType::CompatRefreshToken { - return Err(RouteError::InvalidToken); + return Err(RouteError::InvalidTokenType(token_type)); } let refresh_token = repo .compat_refresh_token() .find_by_token(&input.refresh_token) .await? - .ok_or(RouteError::InvalidToken)?; + .ok_or(RouteError::UnknownToken)?; if !refresh_token.is_valid() { - return Err(RouteError::RefreshTokenConsumed); + return Err(RouteError::RefreshTokenConsumed(refresh_token.id)); } let session = repo .compat_session() .lookup(refresh_token.session_id) .await? - .ok_or(RouteError::UnknownSession)?; + .ok_or(RouteError::UnknownSession(refresh_token.session_id))?; if !session.is_valid() { - return Err(RouteError::InvalidSession); + return Err(RouteError::InvalidSession(refresh_token.session_id)); } activity_tracker