aboutsummaryrefslogtreecommitdiffhomepage
path: root/crates/shirabe/src/repository/vcs/vcs_driver.rs
diff options
context:
space:
mode:
authornsfisis <nsfisis@gmail.com>2026-08-18 07:18:07 +0900
committernsfisis <nsfisis@gmail.com>2026-08-18 07:18:07 +0900
commit6a7e64d04a8b0ad932169df43e2d93efe8ceedf8 (patch)
treef74bca82b226fde11c820ba3988e094a49073940 /crates/shirabe/src/repository/vcs/vcs_driver.rs
parentbed0cd33ca32b95aed9d65891f97649a6f9d0069 (diff)
downloadphp-shirabe-6a7e64d04a8b0ad932169df43e2d93efe8ceedf8.tar.gz
php-shirabe-6a7e64d04a8b0ad932169df43e2d93efe8ceedf8.tar.zst
php-shirabe-6a7e64d04a8b0ad932169df43e2d93efe8ceedf8.zip
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<Response, Box<TransportException>>`, 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<Result<Response, Box<TransportException>>>`: 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<Box<$ty>> for anyhow::Error` so a caught exception can be re-propagated with `?`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe/src/repository/vcs/vcs_driver.rs')
-rw-r--r--crates/shirabe/src/repository/vcs/vcs_driver.rs39
1 files changed, 19 insertions, 20 deletions
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<Response, Box<TransportException>> {
+ pub fn get_contents(
+ &self,
+ url: &str,
+ ) -> anyhow::Result<Result<Response, Box<TransportException>>> {
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::<TransportException>() {
- 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::<TransportException>() {
+ 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<Response, Box<TransportException>> {
+ fn get_contents(&self, url: &str) -> anyhow::Result<Result<Response, Box<TransportException>>> {
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::<TransportException>() {
- 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::<TransportException>() {
+ Some(te) => Ok(Err(Box::new(te.clone()))),
+ None => Err(e),
+ },
+ }
}
fn cleanup(&self) {}