From 547bc2171cb38595d43525cac339a17fd3a05d20 Mon Sep 17 00:00:00 2001 From: strawberry Date: Fri, 26 Jul 2024 19:13:18 -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 | 29 ++++++++--------------------- src/api/client/room.rs | 7 ++----- src/api/client/session.rs | 9 ++++----- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/api/client/media.rs b/src/api/client/media.rs index f0afa290..1ac32014 100644 --- a/src/api/client/media.rs +++ b/src/api/client/media.rs @@ -5,19 +5,19 @@ use std::{io::Cursor, time::Duration}; use axum::extract::State; use axum_client_ip::InsecureClientIp; use conduit::{ - debug, debug_warn, error, + debug, debug_info, debug_warn, err, info, utils::{ self, content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename}, math::ruma_from_usize, }, - warn, Error, Result, + warn, Err, Error, Result, }; use image::io::Reader as ImgReader; use ipaddress::IPAddress; use reqwest::Url; use ruma::api::client::{ - error::{ErrorKind, RetryAfter}, + error::ErrorKind, media::{ create_content, get_content, get_content_as_filename, get_content_thumbnail, get_media_config, get_media_preview, @@ -77,35 +77,22 @@ pub(crate) async fn get_media_preview_route( let url = &body.url; if !url_preview_allowed(&services, url) { - warn!(%sender_user, "URL is not allowed to be previewed: {url}"); + debug_info!(%sender_user, "URL is not allowed to be previewed: {url}"); return Err(Error::BadRequest(ErrorKind::forbidden(), "URL is not allowed to be previewed")); } match get_url_preview(&services, 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..6bb6a0f9 100644 --- a/src/api/client/session.rs +++ b/src/api/client/session.rs @@ -34,9 +34,8 @@ 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")] pub(crate) async fn get_login_types_route( - InsecureClientIp(client): InsecureClientIp, _body: Ruma, + _body: Ruma, ) -> Result { Ok(get_login_types::v3::Response::new(vec![ get_login_types::v3::LoginType::Password(PasswordLoginType::default()), @@ -58,7 +57,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 +220,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 +248,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,