From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Fri, 2 Feb 2024 15:28:20 +0000 (+0000)
Subject: client: Better handling of JSON deser failures
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ian/git?a=commitdiff_plain;h=d1379aa0291bd50795a49b5dced17edc36a06675;p=mastodonochrome.git

client: Better handling of JSON deser failures

If the returned JSON doesn't match the schema implied by our struct,
we now print (a normalised form of) the JSON as part of our error.

If the input isn't JSON at all, we don't include the malformed JSON
text.  It might be hazardous.
---

diff --git a/src/client.rs b/src/client.rs
index b8d66cd..c17077f 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -163,6 +163,8 @@ pub enum ClientError {
 
     LinkParse(String, String), // url, parsing error message
     JSONParse(String, String), // url, parsing error message
+    InvalidJSONSyntax(String, String), // url, parsing error message
+    UnexpectedJSONContent(String, String, String), // url, deser msg, json
     UrlConsistency(String, String), // url, error message
     Consistency(String),       // just error message
 }
@@ -229,6 +231,15 @@ impl std::fmt::Display for ClientError {
             ClientError::JSONParse(ref url, ref msg) => {
                 write!(f, "{} (parsing JSON returned from URL: {})", msg, url)
             }
+            ClientError::InvalidJSONSyntax(url, msg) => {
+                write!(f, "{msg} (bad JSON syntax from URL: {url})")
+            }
+            ClientError::UnexpectedJSONContent(url, msg, json) => {
+                write!(
+                    f,
+            "{msg} (unexpected JSON value from URL: {url}, received {json})",
+                )
+            }
             ClientError::LinkParse(ref url, ref msg) => {
                 write!(
                     f,
@@ -666,13 +677,33 @@ impl Client {
         &mut self,
         req: Req,
     ) -> Result<(T, String), ClientError> {
+        // There's a lot of code here that doesn't depend on T, but
+        // the generics will monomorphise it for each T.  If we cared enough
+        // about binary size, we could make a non-generic inner function.
         let (url, rsp) = self.api_request(req)?;
         let rspstatus = rsp.status();
         if !rspstatus.is_success() {
             return Err(ClientError::from_response(&url, rsp));
         }
-        let t: T = serde_json::from_str(&rsp.text()?)
-            .map_err(|e| ClientError::JSONParse(url.clone(), e.to_string()))?;
+        let text = rsp.text()?;
+        let t: T = serde_json::from_str(&text).map_err(|e0| {
+            let url = url.clone();
+            let val = match serde_json::from_str::<serde_json::Value>(&text) {
+                Ok(y) => y,
+                Err(e) => {
+                    return ClientError::InvalidJSONSyntax(url, e.to_string())
+                },
+            };
+            let val_restring = serde_json::to_string(&val)
+                .unwrap_or_else(|e| format!("failed to regenerate json! {e}"));
+            let e = match serde_json::from_value::<T>(val) {
+                Err(e) => e.to_string(),
+                Ok(_wat) => format!(
+           "unexpectedly parsed Ok from Value! (earlier, from_str gave {e0}",
+                ),
+            };
+            ClientError::UnexpectedJSONContent(url, e, val_restring)
+        })?;
         Ok((t, url))
     }
 
diff --git a/src/text.rs b/src/text.rs
index be1ab9e..b978c80 100644
--- a/src/text.rs
+++ b/src/text.rs
@@ -3413,6 +3413,27 @@ impl ErrorLogEntry {
                 log_msg(&mut paras, msg);
                 log_url(&mut paras, url);
             }
+            ClientError::InvalidJSONSyntax(url, msg) => {
+                title.push_text(
+                    ColouredString::plain("JSON syntax error"),
+                    false,
+                );
+                log_msg(&mut paras, msg);
+                log_url(&mut paras, url);
+            }
+            ClientError::UnexpectedJSONContent(url, msg, json) => {
+                title.push_text(
+                    ColouredString::plain("JSON content error"),
+                    false,
+                );
+                log_msg(&mut paras, msg);
+                log_url(&mut paras, url);
+                paras.push(
+                    Paragraph::new()
+                        .add(ColouredString::plain("response JSON: "))
+                        .add(ColouredString::uniform(&json, 'u')),
+                );
+            }
             ClientError::LinkParse(url, msg) => {
                 title.push_text(
                     ColouredString::plain("Link header parsing error"),