From 7918c7fc7b5f3c3a2728adb66641c8339cfbdee8 Mon Sep 17 00:00:00 2001 From: Arlo Siemsen Date: Mon, 31 Jul 2023 16:01:20 -0500 Subject: [PATCH] Remove from impls --- .../cargo-credential-1password/src/lib.rs | 4 +- .../cargo-credential-wincred/src/lib.rs | 6 +-- credential/cargo-credential/src/error.rs | 39 ++++++++++++------- credential/cargo-credential/src/lib.rs | 13 ++++--- src/cargo/util/credential/adaptor.rs | 12 ++++-- src/cargo/util/credential/process.rs | 26 ++++++------- 6 files changed, 60 insertions(+), 40 deletions(-) diff --git a/credential/cargo-credential-1password/src/lib.rs b/credential/cargo-credential-1password/src/lib.rs index b5116da83..76e01cb79 100644 --- a/credential/cargo-credential-1password/src/lib.rs +++ b/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(()) } diff --git a/credential/cargo-credential-wincred/src/lib.rs b/credential/cargo-credential-wincred/src/lib.rs index 5f75a0f64..9200ca58f 100644 --- a/credential/cargo-credential-wincred/src/lib.rs +++ b/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) } diff --git a/credential/cargo-credential/src/error.rs b/credential/cargo-credential/src/error.rs index 39083cbcf..2ebaf9977 100644 --- a/credential/cargo-credential/src/error.rs +++ b/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), -} -impl From for Error { - fn from(err: std::io::Error) -> Self { - Box::new(err).into() - } -} - -impl From 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 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 diff --git a/credential/cargo-credential/src/lib.rs b/credential/cargo-credential/src/lib.rs index 9e2929452..138eca7ed 100644 --- a/credential/cargo-credential/src/lib.rs +++ b/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)?)) } diff --git a/src/cargo/util/credential/adaptor.rs b/src/cargo/util/credential/adaptor.rs index 1a71fea9a..d9cbc6ffa 100644 --- a/src/cargo/util/credential/adaptor.rs +++ b/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()); } diff --git a/src/cargo/util/credential/process.rs b/src/cargo/util/credential/process.rs index 1e1fc37c7..07551aee3 100644 --- a/src/cargo/util/credential/process.rs +++ b/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 = - 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 {}`",