chiark / gitweb /
client: Better handling of JSON deser failures
authorIan Jackson <ijackson@chiark.greenend.org.uk>
Fri, 2 Feb 2024 15:28:20 +0000 (15:28 +0000)
committerIan Jackson <ijackson@chiark.greenend.org.uk>
Fri, 2 Feb 2024 22:56:25 +0000 (22:56 +0000)
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.

src/client.rs
src/text.rs

index b8d66cd95fa4255849683265c640339434b0b760..c17077f6b98880bda1fcf4cf6a76f96f05edb553 100644 (file)
@@ -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))
     }
 
index be1ab9ec66a3d92b41b7e84c266d53343470f5e2..b978c8003432702c97bdf88f7f0ba3acadf4ff42 100644 (file)
@@ -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"),