From 6a7e64d04a8b0ad932169df43e2d93efe8ceedf8 Mon Sep 17 00:00:00 2001
From: nsfisis <nsfisis@gmail.com>
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<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>
---
 crates/shirabe/src/repository/vcs/vcs_driver.rs | 39 ++++++++++++-------------
 1 file changed, 19 insertions(+), 20 deletions(-)

(limited to 'crates/shirabe/src/repository/vcs/vcs_driver.rs')

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) {}
-- 
cgit v1.3.1-4-g156e