From cbc0c72a74caca3f7f0289363d0695dae79eb6df Mon Sep 17 00:00:00 2001 From: Jordan Doyle Date: Sat, 12 Mar 2022 18:57:49 +0000 Subject: [PATCH] Stop thrussh from swallowing returned `Err`s --- src/main.rs | 31 ++++++++++++++++++++++--------- src/providers/gitlab.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------- 2 files changed, 70 insertions(+), 49 deletions(-) diff --git a/src/main.rs b/src/main.rs index f13595e..7438bce 100644 --- a/src/main.rs +++ b/src/main.rs @@ -279,7 +279,7 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser let fingerprint = public_key.fingerprint(); let user = user.to_string(); - Box::pin(async move { + Box::pin(capture_errors(async move { let mut user = self .gitlab .find_user_by_username_password_combo(&user) @@ -298,13 +298,13 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser } else { self.finished_auth(Auth::Reject).await } - }) + })) } fn data(mut self, channel: ChannelId, data: &[u8], mut session: Session) -> Self::FutureUnit { self.input_bytes.extend_from_slice(data); - Box::pin(async move { + Box::pin(capture_errors(async move { // start building the packfile we're going to send to the user let (commit_hash, packfile_entries) = &*self.build_packfile().await?; @@ -347,7 +347,7 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser } Ok((self, session)) - }) + })) } fn env_request( @@ -367,7 +367,7 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser } fn shell_request(mut self, channel: ChannelId, mut session: Session) -> Self::FutureUnit { - Box::pin(async move { + Box::pin(capture_errors(async move { let username = self.user()?.username.clone(); write!( &mut self.output_bytes, @@ -378,7 +378,7 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser self.flush(&mut session, channel); session.close(channel); Ok((self, session)) - }) + })) } /// Initially when setting up the SSH connection, the remote Git client will send us an @@ -395,12 +395,12 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser ) -> Self::FutureUnit { let data = match std::str::from_utf8(data) { Ok(data) => data, - Err(e) => return Box::pin(futures::future::err(e.into())), + Err(e) => return Box::pin(capture_errors(futures::future::err(e.into()))), }; // parses the given args in the same fashion as a POSIX shell let args = shlex::split(data); - Box::pin(async move { + Box::pin(capture_errors(async move { // if the client didn't send `GIT_PROTOCOL=version=2` as an environment // variable when connecting, we'll just close the connection if !self.is_git_protocol_v2 { @@ -455,10 +455,23 @@ impl<'a, U: UserProvider + PackageProvider + Send + Sync + 'static> thrussh::ser self.flush(&mut session, channel); Ok((self, session)) - }) + })) } } +// a workaround for trussh swallowing errors +async fn capture_errors( + fut: impl Future>, +) -> Result { + let res = fut.await; + + if let Err(e) = &res { + error!("Error: {}", e); + } + + res +} + #[derive(Hash, Debug, PartialEq, Eq)] struct MetadataCacheKey<'a> { checksum: Cow<'a, str>, diff --git a/src/providers/gitlab.rs b/src/providers/gitlab.rs index a85e8ab..9d27d10 100644 --- a/src/providers/gitlab.rs +++ b/src/providers/gitlab.rs @@ -32,7 +32,6 @@ impl Gitlab { } } -// TODO: errors are not yet handled, they're returned as {"error": "abc"} #[async_trait] impl super::UserProvider for Gitlab { async fn find_user_by_username_password_combo( @@ -67,17 +66,19 @@ impl super::UserProvider for Gitlab { } async fn find_user_by_ssh_key(&self, fingerprint: &str) -> anyhow::Result> { - let res: GitlabSshKeyLookupResponse = self - .client - .get(format!( - "{}/keys?fingerprint={}", - self.base_url, - utf8_percent_encode(fingerprint, NON_ALPHANUMERIC) - )) - .send() - .await? - .json() - .await?; + let res: GitlabSshKeyLookupResponse = handle_error( + self.client + .get(format!( + "{}/keys?fingerprint={}", + self.base_url, + utf8_percent_encode(fingerprint, NON_ALPHANUMERIC) + )) + .send() + .await?, + ) + .await? + .json() + .await?; Ok(res.user.map(|u| User { id: u.id, username: u.username, @@ -85,20 +86,22 @@ impl super::UserProvider for Gitlab { } async fn fetch_token_for_user(&self, user: &User) -> anyhow::Result { - let impersonation_token: GitlabImpersonationTokenResponse = self - .client - .post(format!( - "{}/users/{}/impersonation_tokens", - self.base_url, user.id - )) - .json(&GitlabImpersonationTokenRequest { - name: env!("CARGO_PKG_NAME"), - scopes: vec!["api"], - }) - .send() - .await? - .json() - .await?; + let impersonation_token: GitlabImpersonationTokenResponse = handle_error( + self.client + .post(format!( + "{}/users/{}/impersonation_tokens", + self.base_url, user.id + )) + .json(&GitlabImpersonationTokenRequest { + name: env!("CARGO_PKG_NAME"), + scopes: vec!["api"], + }) + .send() + .await?, + ) + .await? + .json() + .await?; Ok(impersonation_token.token) } @@ -140,7 +143,7 @@ impl super::PackageProvider for Gitlab { let futures = FuturesUnordered::new(); while let Some(uri) = next_uri.take() { - let res = self.client.get(uri).send().await?; + let res = handle_error(self.client.get(uri).send().await?).await?; if let Some(link_header) = res.headers().get(reqwest::header::LINK) { let mut link_header = parse_link_header::parse_with_rel(link_header.to_str()?)?; @@ -170,18 +173,20 @@ impl super::PackageProvider for Gitlab { .to_string(), }); - let package_files: Vec = this - .client - .get(format!( - "{}/projects/{}/packages/{}/package_files", - this.base_url, - utf8_percent_encode(project, NON_ALPHANUMERIC), - utf8_percent_encode(package, NON_ALPHANUMERIC), - )) - .send() - .await? - .json() - .await?; + let package_files: Vec = handle_error( + this.client + .get(format!( + "{}/projects/{}/packages/{}/package_files", + this.base_url, + utf8_percent_encode(project, NON_ALPHANUMERIC), + utf8_percent_encode(package, NON_ALPHANUMERIC), + )) + .send() + .await?, + ) + .await? + .json() + .await?; let expected_file_name = format!("{}-{}.crate", release.name, release.version); @@ -218,7 +223,10 @@ impl super::PackageProvider for Gitlab { ) -> anyhow::Result { let uri = format!("{}{}", self.base_url, path.metadata_uri(version),); - Ok(self.client.get(uri).send().await?.json().await?) + Ok(handle_error(self.client.get(uri).send().await?) + .await? + .json() + .await?) } fn cargo_dl_uri(&self, group: &Group, token: &str) -> String { -- libgit2 1.7.2