reduce unnecessary logging on URL preview and event, use sensible error code for URL previews

Signed-off-by: strawberry <strawberry@puppygock.gay>
This commit is contained in:
strawberry 2024-08-11 11:08:26 -04:00 committed by Jason Volk
parent 52ccad04a6
commit 83ef4eecc7
3 changed files with 15 additions and 35 deletions

View file

@ -5,7 +5,7 @@ use std::time::Duration;
use axum::extract::State; use axum::extract::State;
use axum_client_ip::InsecureClientIp; use axum_client_ip::InsecureClientIp;
use conduit::{ use conduit::{
debug_warn, err, error, debug_info, debug_warn, err, info,
utils::{ utils::{
self, self,
content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename}, content_disposition::{content_disposition_type, make_content_disposition, sanitise_filename},
@ -13,12 +13,8 @@ use conduit::{
}, },
warn, Err, Error, Result, warn, Err, Error, Result,
}; };
use ruma::api::client::{ use ruma::api::client::media::{
error::{ErrorKind, RetryAfter}, create_content, get_content, get_content_as_filename, get_content_thumbnail, get_media_config, get_media_preview,
media::{
create_content, get_content, get_content_as_filename, get_content_thumbnail, get_media_config,
get_media_preview,
},
}; };
use service::{ use service::{
media::{FileMeta, MXC_LENGTH}, media::{FileMeta, MXC_LENGTH},
@ -70,35 +66,22 @@ pub(crate) async fn get_media_preview_route(
let url = &body.url; let url = &body.url;
if !services.media.url_preview_allowed(url) { if !services.media.url_preview_allowed(url) {
return Err!(Request(Forbidden( debug_info!(%sender_user, %url, "URL is not allowed to be previewed");
warn!(%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 { match services.media.get_url_preview(url).await {
Ok(preview) => { Ok(preview) => {
let res = serde_json::value::to_raw_value(&preview).map_err(|e| { 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}"); warn!(%sender_user, "Failed to convert UrlPreviewData into a serde json value: {e}");
Error::BadRequest( err!(Request(Unknown("Failed to generate a URL preview")))
ErrorKind::LimitExceeded {
retry_after: Some(RetryAfter::Delay(Duration::from_secs(5))),
},
"Failed to generate a URL preview, try again later.",
)
})?; })?;
Ok(get_media_preview::v3::Response::from_raw_value(res)) Ok(get_media_preview::v3::Response::from_raw_value(res))
}, },
Err(e) => { Err(e) => {
warn!(%sender_user, "Failed to generate a URL preview: {e}"); info!(%sender_user, "Failed to generate a URL preview: {e}");
// there doesn't seem to be an agreed-upon error code in the spec. Err!(Request(Unknown("Failed to generate a URL preview")))
// 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.",
))
}, },
} }
} }

View file

@ -1,7 +1,7 @@
use std::{cmp::max, collections::BTreeMap}; use std::{cmp::max, collections::BTreeMap};
use axum::extract::State; use axum::extract::State;
use conduit::{debug_info, debug_warn}; use conduit::{debug_info, debug_warn, err};
use ruma::{ use ruma::{
api::client::{ api::client::{
error::ErrorKind, error::ErrorKind,
@ -475,10 +475,7 @@ pub(crate) async fn get_room_event_route(
.rooms .rooms
.timeline .timeline
.get_pdu(&body.event_id)? .get_pdu(&body.event_id)?
.ok_or_else(|| { .ok_or_else(|| err!(Request(NotFound("Event {} not found.", &body.event_id))))?;
warn!("Event not found, event ID: {:?}", &body.event_id);
Error::BadRequest(ErrorKind::NotFound, "Event not found.")
})?;
if !services if !services
.rooms .rooms

View file

@ -34,7 +34,7 @@ struct Claims {
/// ///
/// Get the supported login types of this server. One of these should be used as /// Get the supported login types of this server. One of these should be used as
/// the `type` field when logging in. /// 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( pub(crate) async fn get_login_types_route(
InsecureClientIp(client): InsecureClientIp, _body: Ruma<get_login_types::v3::Request>, InsecureClientIp(client): InsecureClientIp, _body: Ruma<get_login_types::v3::Request>,
) -> Result<get_login_types::v3::Response> { ) -> Result<get_login_types::v3::Response> {
@ -58,7 +58,7 @@ pub(crate) async fn get_login_types_route(
/// Note: You can use [`GET /// Note: You can use [`GET
/// /_matrix/client/r0/login`](fn.get_supported_versions_route.html) to see /// /_matrix/client/r0/login`](fn.get_supported_versions_route.html) to see
/// supported login types. /// 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( pub(crate) async fn login_route(
State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp, body: Ruma<login::v3::Request>, State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp, body: Ruma<login::v3::Request>,
) -> Result<login::v3::Response> { ) -> Result<login::v3::Response> {
@ -221,7 +221,7 @@ pub(crate) async fn login_route(
/// last seen ts) /// last seen ts)
/// - Forgets to-device events /// - Forgets to-device events
/// - Triggers device list updates /// - 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( pub(crate) async fn logout_route(
State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp, body: Ruma<logout::v3::Request>, State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp, body: Ruma<logout::v3::Request>,
) -> Result<logout::v3::Response> { ) -> Result<logout::v3::Response> {
@ -249,7 +249,7 @@ pub(crate) async fn logout_route(
/// Note: This is equivalent to calling [`GET /// Note: This is equivalent to calling [`GET
/// /_matrix/client/r0/logout`](fn.logout_route.html) from each device of this /// /_matrix/client/r0/logout`](fn.logout_route.html) from each device of this
/// user. /// user.
#[tracing::instrument(skip_all, fields(%client), name = "register")] #[tracing::instrument(skip_all, fields(%client), name = "logout")]
pub(crate) async fn logout_all_route( pub(crate) async fn logout_all_route(
State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp, State(services): State<crate::State>, InsecureClientIp(client): InsecureClientIp,
body: Ruma<logout_all::v3::Request>, body: Ruma<logout_all::v3::Request>,