From 6a7e64d04a8b0ad932169df43e2d93efe8ceedf8 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 18 Aug 2026 07:18:07 +0900 Subject: fix: propagate ported exceptions instead of flattening them `ProcessExecutor::execute_args` existed only to turn `execute`'s `anyhow::Result` into an exit code of 1, so every one of its ~87 call sites silently took the "command failed" branch on an error PHP would have thrown. It is gone; callers use `execute` and propagate with `?`. Where the enclosing function had no `Result` to propagate into, its signature grew one, up to and including `Git::get_version`, `Svn::binary_version`, `GitHub`/`GitLab`/`Bitbucket::authorize_oauth`, `InitCommand::get_git_config` and `DiagnoseCommand::check_git`. The VCS drivers had the same problem in the other direction: their `get_contents` returned `Result>`, a type too narrow for the PHP method, which lets any Throwable out of the `catch (TransportException $e)` block. Every non-transport error was therefore rewritten into a `TransportException` with code 0, which the callers switch on. They now return `anyhow::Result>>`: the outer `Result` carries what PHP does not catch, the inner one the exception the drivers handle. That signature also restores `GitLabDriver::getContents`: the 400/401 `TransportException`s it raises to force authentication are thrown inside its own `try` block and handled by its own `catch`, but the port returned them straight to the caller, so the authentication flow behind them never ran. `impl_php_exception!` gains `From> for anyhow::Error` so a caught exception can be re-propagated with `?`. Co-Authored-By: Claude Opus 5 (1M context) --- .../shirabe/src/repository/vcs/forgejo_driver.rs | 114 +++++------ crates/shirabe/src/repository/vcs/fossil_driver.rs | 40 ++-- .../src/repository/vcs/git_bitbucket_driver.rs | 69 ++++--- crates/shirabe/src/repository/vcs/git_driver.rs | 26 +-- crates/shirabe/src/repository/vcs/github_driver.rs | 64 +++---- crates/shirabe/src/repository/vcs/gitlab_driver.rs | 212 ++++++++++----------- crates/shirabe/src/repository/vcs/hg_driver.rs | 38 ++-- .../shirabe/src/repository/vcs/perforce_driver.rs | 5 +- crates/shirabe/src/repository/vcs/svn_driver.rs | 6 +- crates/shirabe/src/repository/vcs/vcs_driver.rs | 39 ++-- 10 files changed, 286 insertions(+), 327 deletions(-) (limited to 'crates/shirabe/src/repository/vcs') diff --git a/crates/shirabe/src/repository/vcs/forgejo_driver.rs b/crates/shirabe/src/repository/vcs/forgejo_driver.rs index 4238be8f..8c5cb345 100644 --- a/crates/shirabe/src/repository/vcs/forgejo_driver.rs +++ b/crates/shirabe/src/repository/vcs/forgejo_driver.rs @@ -106,9 +106,7 @@ impl ForgejoDriver { file, urlencode(identifier) ); - let response = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&resource_url, false)??; let mut resource = response.decode_json()?; // The Forgejo contents API only returns files up to 1MB as base64 encoded files; @@ -133,10 +131,7 @@ impl ForgejoDriver { None }; if let Some(git_url) = git_url { - resource = self - .get_contents(&git_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + resource = self.get_contents(&git_url, false)??.decode_json()?; } } @@ -192,10 +187,7 @@ impl ForgejoDriver { api_url, urlencode(identifier) ); - let commit = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let commit = self.get_contents(&resource_url, false)??.decode_json()?; let date_str = if let PhpMixed::Array(ref arr) = commit { arr.get("commit") @@ -241,9 +233,7 @@ impl ForgejoDriver { let mut resource: Option = Some(format!("{}/branches?per_page=100", api_url)); while let Some(url) = resource { - let response = self - .get_contents(&url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&url, false)??; let branch_data = response.decode_json()?; if let PhpMixed::List(ref list) = branch_data { for branch in list { @@ -284,9 +274,7 @@ impl ForgejoDriver { let mut resource: Option = Some(format!("{}/tags?per_page=100", api_url)); while let Some(url) = resource { - let response = self - .get_contents(&url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&url, false)??; let tags_data = response.decode_json()?; if let PhpMixed::List(ref list) = tags_data { for tag in list { @@ -558,7 +546,7 @@ impl ForgejoDriver { } let api_url = self.forgejo_url.as_ref().unwrap().api_url.clone(); - match self.get_contents(&api_url, true) { + match self.get_contents(&api_url, true)? { Err(_) => { if self.git_driver.is_some() { return Ok(()); @@ -598,56 +586,56 @@ impl ForgejoDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - match self.inner.get_contents(url) { - Ok(response) => Ok(response), - Err(e) => match e.get_code() { - 401 | 403 | 404 | 429 => { - if !fetching_repo_data { - return Err(e); - } + ) -> anyhow::Result>> { + let e = match self.inner.get_contents(url)? { + Ok(response) => return Ok(Ok(response)), + Err(e) => e, + }; - if !self.inner.io.is_interactive() { - self.attempt_clone_fallback() - .map_err(|inner_e| TransportException::new(inner_e.to_string(), 0))?; + match e.get_code() { + 401 | 403 | 404 | 429 => { + if !fetching_repo_data { + return Ok(Err(e)); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } + if !self.inner.io.is_interactive() { + self.attempt_clone_fallback()?; - if !self.inner.io.has_authentication(&self.inner.origin_url) { - let origin_url = self.forgejo_url.as_ref().unwrap().origin_url.clone(); - let message = if e.get_code() == 429 { - Some(format!( - "API limit exhausted. Enter your Forgejo credentials to get a larger API limit ({})", - self.inner.url - )) - } else { - None - }; - - let mut forgejo = Forgejo::new( - self.inner.io.clone(), - self.inner.config.clone(), - self.inner.http_downloader.clone(), - ); - let auth_result = forgejo - .authorize_o_auth_interactively(&origin_url, message.as_deref()) - .map_err(|inner_e| TransportException::new(inner_e.to_string(), 0))?; - - if let Ok(true) = auth_result { - return self.inner.get_contents(url); - } - } + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); + } - Err(e) + if !self.inner.io.has_authentication(&self.inner.origin_url) { + let origin_url = self.forgejo_url.as_ref().unwrap().origin_url.clone(); + let message = if e.get_code() == 429 { + Some(format!( + "API limit exhausted. Enter your Forgejo credentials to get a larger API limit ({})", + self.inner.url + )) + } else { + None + }; + + let mut forgejo = Forgejo::new( + self.inner.io.clone(), + self.inner.config.clone(), + self.inner.http_downloader.clone(), + ); + let auth_result = + forgejo.authorize_o_auth_interactively(&origin_url, message.as_deref())?; + + if let Ok(true) = auth_result { + return self.inner.get_contents(url); + } } - _ => Err(e), - }, + + Ok(Err(e)) + } + _ => Ok(Err(e)), } } diff --git a/crates/shirabe/src/repository/vcs/fossil_driver.rs b/crates/shirabe/src/repository/vcs/fossil_driver.rs index 9a678284..1824ad5f 100644 --- a/crates/shirabe/src/repository/vcs/fossil_driver.rs +++ b/crates/shirabe/src/repository/vcs/fossil_driver.rs @@ -97,11 +97,11 @@ impl FossilDriver { fn check_fossil(&self) -> anyhow::Result<()> { let mut ignored_output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "version"].map(|s| s.to_string()).as_ref(), &mut ignored_output, None, - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "fossil was not found, check that it is installed and in your PATH env.\n\n{}", @@ -131,17 +131,17 @@ impl FossilDriver { // update the repo if it is a valid fossil repository if is_file(&repo_file) && is_dir(&self.checkout_dir) - && self.inner.process.borrow_mut().execute_args( + && self.inner.process.borrow_mut().execute( ["fossil", "info"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.checkout_dir), - ) == 0 + )? == 0 { - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "pull"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.checkout_dir), - ) != 0 + )? != 0 { self.inner.io.write_error3(&format!( "Failed to update {}, package information from this repository may be outdated ({})", @@ -156,13 +156,13 @@ impl FossilDriver { fs.ensure_directory_exists(&self.checkout_dir)?; let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "clone", "--", &self.inner.url, &repo_file] .map(|s| s.to_string()) .as_ref(), &mut output, None, - ) != 0 + )? != 0 { let output = self.inner.process.borrow().get_error_output().to_string(); return Err(RuntimeException::new(format!( @@ -172,13 +172,13 @@ impl FossilDriver { .into()); } - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "open", "--nested", "--", &repo_file] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.checkout_dir), - ) != 0 + )? != 0 { let output = self.inner.process.borrow().get_error_output().to_string(); return Err(RuntimeException::new(format!( @@ -225,13 +225,13 @@ impl FossilDriver { } let mut content = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "cat", "-r", identifier, "--", file] .map(|s| s.to_string()) .as_ref(), &mut content, Some(&self.checkout_dir), - ); + )?; if content.trim().is_empty() { return Ok(None); @@ -245,13 +245,13 @@ impl FossilDriver { _identifier: &str, ) -> anyhow::Result>> { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "finfo", "-b", "-n", "1", "composer.json"] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; let parts: Vec<&str> = output.trim().splitn(3, ' ').collect(); let date = parts.get(1).copied().unwrap_or(""); @@ -263,11 +263,11 @@ impl FossilDriver { if self.tags.is_none() { let mut tags: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "tag", "list"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { tags.insert(tag.clone(), tag); } @@ -280,11 +280,11 @@ impl FossilDriver { if self.branches.is_none() { let mut branches: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "branch", "list"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { let branch = preg_replace(php_regex!(r"/^\*/"), "", branch.trim()); let branch = branch.trim().to_string(); @@ -321,11 +321,11 @@ impl FossilDriver { let mut process = ProcessExecutor::new(Some(io)); let mut output = String::new(); - if process.execute_args( + if process.execute( ["fossil", "info"].map(|s| s.to_string()).as_ref(), &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } diff --git a/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs b/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs index b1960974..3761144f 100644 --- a/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs +++ b/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs @@ -641,48 +641,43 @@ impl GitBitbucketDriver { url: &str, fetching_repo_data: bool, ) -> anyhow::Result { - match self.inner.get_contents(url) { - Ok(r) => Ok(r), - Err(e) => { - let mut bitbucket_util = Bitbucket::new( - self.inner.io.clone(), - self.inner.config.clone(), - Some(self.inner.process.clone()), - Some(self.inner.http_downloader.clone()), - None, - )?; - - { - let te = &e; - let code = te.get_code(); - let in_set = matches!(code, 403 | 404); - if in_set - || (401 == code - && strpos(te.get_message(), "Could not authenticate against") - == Some(0)) - { - if !self.inner.io.has_authentication(&self.inner.origin_url) - && bitbucket_util.authorize_oauth(&self.inner.origin_url) - { - return self.inner.get_contents(url).map_err(|e| (*e).into()); - } + let e = match self.inner.get_contents(url)? { + Ok(r) => return Ok(r), + Err(e) => e, + }; - if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback()?; + let mut bitbucket_util = Bitbucket::new( + self.inner.io.clone(), + self.inner.config.clone(), + Some(self.inner.process.clone()), + Some(self.inner.http_downloader.clone()), + None, + )?; + + let code = e.get_code(); + let in_set = matches!(code, 403 | 404); + if in_set + || (401 == code && strpos(e.get_message(), "Could not authenticate against") == Some(0)) + { + if !self.inner.io.has_authentication(&self.inner.origin_url) + && bitbucket_util.authorize_oauth(&self.inner.origin_url)? + { + return Ok(self.inner.get_contents(url)??); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } - } - } + if !self.inner.io.is_interactive() && fetching_repo_data { + self.attempt_clone_fallback()?; - Err((*e).into()) + return Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + )); } } + + Err((*e).into()) } /// Generate an SSH URL diff --git a/crates/shirabe/src/repository/vcs/git_driver.rs b/crates/shirabe/src/repository/vcs/git_driver.rs index f45dbcaa..776a18e5 100644 --- a/crates/shirabe/src/repository/vcs/git_driver.rs +++ b/crates/shirabe/src/repository/vcs/git_driver.rs @@ -185,7 +185,7 @@ impl GitDriver { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "branch".to_string(), @@ -193,7 +193,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; let branches = self.inner.process.borrow().split_lines(&output); if !branches.contains(&"* master".to_string()) { for branch in &branches { @@ -241,7 +241,7 @@ impl GitDriver { } let mut content = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "show".to_string(), @@ -249,7 +249,7 @@ impl GitDriver { ], &mut content, Some(&self.repo_dir), - ); + )?; if content.trim().is_empty() { return Ok(None); @@ -277,14 +277,14 @@ impl GitDriver { "--format=%at".to_string(), identifier.to_string(), ], - ); + )?; let mut output = String::new(); self.inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(&self.repo_dir)); + .execute(&command, &mut output, Some(&self.repo_dir))?; - let timestamp_str = GitUtil::parse_rev_list_output(&output, &self.inner.process); + let timestamp_str = GitUtil::parse_rev_list_output(&output, &self.inner.process)?; let timestamp: i64 = timestamp_str.trim().parse().unwrap_or(0); Ok(Some( Utc.timestamp_opt(timestamp, 0).unwrap().fixed_offset(), @@ -296,7 +296,7 @@ impl GitDriver { self.tags = Some(IndexMap::new()); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "show-ref".to_string(), @@ -305,7 +305,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { if !tag.is_empty() && let Some(caps) = preg_match( @@ -330,7 +330,7 @@ impl GitDriver { let mut branches = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "branch".to_string(), @@ -340,7 +340,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && !preg_is_match(php_regex!(r"{^ *[^/]+/HEAD }"), &branch) @@ -384,11 +384,11 @@ impl GitDriver { io.clone(), )))); let mut output = String::new(); - if process.borrow_mut().execute_args( + if process.borrow_mut().execute( &["git".to_string(), "tag".to_string()], &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } diff --git a/crates/shirabe/src/repository/vcs/github_driver.rs b/crates/shirabe/src/repository/vcs/github_driver.rs index c41cc2d0..f2b2d458 100644 --- a/crates/shirabe/src/repository/vcs/github_driver.rs +++ b/crates/shirabe/src/repository/vcs/github_driver.rs @@ -696,10 +696,7 @@ impl GitHubDriver { file, urlencode(identifier) ); - let mut resource = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let mut resource = self.get_contents(&resource_url, false)??.decode_json()?; // The GitHub contents API only returns files up to 1MB as base64 encoded files // larger files either need be fetched with a raw accept header or by using the git blob endpoint @@ -724,10 +721,7 @@ impl GitHubDriver { .and_then(|v| v.as_string()) .unwrap_or("") .to_string(); - resource = self - .get_contents(&git_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + resource = self.get_contents(&git_url, false)??.decode_json()?; } let resource_map = match resource { @@ -776,10 +770,7 @@ impl GitHubDriver { self.repository, urlencode(identifier) ); - let commit = self - .get_contents(&resource, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let commit = self.get_contents(&resource, false)??.decode_json()?; let date_str = match commit { PhpMixed::Array(m) => m @@ -812,9 +803,7 @@ impl GitHubDriver { )); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let tags_data = response.decode_json()?; if let PhpMixed::List(ref list) = tags_data { for tag in list { @@ -862,9 +851,7 @@ impl GitHubDriver { )); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let branch_data = response.decode_json()?; if let PhpMixed::List(ref list) = branch_data { for branch in list { @@ -969,40 +956,38 @@ impl GitHubDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - let response_result = self.inner.get_contents(url); + ) -> anyhow::Result>> { + let response_result = self.inner.get_contents(url)?; match response_result { - Ok(r) => Ok(r), + Ok(r) => Ok(Ok(r)), Err(e) => { let mut git_hub_util = GitHub::new( self.inner.io.clone(), self.inner.config.clone(), Some(self.inner.process.clone()), Some(self.inner.http_downloader.clone()), - ) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + )?; match e.get_code() { 401 | 404 => { // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 if !fetching_repo_data { - return Err(e); + return Ok(Err(e)); } - if git_hub_util.authorize_oauth(&self.inner.origin_url) { + if git_hub_util.authorize_oauth(&self.inner.origin_url)? { return self.inner.get_contents(url); } if !self.inner.io.is_interactive() { - self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into()))) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into())))?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } let mut scopes_issued: Vec = vec![]; @@ -1033,28 +1018,27 @@ impl GitHubDriver { "Your GitHub credentials are required to fetch private repository metadata ({})", self.inner.url )), - ); + )?; } self.inner.get_contents(url) } 403 => { if !self.inner.io.has_authentication(&self.inner.origin_url) - && git_hub_util.authorize_oauth(&self.inner.origin_url) + && git_hub_util.authorize_oauth(&self.inner.origin_url)? { return self.inner.get_contents(url); } if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into()))) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into())))?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } let rate_limited = git_hub_util @@ -1070,7 +1054,7 @@ impl GitHubDriver { true, io_interface::NORMAL, ); - return Err(e); + return Ok(Err(e)); } git_hub_util.authorize_oauth_interactively( @@ -1079,7 +1063,7 @@ impl GitHubDriver { "API limit exhausted. Enter your GitHub credentials to get a larger API limit ({})", self.inner.url )), - ); + )?; return self.inner.get_contents(url); } @@ -1099,9 +1083,9 @@ impl GitHubDriver { ); } - Err(e) + Ok(Err(e)) } - _ => Err(e), + _ => Ok(Err(e)), } } } @@ -1122,7 +1106,7 @@ impl GitHubDriver { self.repository ); - let repo_data_result = self.get_contents(&repo_data_url, true); + let repo_data_result = self.get_contents(&repo_data_url, true)?; match repo_data_result { Ok(response) => { let data = response.decode_json()?; diff --git a/crates/shirabe/src/repository/vcs/gitlab_driver.rs b/crates/shirabe/src/repository/vcs/gitlab_driver.rs index b40297da..d21a9ca1 100644 --- a/crates/shirabe/src/repository/vcs/gitlab_driver.rs +++ b/crates/shirabe/src/repository/vcs/gitlab_driver.rs @@ -396,7 +396,7 @@ impl GitLabDriver { identifier, ); - let content = match self.get_contents(&resource, false) { + let content = match self.get_contents(&resource, false)? { Ok(response) => response.get_body().map(|s| s.to_string()), Err(e) => { if e.get_code() != 404 { @@ -587,9 +587,7 @@ impl GitLabDriver { let mut references: IndexMap = IndexMap::new(); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let data = response.decode_json()?; if let PhpMixed::List(ref list) = data { @@ -646,10 +644,7 @@ impl GitLabDriver { // we need to fetch the default branch from the api let resource = self.get_api_url(); - let project = self - .get_contents(&resource, true) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let project = self.get_contents(&resource, true)??.decode_json()?; self.project = match project { PhpMixed::Array(m) => Some(m), _ => None, @@ -741,14 +736,18 @@ impl GitLabDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - let response_result = self.inner.get_contents(url); - match response_result { - Ok(response) => { + ) -> anyhow::Result>> { + // PHP body of the `try` block: a TransportException raised anywhere in here, including the + // ones thrown below to force authentication, is handled by the `catch` that follows. + let response_result = + (|this: &mut Self| -> anyhow::Result>> { + let response = match this.inner.get_contents(url)? { + Ok(response) => response, + Err(e) => return Ok(Err(e)), + }; + if fetching_repo_data { - let json = response - .decode_json() - .map_err(|e| TransportException::new(e.to_string(), 0))?; + let json = response.decode_json()?; let json_map = match json { PhpMixed::Array(ref m) => m.clone(), _ => IndexMap::new(), @@ -760,7 +759,7 @@ impl GitLabDriver { if !json_map.contains_key("default_branch") && json_map.contains_key("permissions") { - self.is_private = json_map + this.is_private = json_map .get("visibility") .and_then(|v| v.as_string()) .map(|s| s != "public") @@ -785,21 +784,20 @@ impl GitLabDriver { } if !more_than_guest_access { - self.inner.io.write_error3( - "GitLab token with Guest or Planner only access detected", - true, - io_interface::NORMAL, - ); + this.inner.io.write_error3( + "GitLab token with Guest or Planner only access detected", + true, + io_interface::NORMAL, + ); - self.attempt_clone_fallback() - .map_err(|e| TransportException::new(e.to_string(), 0))?; + this.attempt_clone_fallback()?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } } @@ -811,110 +809,108 @@ impl GitLabDriver { .and_then(|v| v.as_string()) == Some("disabled") { - return Err(Box::new(TransportException::new( + return Ok(Err(Box::new(TransportException::new( "The GitLab repository is disabled in the project".to_string(), 400, - ))); + )))); } if !empty(&json_map.get("id").cloned().unwrap_or(PhpMixed::Null)) { - self.is_private = false; + this.is_private = false; } - return Err(Box::new(TransportException::new( - "GitLab API seems to not be authenticated as it did not return a default_branch" - .to_string(), - 401, - ))); + return Ok(Err(Box::new(TransportException::new( + "GitLab API seems to not be authenticated as it did not return a default_branch" + .to_string(), + 401, + )))); } } - Ok(response) - } - Err(e) => { - let mut git_lab_util = GitLab::new( - self.inner.io.clone(), - self.inner.config.clone(), - Some(self.inner.process.clone()), - Some(self.inner.http_downloader.clone()), - ) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + Ok(Ok(response)) + })(self); - match e.get_code() { - 401 | 404 => { - // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 - if !fetching_repo_data { - return Err(e); - } + let e = match response_result? { + Ok(response) => return Ok(Ok(response)), + Err(e) => e, + }; - if git_lab_util.authorize_oauth(&self.inner.origin_url) { - return self.inner.get_contents(url); - } + let mut git_lab_util = GitLab::new( + self.inner.io.clone(), + self.inner.config.clone(), + Some(self.inner.process.clone()), + Some(self.inner.http_downloader.clone()), + )?; + + match e.get_code() { + 401 | 404 => { + // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 + if !fetching_repo_data { + return Ok(Err(e)); + } - if git_lab_util.is_oauth_expired(&self.inner.origin_url) - && git_lab_util - .authorize_oauth_refresh(&self.scheme, &self.inner.origin_url) - .map_err(|err| TransportException::new(err.to_string(), 0))? - { - return self.inner.get_contents(url); - } + if git_lab_util.authorize_oauth(&self.inner.origin_url)? { + return self.inner.get_contents(url); + } - if !self.inner.io.is_interactive() { - self.attempt_clone_fallback() - .map_err(|err| TransportException::new(err.to_string(), 0))?; + if git_lab_util.is_oauth_expired(&self.inner.origin_url) + && git_lab_util.authorize_oauth_refresh(&self.scheme, &self.inner.origin_url)? + { + return self.inner.get_contents(url); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } - self.inner.io.write_error3( - &format!( - "Failed to download {}/{}:{}", - self.namespace, - self.repository, - e.get_message() - ), - true, - io_interface::NORMAL, - ); - git_lab_util.authorize_oauth_interactively( - &self.scheme, - &self.inner.origin_url, - Some(&format!( - "Your credentials are required to fetch private repository metadata ({})", - self.inner.url - )), - ); + if !self.inner.io.is_interactive() { + self.attempt_clone_fallback()?; - self.inner.get_contents(url) - } - 403 => { - if !self.inner.io.has_authentication(&self.inner.origin_url) - && git_lab_util.authorize_oauth(&self.inner.origin_url) - { - return self.inner.get_contents(url); - } + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); + } + self.inner.io.write_error3( + &format!( + "Failed to download {}/{}:{}", + self.namespace, + self.repository, + e.get_message() + ), + true, + io_interface::NORMAL, + ); + git_lab_util.authorize_oauth_interactively( + &self.scheme, + &self.inner.origin_url, + Some(&format!( + "Your credentials are required to fetch private repository metadata ({})", + self.inner.url + )), + )?; - if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback() - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.inner.get_contents(url) + } + 403 => { + if !self.inner.io.has_authentication(&self.inner.origin_url) + && git_lab_util.authorize_oauth(&self.inner.origin_url)? + { + return self.inner.get_contents(url); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } + if !self.inner.io.is_interactive() && fetching_repo_data { + self.attempt_clone_fallback()?; - Err(e) - } - _ => Err(e), + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); } + + Ok(Err(e)) } + _ => Ok(Err(e)), } } diff --git a/crates/shirabe/src/repository/vcs/hg_driver.rs b/crates/shirabe/src/repository/vcs/hg_driver.rs index 54341070..c952eb0f 100644 --- a/crates/shirabe/src/repository/vcs/hg_driver.rs +++ b/crates/shirabe/src/repository/vcs/hg_driver.rs @@ -89,17 +89,17 @@ impl HgDriver { ); if is_dir(&self.repo_dir) - && self.inner.process.borrow_mut().execute_args( + && self.inner.process.borrow_mut().execute( ["hg", "summary"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.repo_dir), - ) == 0 + )? == 0 { - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["hg", "pull"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.repo_dir), - ) != 0 + )? != 0 { self.inner.io.write_error3(&format!("Failed to update {}, package information from this repository may be outdated ({})", self.inner.url, self.inner.process.borrow().get_error_output()), true, crate::io::NORMAL); } @@ -132,13 +132,13 @@ impl HgDriver { pub fn get_root_identifier(&mut self) -> anyhow::Result { if self.root_identifier.is_none() { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "tip", "--template", "{node}"] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; let lines = self.inner.process.borrow().split_lines(&output); self.root_identifier = lines.into_iter().next(); } @@ -183,7 +183,7 @@ impl HgDriver { self.inner .process .borrow_mut() - .execute_args(&resource, &mut content, Some(&self.repo_dir)); + .execute(&resource, &mut content, Some(&self.repo_dir))?; if content.trim().is_empty() { return Ok(None); @@ -205,7 +205,7 @@ impl HgDriver { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( [ "hg", "log", @@ -218,7 +218,7 @@ impl HgDriver { .as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; let date: DateTime = shirabe_php_shim::date_create(output.trim())?; Ok(Some(date.fixed_offset())) @@ -228,11 +228,11 @@ impl HgDriver { if self.tags.is_none() { let mut tags: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "tags"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { if !tag.is_empty() && let Some(m) = preg_match(php_regex!(r"(^([^\s]+)\s+\d+:(.*)$)"), &tag) @@ -257,11 +257,11 @@ impl HgDriver { let mut bookmarks: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "branches"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && let Some(m) = @@ -275,11 +275,11 @@ impl HgDriver { } output.clear(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "bookmarks"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && let Some(m) = @@ -323,11 +323,11 @@ impl HgDriver { let mut process = crate::util::ProcessExecutor::new(Some(io.clone())); let mut output = String::new(); - if process.execute_args( + if process.execute( ["hg", "summary"].map(|s| s.to_string()).as_ref(), &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } @@ -339,13 +339,13 @@ impl HgDriver { let mut process = crate::util::ProcessExecutor::new(Some(io)); let mut ignored = String::new(); - let exit = process.execute_args( + let exit = process.execute( ["hg", "identify", "--", url] .map(|s| s.to_string()) .as_ref(), &mut ignored, None, - ); + )?; Ok(exit == 0) } diff --git a/crates/shirabe/src/repository/vcs/perforce_driver.rs b/crates/shirabe/src/repository/vcs/perforce_driver.rs index d55d361c..6a1c1d70 100644 --- a/crates/shirabe/src/repository/vcs/perforce_driver.rs +++ b/crates/shirabe/src/repository/vcs/perforce_driver.rs @@ -191,10 +191,7 @@ impl PerforceDriver { deep: bool, ) -> anyhow::Result { if deep || preg_is_match(php_regex!(r"#\b(perforce|p4)\b#i"), url) { - return Ok(Perforce::check_server_exists( - url, - &mut ProcessExecutor::new(Some(io)), - )); + return Perforce::check_server_exists(url, &mut ProcessExecutor::new(Some(io))); } Ok(false) } diff --git a/crates/shirabe/src/repository/vcs/svn_driver.rs b/crates/shirabe/src/repository/vcs/svn_driver.rs index 0431c33b..c92c9c9f 100644 --- a/crates/shirabe/src/repository/vcs/svn_driver.rs +++ b/crates/shirabe/src/repository/vcs/svn_driver.rs @@ -451,7 +451,7 @@ impl SvnDriver { let mut process = ProcessExecutor::new(Some(io)); let mut ignored_output = String::new(); - let exit = process.execute_args( + let exit = process.execute( &[ "svn".to_string(), "info".to_string(), @@ -461,7 +461,7 @@ impl SvnDriver { ], &mut ignored_output, None, - ); + )?; if exit == 0 { // This is definitely a Subversion repository. @@ -523,7 +523,7 @@ impl SvnDriver { { Ok(o) => Ok(o), Err(e) => { - if self.util.as_mut().unwrap().binary_version().is_none() { + if self.util.as_mut().unwrap().binary_version()?.is_none() { return Err(RuntimeException::new(format!( "Failed to load {}, svn was not found, check that it is installed and in your PATH env.\n\n{}", self.inner.url, diff --git a/crates/shirabe/src/repository/vcs/vcs_driver.rs b/crates/shirabe/src/repository/vcs/vcs_driver.rs index 17616c36..90cfd2db 100644 --- a/crates/shirabe/src/repository/vcs/vcs_driver.rs +++ b/crates/shirabe/src/repository/vcs/vcs_driver.rs @@ -66,7 +66,10 @@ impl VcsDriverBase { "http" } - pub fn get_contents(&self, url: &str) -> anyhow::Result> { + pub fn get_contents( + &self, + url: &str, + ) -> anyhow::Result>> { let options_mixed = self .repo_config .get("options") @@ -76,15 +79,13 @@ impl VcsDriverBase { PhpMixed::Array(a) => a, _ => IndexMap::new(), }; - self.http_downloader - .borrow_mut() - .get(url, options) - .map_err(|e| { - Box::new(match e.catch::() { - Some(te) => te.clone(), - None => TransportException::new(e.to_string(), 0), - }) - }) + match self.http_downloader.borrow_mut().get(url, options) { + Ok(response) => Ok(Ok(response)), + Err(e) => match e.catch::() { + Some(te) => Ok(Err(Box::new(te.clone()))), + None => Err(e), + }, + } } // Helper for concrete drivers: produces the same value as the trait default @@ -301,7 +302,7 @@ pub trait VcsDriver: VcsDriverInterface { "http" } - fn get_contents(&self, url: &str) -> anyhow::Result> { + fn get_contents(&self, url: &str) -> anyhow::Result>> { let options_mixed = self .repo_config() .get("options") @@ -311,15 +312,13 @@ pub trait VcsDriver: VcsDriverInterface { PhpMixed::Array(a) => a, _ => IndexMap::new(), }; - self.http_downloader() - .borrow_mut() - .get(url, options) - .map_err(|e| { - Box::new(match e.catch::() { - Some(te) => te.clone(), - None => TransportException::new(e.to_string(), 0), - }) - }) + match self.http_downloader().borrow_mut().get(url, options) { + Ok(response) => Ok(Ok(response)), + Err(e) => match e.catch::() { + Some(te) => Ok(Err(Box::new(te.clone()))), + None => Err(e), + }, + } } fn cleanup(&self) {} -- cgit v1.3.1-4-g156e