review-classifier: keep the stored value out of a failed read's text

swarm-review-classifier/src/run.rs

@@ -186,6 +186,35 @@ impl KeySource for Store {
     }
 }
 
+/// `e` and each of its sources, joined by `: `. The store client's errors
+/// carry only a generic message at the top; the URL and the failure kind sit
+/// below it. For login errors only: they hold URLs and error kinds, not values.
+fn chain(e: &dyn std::error::Error) -> String {
+    let mut out = e.to_string();
+    let mut next = e.source();
+    while let Some(cause) = next {
+        out.push_str(": ");
+        out.push_str(&cause.to_string());
+        next = cause.source();
+    }
+    out
+}
+
+/// The run failure for a store login that failed with `e`.
+fn login_failure(what: &str, e: &dyn std::error::Error) -> String {
+    format!(
+        "logging in to the secret store to read the {what}: {}",
+        chain(e)
+    )
+}
+
+/// The run failure for a store read that failed with `e`. Shows `e` alone:
+/// a parse error's sources quote the stored value, and the failure text ends
+/// up in a review comment.
+fn read_failure(path: &str, e: &dyn std::error::Error) -> String {
+    format!("reading secret/{path}: {e}")
+}
+
 /// The key `field` takes from the object at `path`, trimmed; `None` when
 /// nothing or a blank is stored.
 async fn read_key<T: serde::de::DeserializeOwned>(
@@ -195,11 +224,11 @@ async fn read_key<T: serde::de::DeserializeOwned>(
 ) -> Result<Option<String>, String> {
     let store = swarm_secret_client::SecretStore::from_env(crate::CERT_ROLE)
         .await
-        .map_err(|e| format!("logging in to the secret store to read the {what}: {e}"))?;
+        .map_err(|e| login_failure(what, &e))?;
     let stored: Option<T> = store
         .read_optional(path)
         .await
-        .map_err(|e| format!("reading secret/{path}: {e}"))?;
+        .map_err(|e| read_failure(path, &e))?;
     Ok(stored
         .map(field)
         .map(|key| key.trim().to_owned())
@@ -380,6 +409,69 @@ mod tests {
 
     use super::*;
 
+    /// A failure with a cause, as the store client's errors nest them.
+    #[derive(Debug)]
+    struct Level(&'static str, Option<Box<Level>>);
+
+    impl std::fmt::Display for Level {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            f.write_str(self.0)
+        }
+    }
+
+    impl std::error::Error for Level {
+        fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+            self.1.as_deref().map(|l| l as _)
+        }
+    }
+
+    /// A parse failure whose source quotes the stored value.
+    #[derive(Debug)]
+    struct Unparsable(serde_json::Error);
+
+    impl std::fmt::Display for Unparsable {
+        fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+            f.write_str("error parsing the response")
+        }
+    }
+
+    impl std::error::Error for Unparsable {
+        fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+            Some(&self.0)
+        }
+    }
+
+    #[test]
+    fn read_failure_leaves_out_the_stored_value() {
+        let cause = serde_json::from_str::<u32>("\"hunter2\"").unwrap_err();
+        assert!(cause.to_string().contains("hunter2"), "control");
+        let text = read_failure("p", &Unparsable(cause));
+        assert_eq!(text, "reading secret/p: error parsing the response");
+    }
+
+    #[test]
+    fn chain_renders_every_level() {
+        let e = Level(
+            "An error occurred with the request",
+            Some(Box::new(Level(
+                "error sending request for url (https://bao/v1/auth/cert/login)",
+                Some(Box::new(Level("tls handshake eof", None))),
+            ))),
+        );
+        assert_eq!(
+            chain(&e),
+            "An error occurred with the request: error sending request for url \
+             (https://bao/v1/auth/cert/login): tls handshake eof"
+        );
+        assert_eq!(
+            login_failure("API key", &e),
+            "logging in to the secret store to read the API key: An error occurred with the \
+             request: error sending request for url (https://bao/v1/auth/cert/login): tls \
+             handshake eof"
+        );
+        assert_eq!(chain(&Level("alone", None)), "alone");
+    }
+
     /// The API key runtime's key is `k` and the ACP runtime's `p`, or both
     /// fail with the same error.
     struct Key(Result<(), &'static str>);