From 9e8d466b40d9c65be7c3efc37f042b771ae74c25 Mon Sep 17 00:00:00 2001 From: Nutomic Date: Wed, 20 Dec 2023 12:19:58 +0100 Subject: [PATCH] Better JSON error messages (#91) --- src/activity_sending.rs | 6 ++++-- src/actix_web/inbox.rs | 4 ++-- src/error.rs | 16 +++++++++++----- src/fetch/mod.rs | 15 ++++++++++----- src/lib.rs | 6 ++---- src/reqwest_shim.rs | 34 ---------------------------------- 6 files changed, 29 insertions(+), 52 deletions(-) diff --git a/src/activity_sending.rs b/src/activity_sending.rs index 849fde6..bd236ad 100644 --- a/src/activity_sending.rs +++ b/src/activity_sending.rs @@ -56,14 +56,16 @@ impl SendActivityTask<'_> { data: &Data, ) -> Result>, Error> where - Activity: ActivityHandler + Serialize, + Activity: ActivityHandler + Serialize + Debug, Datatype: Clone, ActorType: Actor, { let config = &data.config; let actor_id = activity.actor(); let activity_id = activity.id(); - let activity_serialized: Bytes = serde_json::to_vec(&activity).map_err(Error::Json)?.into(); + let activity_serialized: Bytes = serde_json::to_vec(&activity) + .map_err(|e| Error::SerializeOutgoingActivity(e, format!("{:?}", activity)))? + .into(); let private_key = get_pkey_cached(data, actor).await?; Ok(futures::stream::iter( diff --git a/src/actix_web/inbox.rs b/src/actix_web/inbox.rs index a2b55d4..b9c6379 100644 --- a/src/actix_web/inbox.rs +++ b/src/actix_web/inbox.rs @@ -130,8 +130,8 @@ mod test { .await; match res { - Err(Error::ParseReceivedActivity(url, _)) => { - assert_eq!(id, url.as_str()); + Err(Error::ParseReceivedActivity(_, url)) => { + assert_eq!(id, url.expect("has url").as_str()); } _ => unreachable!(), } diff --git a/src/error.rs b/src/error.rs index 89f6abf..ba9248a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -35,12 +35,18 @@ pub enum Error { /// Failed to resolve actor via webfinger #[error("Failed to resolve actor via webfinger")] WebfingerResolveFailed(#[from] WebFingerError), - /// JSON Error - #[error(transparent)] - Json(#[from] serde_json::Error), + /// Failed to serialize outgoing activity + #[error("Failed to serialize outgoing activity {1}: {0}")] + SerializeOutgoingActivity(serde_json::Error, String), + /// Failed to parse an object fetched from url + #[error("Failed to parse object {1} with content {2}: {0}")] + ParseFetchedObject(serde_json::Error, Url, String), /// Failed to parse an activity received from another instance - #[error("Failed to parse incoming activity with id {0}: {1}")] - ParseReceivedActivity(Url, serde_json::Error), + #[error("Failed to parse incoming activity {}: {0}", match .1 { + Some(t) => format!("with id {t}"), + None => String::new(), + })] + ParseReceivedActivity(serde_json::Error, Option), /// Reqwest Middleware Error #[error(transparent)] ReqwestMiddleware(#[from] reqwest_middleware::Error), diff --git a/src/fetch/mod.rs b/src/fetch/mod.rs index 4a0d502..474d946 100644 --- a/src/fetch/mod.rs +++ b/src/fetch/mod.rs @@ -4,7 +4,7 @@ use crate::{ config::Data, - error::Error, + error::{Error, Error::ParseFetchedObject}, http_signatures::sign_request, reqwest_shim::ResponseExt, FEDERATION_CONTENT_TYPE, @@ -93,8 +93,13 @@ async fn fetch_object_http_with_accept( } let url = res.url().clone(); - Ok(FetchObjectResponse { - object: res.json_limited().await?, - url, - }) + let text = res.bytes_limited().await?; + match serde_json::from_slice(&text) { + Ok(object) => Ok(FetchObjectResponse { object, url }), + Err(e) => Err(ParseFetchedObject( + e, + url, + String::from_utf8(Vec::from(text))?, + )), + } } diff --git a/src/lib.rs b/src/lib.rs index 42da8df..f482aa0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -57,10 +57,8 @@ where struct Id { id: Url, } - match serde_json::from_slice::(body) { - Ok(id) => Error::ParseReceivedActivity(id.id, e), - Err(e) => Error::Json(e), - } + let id = serde_json::from_slice::(body).ok(); + Error::ParseReceivedActivity(e, id.map(|i| i.id)) })?; data.config.verify_url_and_domain(&activity).await?; let actor = ObjectId::::from(activity.actor().clone()) diff --git a/src/reqwest_shim.rs b/src/reqwest_shim.rs index 9db846e..9ebe108 100644 --- a/src/reqwest_shim.rs +++ b/src/reqwest_shim.rs @@ -3,10 +3,8 @@ use bytes::{BufMut, Bytes, BytesMut}; use futures_core::{ready, stream::BoxStream, Stream}; use pin_project_lite::pin_project; use reqwest::Response; -use serde::de::DeserializeOwned; use std::{ future::Future, - marker::PhantomData, mem, pin::Pin, task::{Context, Poll}, @@ -46,27 +44,6 @@ impl Future for BytesFuture { } } -pin_project! { - pub struct JsonFuture { - _t: PhantomData, - #[pin] - future: BytesFuture, - } -} - -impl Future for JsonFuture -where - T: DeserializeOwned, -{ - type Output = Result; - - fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { - let this = self.project(); - let bytes = ready!(this.future.poll(cx))?; - Poll::Ready(serde_json::from_slice(&bytes).map_err(Error::Json)) - } -} - pin_project! { pub struct TextFuture { #[pin] @@ -94,20 +71,16 @@ impl Future for TextFuture { /// TODO: Remove this shim as soon as reqwest gets support for size-limited bodies. pub trait ResponseExt { type BytesFuture; - type JsonFuture; type TextFuture; /// Size limited version of `bytes` to work around a reqwest issue. Check [`ResponseExt`] docs for details. fn bytes_limited(self) -> Self::BytesFuture; - /// Size limited version of `json` to work around a reqwest issue. Check [`ResponseExt`] docs for details. - fn json_limited(self) -> Self::JsonFuture; /// Size limited version of `text` to work around a reqwest issue. Check [`ResponseExt`] docs for details. fn text_limited(self) -> Self::TextFuture; } impl ResponseExt for Response { type BytesFuture = BytesFuture; - type JsonFuture = JsonFuture; type TextFuture = TextFuture; fn bytes_limited(self) -> Self::BytesFuture { @@ -118,13 +91,6 @@ impl ResponseExt for Response { } } - fn json_limited(self) -> Self::JsonFuture { - JsonFuture { - _t: PhantomData, - future: self.bytes_limited(), - } - } - fn text_limited(self) -> Self::TextFuture { TextFuture { future: self.bytes_limited(),