From 83ef4eecc73c5902234fd6fe24ebda87a5e09bdf Mon Sep 17 00:00:00 2001 From: strawberry Date: Sun, 11 Aug 2024 11:08:26 -0400 Subject: [PATCH] reduce unnecessary logging on URL preview and event, use sensible error code for URL previews Signed-off-by: strawberry --- src/api/client/media.rs | 35 +++++++++-------------------------- src/api/client/room.rs | 7 ++----- src/api/client/session.rs | 8 ++++---- 3 files changed, 15 insertions(+), 35 deletions(-) diff --git a/src/api/client/media.rs b/src/api/client/media.rs index bce9b2b5..712c3fe2 100644 --- a/src/api/client/media.rs +++ b/src/api/client/media.rs @@ -5,7 +5,7 @@ use std::time::Duration; use axum::extract::State; use axum_client_ip::InsecureClientIp; use conduit::{ - debug_warn, err, error, + debug_info, debug_warn, err, info, utils::{ self, content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename}, @@ -13,12 +13,8 @@ use conduit::{ }, warn, Err, Error, Result, }; -use ruma::api::client::{ - error::{ErrorKind, RetryAfter}, - media::{ - create_content, get_content, get_content_as_filename, get_content_thumbnail, get_media_config, - get_media_preview, - }, +use ruma::api::client::media::{ + create_content, get_content, get_content_as_filename, get_content_thumbnail, get_media_config, get_media_preview, }; use service::{ media::{FileMeta, MXC_LENGTH}, @@ -70,35 +66,22 @@ pub(crate) async fn get_media_preview_route( let url = &body.url; if !services.media.url_preview_allowed(url) { - return Err!(Request(Forbidden( - warn!(%sender_user, %url, "URL is not allowed to be previewed") - ))); + debug_info!(%sender_user, %url, "URL is not allowed to be previewed"); + return Err!(Request(Forbidden("URL is not allowed to be previewed"))); } match services.media.get_url_preview(url).await { Ok(preview) => { let res = serde_json::value::to_raw_value(&preview).map_err(|e| { - error!(%sender_user, "Failed to convert UrlPreviewData into a serde json value: {e}"); - Error::BadRequest( - ErrorKind::LimitExceeded { - retry_after: Some(RetryAfter::Delay(Duration::from_secs(5))), - }, - "Failed to generate a URL preview, try again later.", - ) + warn!(%sender_user, "Failed to convert UrlPreviewData into a serde json value: {e}"); + err!(Request(Unknown("Failed to generate a URL preview"))) })?; Ok(get_media_preview::v3::Response::from_raw_value(res)) }, Err(e) => { - warn!(%sender_user, "Failed to generate a URL preview: {e}"); - // there doesn't seem to be an agreed-upon error code in the spec. - // the only response codes in the preview_url spec page are 200 and 429. - Err(Error::BadRequest( - ErrorKind::LimitExceeded { - retry_after: Some(RetryAfter::Delay(Duration::from_secs(5))), - }, - "Failed to generate a URL preview, try again later.", - )) + info!(%sender_user, "Failed to generate a URL preview: {e}"); + Err!(Request(Unknown("Failed to generate a URL preview"))) }, } } diff --git a/src/api/client/room.rs b/src/api/client/room.rs index c78ba6ed..e894b878 100644 --- a/src/api/client/room.rs +++ b/src/api/client/room.rs @@ -1,7 +1,7 @@ use std::{cmp::max, collections::BTreeMap}; use axum::extract::State; -use conduit::{debug_info, debug_warn}; +use conduit::{debug_info, debug_warn, err}; use ruma::{ api::client::{ error::ErrorKind, @@ -475,10 +475,7 @@ pub(crate) async fn get_room_event_route( .rooms .timeline .get_pdu(&body.event_id)? - .ok_or_else(|| { - warn!("Event not found, event ID: {:?}", &body.event_id); - Error::BadRequest(ErrorKind::NotFound, "Event not found.") - })?; + .ok_or_else(|| err!(Request(NotFound("Event {} not found.", &body.event_id))))?; if !services .rooms diff --git a/src/api/client/session.rs b/src/api/client/session.rs index 7c2a9718..4702b0ec 100644 --- a/src/api/client/session.rs +++ b/src/api/client/session.rs @@ -34,7 +34,7 @@ struct Claims { /// /// Get the supported login types of this server. One of these should be used as /// the `type` field when logging in. -#[tracing::instrument(skip_all, fields(%client), name = "register")] +#[tracing::instrument(skip_all, fields(%client), name = "login")] pub(crate) async fn get_login_types_route( InsecureClientIp(client): InsecureClientIp, _body: Ruma, ) -> Result { @@ -58,7 +58,7 @@ pub(crate) async fn get_login_types_route( /// Note: You can use [`GET /// /_matrix/client/r0/login`](fn.get_supported_versions_route.html) to see /// supported login types. -#[tracing::instrument(skip_all, fields(%client), name = "register")] +#[tracing::instrument(skip_all, fields(%client), name = "login")] pub(crate) async fn login_route( State(services): State, InsecureClientIp(client): InsecureClientIp, body: Ruma, ) -> Result { @@ -221,7 +221,7 @@ pub(crate) async fn login_route( /// last seen ts) /// - Forgets to-device events /// - Triggers device list updates -#[tracing::instrument(skip_all, fields(%client), name = "register")] +#[tracing::instrument(skip_all, fields(%client), name = "logout")] pub(crate) async fn logout_route( State(services): State, InsecureClientIp(client): InsecureClientIp, body: Ruma, ) -> Result { @@ -249,7 +249,7 @@ pub(crate) async fn logout_route( /// Note: This is equivalent to calling [`GET /// /_matrix/client/r0/logout`](fn.logout_route.html) from each device of this /// user. -#[tracing::instrument(skip_all, fields(%client), name = "register")] +#[tracing::instrument(skip_all, fields(%client), name = "logout")] pub(crate) async fn logout_all_route( State(services): State, InsecureClientIp(client): InsecureClientIp, body: Ruma,