Use thiserror for credential provider errors

credential/cargo-credential-1password/src/lib.rs

@@ -80,7 +80,7 @@ impl OnePasswordKeychain {
         let mut cmd = Command::new("op");
         cmd.args(["signin", "--raw"]);
         cmd.stdout(Stdio::piped());
-        cmd.stdin(cargo_credential::tty()?);
+        cmd.stdin(cargo_credential::tty().map_err(Box::new)?);
         let mut child = cmd
             .spawn()
             .map_err(|e| format!("failed to spawn `op`: {}", e))?;
@@ -228,7 +228,7 @@ impl OnePasswordKeychain {
         // For unknown reasons, `op item create` seems to not be happy if
         // stdin is not a tty. Otherwise it returns with a 0 exit code without
         // doing anything.
-        cmd.stdin(cargo_credential::tty()?);
+        cmd.stdin(cargo_credential::tty().map_err(Box::new)?);
         self.run_cmd(cmd)?;
         Ok(())
     }

credential/cargo-credential-wincred/src/lib.rs

@@ -58,7 +58,7 @@ mod win {
                             if err.raw_os_error() == Some(ERROR_NOT_FOUND as i32) {
                                 return Err(Error::NotFound);
                             }
-                            return Err(err.into());
+                            return Err(Box::new(err).into());
                         }
                         std::slice::from_raw_parts(
                             (*p_credential).CredentialBlob,
@@ -97,7 +97,7 @@ mod win {
                     let result = unsafe { CredWriteW(&credential, 0) };
                     if result != TRUE {
                         let err = std::io::Error::last_os_error();
-                        return Err(err.into());
+                        return Err(Box::new(err).into());
                     }
                     Ok(CredentialResponse::Login)
                 }
@@ -109,7 +109,7 @@ mod win {
                         if err.raw_os_error() == Some(ERROR_NOT_FOUND as i32) {
                             return Err(Error::NotFound);
                         }
-                        return Err(err.into());
+                        return Err(Box::new(err).into());
                     }
                     Ok(CredentialResponse::Logout)
                 }

credential/cargo-credential/src/error.rs

@@ -13,29 +13,32 @@ use thiserror::Error as ThisError;
 #[serde(rename_all = "kebab-case", tag = "kind")]
 #[non_exhaustive]
 pub enum Error {
+    /// Registry URL is not supported. This should be used if
+    /// the provider only works for some registries. Cargo will
+    /// try another provider, if available
     #[error("registry not supported")]
     UrlNotSupported,
+
+    /// Credentials could not be found. Cargo will try another
+    /// provider, if available
     #[error("credential not found")]
     NotFound,
+
+    /// The provider doesn't support this operation, such as
+    /// a provider that can't support 'login' / 'logout'
     #[error("requested operation not supported")]
     OperationNotSupported,
-    #[error("protocol version {version} not supported")]
-    ProtocolNotSupported { version: u32 },
+
+    /// The provider failed to perform the operation. Other
+    /// providers will not be attempted
     #[error(transparent)]
     #[serde(with = "error_serialize")]
     Other(Box<dyn StdError + Sync + Send>),
-}
-
-impl From<std::io::Error> for Error {
-    fn from(err: std::io::Error) -> Self {
-        Box::new(err).into()
-    }
-}
 
-impl From<serde_json::Error> for Error {
-    fn from(err: serde_json::Error) -> Self {
-        Box::new(err).into()
-    }
+    /// A new variant was added to this enum since Cargo was built
+    #[error("unknown error kind; try updating Cargo?")]
+    #[serde(other)]
+    Unknown,
 }
 
 impl From<String> for Error {
@@ -156,6 +159,16 @@ mod error_serialize {
 mod tests {
     use super::Error;
 
+    #[test]
+    pub fn unknown_kind() {
+        let json = r#"{
+            "kind": "unexpected-kind",
+            "unexpected-content": "test"
+          }"#;
+        let e: Error = serde_json::from_str(&json).unwrap();
+        assert!(matches!(e, Error::Unknown));
+    }
+
     #[test]
     pub fn roundtrip() {
         // Construct an error with context

credential/cargo-credential/src/lib.rs

@@ -189,23 +189,24 @@ fn doit(credential: impl Credential) -> Result<(), Error> {
     let hello = CredentialHello {
         v: vec![PROTOCOL_VERSION_1],
     };
-    serde_json::to_writer(std::io::stdout(), &hello)?;
+    serde_json::to_writer(std::io::stdout(), &hello).map_err(Box::new)?;
     println!();
 
     loop {
         let mut buffer = String::new();
-        let len = std::io::stdin().read_line(&mut buffer)?;
+        let len = std::io::stdin().read_line(&mut buffer).map_err(Box::new)?;
         if len == 0 {
             return Ok(());
         }
-        let request: CredentialRequest = serde_json::from_str(&buffer)?;
+        let request: CredentialRequest = serde_json::from_str(&buffer).map_err(Box::new)?;
         if request.v != PROTOCOL_VERSION_1 {
-            return Err(Error::ProtocolNotSupported { version: request.v });
+            return Err(format!("unsupported protocol version {}", request.v).into());
         }
         serde_json::to_writer(
             std::io::stdout(),
             &credential.perform(&request.registry, &request.action, &request.args),
-        )?;
+        )
+        .map_err(Box::new)?;
         println!();
     }
 }
@@ -248,5 +249,5 @@ pub fn read_token(
         eprintln!("please paste the token for {} below", registry.index_url);
     }
 
-    Ok(Secret::from(read_line()?))
+    Ok(Secret::from(read_line().map_err(Box::new)?))
 }

src/cargo/util/credential/adaptor.rs

@@ -5,6 +5,7 @@ use std::{
     process::{Command, Stdio},
 };
 
+use anyhow::Context;
 use cargo_credential::{
     Action, CacheControl, Credential, CredentialResponse, RegistryInfo, Secret,
 };
@@ -32,9 +33,14 @@ impl Credential for BasicProcessCredential {
                     cmd.env("CARGO_REGISTRY_NAME_OPT", name);
                 }
                 cmd.stdout(Stdio::piped());
-                let mut child = cmd.spawn()?;
+                let mut child = cmd.spawn().context("failed to spawn credential process")?;
                 let mut buffer = String::new();
-                child.stdout.take().unwrap().read_to_string(&mut buffer)?;
+                child
+                    .stdout
+                    .take()
+                    .unwrap()
+                    .read_to_string(&mut buffer)
+                    .context("failed to read from credential provider")?;
                 if let Some(end) = buffer.find('\n') {
                     if buffer.len() > end + 1 {
                         return Err(format!(
@@ -46,7 +52,7 @@ impl Credential for BasicProcessCredential {
                     }
                     buffer.truncate(end);
                 }
-                let status = child.wait().expect("process was started");
+                let status = child.wait().context("credential process never started")?;
                 if !status.success() {
                     return Err(format!("process `{}` failed with status `{status}`", exe).into());
                 }

src/cargo/util/credential/process.rs

@@ -36,17 +36,15 @@ impl<'a> Credential for CredentialProcessCredential {
         cmd.stdin(Stdio::piped());
         cmd.arg("--cargo-plugin");
         log::debug!("credential-process: {cmd:?}");
-        let mut child = cmd.spawn().with_context(|| {
-            format!(
-                "failed to spawn credential process `{}`",
-                self.path.display()
-            )
-        })?;
+        let mut child = cmd.spawn().context("failed to spawn credential process")?;
         let mut output_from_child = BufReader::new(child.stdout.take().unwrap());
         let mut input_to_child = child.stdin.take().unwrap();
         let mut buffer = String::new();
-        output_from_child.read_line(&mut buffer)?;
-        let credential_hello: CredentialHello = serde_json::from_str(&buffer)?;
+        output_from_child
+            .read_line(&mut buffer)
+            .context("failed to read hello from credential provider")?;
+        let credential_hello: CredentialHello =
+            serde_json::from_str(&buffer).context("failed to deserialize hello")?;
         log::debug!("credential-process > {credential_hello:?}");
 
         let req = CredentialRequest {
@@ -55,17 +53,19 @@ impl<'a> Credential for CredentialProcessCredential {
             registry: registry.clone(),
             args: args.to_vec(),
         };
-        let request = serde_json::to_string(&req)?;
+        let request = serde_json::to_string(&req).context("failed to serialize request")?;
         log::debug!("credential-process < {req:?}");
-        writeln!(input_to_child, "{request}")?;
+        writeln!(input_to_child, "{request}").context("failed to write to credential provider")?;
 
         buffer.clear();
-        output_from_child.read_line(&mut buffer)?;
+        output_from_child
+            .read_line(&mut buffer)
+            .context("failed to read response from credential provider")?;
         let response: Result<CredentialResponse, cargo_credential::Error> =
-            serde_json::from_str(&buffer)?;
+            serde_json::from_str(&buffer).context("failed to deserialize response")?;
         log::debug!("credential-process > {response:?}");
         drop(input_to_child);
-        let status = child.wait().expect("credential process never started");
+        let status = child.wait().context("credential process never started")?;
         if !status.success() {
             return Err(anyhow::anyhow!(
                 "credential process `{}` failed with status {}`",