From 1b38bdb5a1de127ca8040f5a132d08c73ccb3c67 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 20 Jun 2026 15:37:24 +0900 Subject: refactor(console): propagate getComposer's exit(1) as ExitException Composer's Application::getComposer() calls exit(1) when a required Composer instance fails to build under areExceptionsCaught(). Replace the deep std::process::exit with a php-shim ExitException that mirrors PHP's `exit` construct: it bypasses parent::doRun()'s catch and Symfony's renderThrowable (excluded at both broad catches), so the already-written plain error message is not re-rendered and the process exits with code 1. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shirabe-php-shim/src/lib.rs | 19 +++++++++++++++++++ crates/shirabe/src/console/application.rs | 18 +++++++++++++++++- crates/shirabe/tests/cli.rs | 10 ---------- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/crates/shirabe-php-shim/src/lib.rs b/crates/shirabe-php-shim/src/lib.rs index 4f7b4d2..949c5e2 100644 --- a/crates/shirabe-php-shim/src/lib.rs +++ b/crates/shirabe-php-shim/src/lib.rs @@ -3150,6 +3150,25 @@ pub fn exit(status: i64) -> ! { std::process::exit(status as i32); } +/// Models PHP's `exit`/`die` language construct propagated as a recoverable error so the actual +/// process termination happens at a single top-level site instead of deep in the call stack. +/// +/// Like PHP's `exit`, this must NOT be caught by ported `try`/`catch` blocks: any broad catch on +/// the propagation path has to re-raise it untouched, and only the outermost handler converts it +/// into the process exit code. +#[derive(Debug)] +pub struct ExitException { + pub code: i64, +} + +impl std::fmt::Display for ExitException { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "exit({})", self.code) + } +} + +impl std::error::Error for ExitException {} + pub fn is_resource_value(_resource: &PhpResource) -> bool { true } diff --git a/crates/shirabe/src/console/application.rs b/crates/shirabe/src/console/application.rs index 6622a2d..9c3ef01 100644 --- a/crates/shirabe/src/console/application.rs +++ b/crates/shirabe/src/console/application.rs @@ -953,6 +953,13 @@ impl Application { let outcome = match result_outcome { Ok(r) => Ok(r), Err(e) => { + // PHP's `exit` bypasses parent::doRun()'s catch entirely; re-raise it untouched so + // the GitHub Actions annotation and error hints below are skipped. + if e.downcast_ref::() + .is_some() + { + return Err(e); + } if let Some(see) = e.downcast_ref::() { if application.borrow().get_disable_plugins_by_default() && application.borrow().is_running_as_root() @@ -1203,7 +1210,10 @@ impl Application { if required { self.io.write_error(&e.to_string()); if self.are_exceptions_caught() { - std::process::exit(1); + // PHP calls `exit(1)` here, terminating before parent::run() can + // re-render the exception. Propagate it as an ExitException so the + // top-level handler turns it into exit code 1 without re-rendering. + return Err(shirabe_php_shim::ExitException { code: 1 }.into()); } return Err(e); } @@ -1471,6 +1481,12 @@ impl Application { let exit_code = match result { Ok(exit_code) => exit_code, Err(e) => { + // PHP's `exit` bypasses Symfony's try/catch: it never renders and forces the exit + // code it carries. The message, if any, has already been written at the exit site. + if let Some(exit) = e.downcast_ref::() { + return Ok(exit.code as i32); + } + if !application.borrow().catch_exceptions { return Err(e); } diff --git a/crates/shirabe/tests/cli.rs b/crates/shirabe/tests/cli.rs index 9fd6470..5fd3980 100644 --- a/crates/shirabe/tests/cli.rs +++ b/crates/shirabe/tests/cli.rs @@ -125,12 +125,10 @@ run_no_panic_tests! { run_about => "about", #[ignore = "currently panics"] run_archive => "archive", - #[ignore = "currently panics"] run_audit => "audit", #[ignore = "currently panics"] run_browse => "browse", run_bump => "bump", - #[ignore = "currently panics"] run_check_platform_reqs => "check-platform-reqs", #[ignore = "currently panics"] run_clear_cache => "clear-cache", @@ -142,25 +140,19 @@ run_no_panic_tests! { run_depends => "depends", #[ignore = "currently panics"] run_diagnose => "diagnose", - #[ignore = "currently panics"] run_dump_autoload => "dump-autoload", - #[ignore = "currently panics"] run_exec => "exec", - #[ignore = "currently panics"] run_fund => "fund", #[ignore = "currently panics"] run_global => "global", #[ignore = "currently panics"] run_init => "init", - #[ignore = "currently panics"] run_install => "install", - #[ignore = "currently panics"] run_licenses => "licenses", #[ignore = "currently panics"] run_outdated => "outdated", #[ignore = "currently panics"] run_prohibits => "prohibits", - #[ignore = "currently panics"] run_reinstall => "reinstall", #[ignore = "currently panics"] run_remove => "remove", @@ -175,9 +167,7 @@ run_no_panic_tests! { run_self_update => "self-update", #[ignore = "currently panics"] run_show => "show", - #[ignore = "currently panics"] run_status => "status", - #[ignore = "currently panics"] run_suggests => "suggests", #[ignore = "currently panics"] run_update => "update", -- cgit v1.3.1