diff options
| author | nsfisis <nsfisis@gmail.com> | 2026-08-30 23:57:25 +0900 |
|---|---|---|
| committer | nsfisis <nsfisis@gmail.com> | 2026-08-30 23:57:25 +0900 |
| commit | a38ed045e9981664d90c05ab43ab4ab6026eb31b (patch) | |
| tree | 537dbd660882e9af7eec9ddcb7abee4c8aae6e1a /crates/shirabe | |
| parent | d3bc3354c9705dfc6dc5e9b9adb5eb64d41e4c49 (diff) | |
| download | php-shirabe-a38ed045e9981664d90c05ab43ab4ab6026eb31b.tar.gz php-shirabe-a38ed045e9981664d90c05ab43ab4ab6026eb31b.tar.zst php-shirabe-a38ed045e9981664d90c05ab43ab4ab6026eb31b.zip | |
feat(plugin): carry an exception's class and state across the boundary
A Rust-side failure reached plugin code as a RuntimeException whose message
carried the name of the call that failed, so `catch (TransportException $e)`
never matched and the status code the plugin branches on was gone.
The Throw frame now names the class the exception was thrown as and carries
the state that class declares beyond message and code.
\Shirabe\MaterializedThrowable rebuilds it in the child: `new $class($message,
$code)` for a class whose constructor has \Exception's shape, then the
properties by reflection. A class the child cannot build that way keeps the
RuntimeException shape.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Diffstat (limited to 'crates/shirabe')
8 files changed, 297 insertions, 29 deletions
diff --git a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs index 7b620e5b..af629190 100644 --- a/crates/shirabe/src/event_dispatcher/event_dispatcher.rs +++ b/crates/shirabe/src/event_dispatcher/event_dispatcher.rs @@ -1840,6 +1840,7 @@ fn runtime_throw(message: String) -> PhpThrow { exception_class: "RuntimeException".to_string(), message, code: 0, + properties: Box::new(IndexMap::new()), } } diff --git a/crates/shirabe/src/plugin/php_plugin_proxy.rs b/crates/shirabe/src/plugin/php_plugin_proxy.rs index 84bf195f..a1233391 100644 --- a/crates/shirabe/src/plugin/php_plugin_proxy.rs +++ b/crates/shirabe/src/plugin/php_plugin_proxy.rs @@ -9,6 +9,7 @@ use crate::autoload::ClassLoader; use crate::command::BaseCommand; use crate::composer::ComposerHandle; use crate::dependency_resolver::operation::AnyOperation; +use crate::downloader::TransportException; use crate::event_dispatcher::event_dispatcher::dispatch_event_method; use crate::event_dispatcher::{ EventInterface, EventSubscriberInterface, SubscribedEventEntry, unwrap_php_result, @@ -32,7 +33,7 @@ use shirabe_php_rpc::{ PhpObjHandle, PhpThrow, PluginValue, RustMethodDispatcher, RustObjHandle, call_function_with_dispatcher, call_php_method, new_object, release_php_handle, }; -use shirabe_php_shim::PhpMixed; +use shirabe_php_shim::{AnyThrowable, Catch as _, PhpClass as _, PhpMixed}; use shirabe_symfony_console::command::Command; use shirabe_symfony_console::input::InputInterface; use shirabe_symfony_console::input::InputValue; @@ -640,7 +641,7 @@ fn dispatch_plugin_method( })?; let capabilities = capable .get_capabilities() - .map_err(|error| runtime_throw(format!("getCapabilities failed: {error:#}")))?; + .map_err(|error| error_throw("getCapabilities failed", &error))?; Ok(capabilities.to_plugin_value()) } // TODO(plugin): the lifecycle methods would have to turn the `$composer`/`$io` stubs the @@ -718,14 +719,14 @@ fn dispatch_config_method( let value = config .borrow() .get_with_flags(&key(0)?, flags(1)?) - .map_err(|error| runtime_throw(format!("get failed over RPC: {error}")))?; + .map_err(|error| error_throw("get failed over RPC", &error))?; Ok(value.to_plugin_value()) } "all" => { let all = config .borrow_mut() .all(flags(0)?) - .map_err(|error| runtime_throw(format!("all failed over RPC: {error}")))?; + .map_err(|error| error_throw("all failed over RPC", &error))?; Ok(all.to_plugin_value()) } "raw" => Ok(config.borrow().raw().to_plugin_value()), @@ -831,11 +832,7 @@ fn dispatch_download_manager_method( .await } } - .map_err(|error| { - // TODO(plugin): the original exception class is collapsed to - // RuntimeException on this side of the boundary. - runtime_throw(format!("{method_name} failed over RPC: {error:#}")) - }) + .map_err(|error| error_throw(&format!("{method_name} failed over RPC"), &error)) })?; resolved_promise(resolved.to_plugin_value()) } @@ -870,9 +867,7 @@ fn dispatch_filesystem_method( args: &[PluginValue], ) -> Result<PluginValue, PhpThrow> { let string_arg = |position: usize| arg::<String>(method_name, args, position); - // TODO(plugin): the exception class the real method throws (RuntimeException, IOException, - // LogicException) is collapsed to RuntimeException on this side of the boundary. - let failed = |error: anyhow::Error| runtime_throw(format!("{method_name} failed: {error:#}")); + let failed = |error: anyhow::Error| error_throw(&format!("{method_name} failed"), &error); match method_name { "remove" => Ok(fs .borrow_mut() @@ -949,6 +944,7 @@ fn dispatch_filesystem_method( exception_class: "InvalidArgumentException".to_string(), message: format!("$from ({from}) and $to ({to}) must be absolute paths."), code: 0, + properties: Box::new(IndexMap::new()), }); } Ok(if method_name == "findShortestPath" { @@ -1029,7 +1025,7 @@ fn dispatch_process_executor_method( args: &[PluginValue], out_params: &mut IndexMap<u32, PluginValue>, ) -> Result<PluginValue, PhpThrow> { - let failed = |error: anyhow::Error| runtime_throw(format!("{method_name} failed: {error:#}")); + let failed = |error: anyhow::Error| error_throw(&format!("{method_name} failed"), &error); let cwd_arg = |position: usize| -> Result<Option<String>, PhpThrow> { match args.get(position) { None | Some(PluginValue::Null) => Ok(None), @@ -1334,11 +1330,9 @@ fn dispatch_repository_method( match method_name { "hasPackage" => { let package = arg::<PackageInterfaceHandle>(method_name, args, 0)?; - let has = repository.has_package(package).map_err(|error| { - // TODO(plugin): the original exception class is collapsed to RuntimeException - // on this side of the boundary. - runtime_throw(format!("hasPackage failed over RPC: {error}")) - })?; + let has = repository + .has_package(package) + .map_err(|error| error_throw("hasPackage failed over RPC", &error))?; Ok(has.to_plugin_value()) } "addPackage" | "removePackage" => { @@ -1356,19 +1350,15 @@ fn dispatch_repository_method( } else { writable.remove_package(package) }; - outcome.map_err(|error| { - // TODO(plugin): the original exception class is collapsed to RuntimeException - // on this side of the boundary. - runtime_throw(format!("{method_name} failed over RPC: {error}")) - })?; + outcome + .map_err(|error| error_throw(&format!("{method_name} failed over RPC"), &error))?; Ok(PluginValue::Null) } "getPackages" => { - let packages = repository.borrow_mut().get_packages().map_err(|error| { - // TODO(plugin): the original exception class is collapsed to RuntimeException - // on this side of the boundary. - runtime_throw(format!("getPackages failed over RPC: {error}")) - })?; + let packages = repository + .borrow_mut() + .get_packages() + .map_err(|error| error_throw("getPackages failed over RPC", &error))?; let mut items = Vec::with_capacity(packages.len()); for package in packages { items.push(package_handle_value(package.as_rc())); @@ -1932,7 +1922,7 @@ fn dispatch_package_method( .borrow_mut() .as_package_interface_mut() .set_repository(repository) - .map_err(|error| runtime_throw(format!("setRepository failed: {error}")))?; + .map_err(|error| error_throw("setRepository failed", &error))?; return Ok(PluginValue::Null); } "setTransportOptions" => { @@ -2429,7 +2419,70 @@ fn runtime_throw(message: String) -> PhpThrow { exception_class: "RuntimeException".to_string(), message, code: 0, + properties: Box::new(IndexMap::new()), + } +} + +/// The `Throw` a failed Rust-side call crosses the boundary as. An error carrying a ported PHP +/// exception keeps that exception's class, code and declared state, so a plugin catches what it +/// would catch under Composer; one carrying no exception keeps the `RuntimeException` shape, +/// with `context` naming the call that failed. +fn error_throw(context: &str, error: &anyhow::Error) -> PhpThrow { + let Some(exception) = AnyThrowable::of(error.as_ref()) else { + return runtime_throw(format!("{context}: {error:#}")); + }; + PhpThrow { + exception_class: exception.php_class_name(), + message: exception.get_message().to_string(), + code: exception.get_code(), + properties: Box::new(throwable_properties(error)), + } +} + +/// The state a ported exception carries beyond `message` and `code`, keyed by the property names +/// its PHP class declares. The child revives them onto the instance it rebuilds. +/// +/// TODO(plugin): only `TransportException` is projected. Every other exception with state of its +/// own (`InvalidPackageException`, `JsonValidationException`, `CommandNotFoundException`, +/// `ProcessSignaledException`, `SolverProblemsException`) crosses with message and code alone. +fn throwable_properties(error: &anyhow::Error) -> IndexMap<String, PluginValue> { + let mut properties = IndexMap::new(); + if let Some(exception) = error.catch::<TransportException>() { + properties.insert( + "headers".to_string(), + match exception.get_headers() { + Some(headers) => { + PluginValue::List(headers.iter().cloned().map(PluginValue::string).collect()) + } + None => PluginValue::Null, + }, + ); + properties.insert( + "response".to_string(), + match exception.get_response() { + Some(response) => PluginValue::string(response), + None => PluginValue::Null, + }, + ); + properties.insert( + "statusCode".to_string(), + match exception.get_status_code() { + Some(status_code) => PluginValue::Int(status_code), + None => PluginValue::Null, + }, + ); + properties.insert( + "responseInfo".to_string(), + PluginValue::List( + exception + .get_response_info() + .iter() + .map(PluginValue::from_php_mixed) + .collect(), + ), + ); } + properties } /// `PluginInterface` adapter for a plugin entity living in the PHP child process: every diff --git a/crates/shirabe/src/plugin/php_plugin_value.rs b/crates/shirabe/src/plugin/php_plugin_value.rs index a61d4873..cb9b990e 100644 --- a/crates/shirabe/src/plugin/php_plugin_value.rs +++ b/crates/shirabe/src/plugin/php_plugin_value.rs @@ -44,6 +44,7 @@ fn throw(message: String) -> PhpThrow { exception_class: "RuntimeException".to_string(), message, code: 0, + properties: Box::new(indexmap::IndexMap::new()), } } diff --git a/crates/shirabe/tests/plugin/e2e_exception_test.rs b/crates/shirabe/tests/plugin/e2e_exception_test.rs new file mode 100644 index 00000000..ddbd1cf8 --- /dev/null +++ b/crates/shirabe/tests/plugin/e2e_exception_test.rs @@ -0,0 +1,79 @@ +//! Exception fidelity E2E check: upstream Composer and Shirabe each install a fixture project +//! whose plugin catches the exceptions a Composer service raises at it and writes what it saw to +//! a trace file. Upstream has no test that inspects an exception from plugin code, so the whole +//! fixture is Shirabe-authored (`fixtures/e2e-exception/`) and nothing has to be fetched; the +//! test skips only while the PHP runtime or the Composer checkout is missing. + +use crate::e2e_extension_installer_test::{copy_dir, upstream_composer_bin}; +use crate::php_worker::{lock_php_worker, php_runtime_available}; +use std::path::{Path, PathBuf}; +use tempfile::TempDir; + +fn fixture_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/plugin/fixtures/e2e-exception") +} + +struct Run { + exit_code: i32, + trace: String, +} + +/// Runs `install` in a fresh copy of the fixture and returns the exit code with the plugin's trace. +fn install(program: &str, prefix_args: &[&str]) -> Run { + let work = TempDir::new().unwrap(); + copy_dir(&fixture_dir(), work.path()); + let project = work.path().join("project"); + let output = std::process::Command::new(program) + .args(prefix_args) + .arg("install") + .current_dir(&project) + .env("COMPOSER_HOME", work.path().join("home")) + .env("COMPOSER_CACHE_DIR", work.path().join("cache")) + .env("COMPOSER_NO_INTERACTION", "1") + .env("COLUMNS", "120") + .env("LINES", "30") + .output() + .unwrap(); + Run { + exit_code: output.status.code().unwrap_or(-1), + trace: std::fs::read_to_string(project.join("exception-trace.txt")).unwrap_or_default(), + } +} + +#[test] +fn test_exceptions_reach_a_plugin_as_their_own_class() { + if !php_runtime_available() { + return; + } + let Some(composer_bin) = upstream_composer_bin() else { + return; + }; + let _worker = lock_php_worker(); + let composer_bin = composer_bin.to_str().unwrap().to_string(); + + let upstream = install("php", &[composer_bin.as_str()]); + let shirabe = install(env!("CARGO_BIN_EXE_shirabe"), &[]); + + assert_eq!(0, upstream.exit_code, "upstream install must succeed"); + assert_eq!(upstream.exit_code, shirabe.exit_code); + assert_eq!(upstream.trace, shirabe.trace); + + // Pinned as well as compared, so a run where neither side wrote a trace cannot pass. The + // class names and the two hierarchy answers are the evidence that the exception crossed as + // itself rather than as one collapsed shape. + assert_eq!( + "\ +event=post-update-cmd +findShortestPath class=\"InvalidArgumentException\" \ +message=\"$from (relative) and $to (\\/absolute) must be absolute paths.\" code=0 \ +logic=true runtime=false +findShortestPathCode class=\"InvalidArgumentException\" \ +message=\"$from (\\/absolute) and $to (relative) must be absolute paths.\" code=0 \ +logic=true runtime=false +ensureDirectoryExists class=\"RuntimeException\" \ +message=\"not-a-directory exists and is not a directory.\" code=0 logic=false runtime=true +catch-clause=InvalidArgumentException +", + upstream.trace + ); +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/composer.json b/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/composer.json new file mode 100644 index 00000000..4bc436ec --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/composer.json @@ -0,0 +1,17 @@ +{ + "name": "shirabe-test/exception-probe", + "version": "1.0.0", + "type": "composer-plugin", + "description": "Fixture plugin recording the exceptions a Composer service raises at it.", + "autoload": { + "psr-4": { + "ShirabeTest\\Exception\\": "src/" + } + }, + "require": { + "composer-plugin-api": "^2.0" + }, + "extra": { + "class": "ShirabeTest\\Exception\\Plugin" + } +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/src/Plugin.php b/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/src/Plugin.php new file mode 100644 index 00000000..547f4fbb --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-exception/plugin/src/Plugin.php @@ -0,0 +1,92 @@ +<?php + +namespace ShirabeTest\Exception; + +use Composer\Composer; +use Composer\EventDispatcher\EventSubscriberInterface; +use Composer\IO\IOInterface; +use Composer\Plugin\PluginInterface; +use Composer\Script\Event; +use Composer\Script\ScriptEvents; +use Composer\Util\Filesystem; + +/** + * Records what a plugin sees when a Composer service raises an exception at it: the class it was + * thrown as, its message and code, whether it is still an instance of the parent classes the real + * hierarchy gives it, and whether a `catch` naming that class matches. Composer plugins branch on + * the exception class rather than on its message, so the whole surface is compared line by line + * between implementations. + */ +class Plugin implements PluginInterface, EventSubscriberInterface +{ + public function activate(Composer $composer, IOInterface $io): void + { + } + + public function deactivate(Composer $composer, IOInterface $io): void + { + } + + public function uninstall(Composer $composer, IOInterface $io): void + { + } + + public static function getSubscribedEvents() + { + // Whether an install resolves or replays a lock file decides which of the two fires, so + // both are subscribed and the trace records the one that ran. + return [ + ScriptEvents::POST_INSTALL_CMD => 'onPostCommand', + ScriptEvents::POST_UPDATE_CMD => 'onPostCommand', + ]; + } + + public function onPostCommand(Event $event): void + { + $filesystem = new Filesystem(); + $lines = ['event=' . $event->getName()]; + + $lines[] = 'findShortestPath ' . $this->describe(static function () use ($filesystem): void { + $filesystem->findShortestPath('relative', '/absolute'); + }); + $lines[] = 'findShortestPathCode ' . $this->describe(static function () use ($filesystem): void { + $filesystem->findShortestPathCode('/absolute', 'relative'); + }); + + // A different class through the same seam, so the trace shows the class travelling rather + // than every failure arriving under one name. + file_put_contents('not-a-directory', ''); + $lines[] = 'ensureDirectoryExists ' . $this->describe(static function () use ($filesystem): void { + $filesystem->ensureDirectoryExists('not-a-directory'); + }); + + // get_class() answers for the object; a catch clause answers for the class hierarchy the + // child holds, which is what plugin code is actually written against. + try { + $filesystem->findShortestPath('relative', '/absolute'); + $caught = 'nothing-thrown'; + } catch (\InvalidArgumentException $e) { + $caught = 'InvalidArgumentException'; + } catch (\Throwable $e) { + $caught = 'unmatched:' . \get_class($e); + } + $lines[] = 'catch-clause=' . $caught; + + file_put_contents('exception-trace.txt', implode("\n", $lines) . "\n"); + } + + private function describe(callable $call): string + { + try { + $call(); + + return 'class=none'; + } catch (\Throwable $e) { + return 'class=' . json_encode(\get_class($e)) + . ' message=' . json_encode($e->getMessage()) + . ' code=' . json_encode($e->getCode()) + . ' logic=' . json_encode($e instanceof \LogicException) + . ' runtime=' . json_encode($e instanceof \RuntimeException); + } + } +} diff --git a/crates/shirabe/tests/plugin/fixtures/e2e-exception/project/composer.json b/crates/shirabe/tests/plugin/fixtures/e2e-exception/project/composer.json new file mode 100644 index 00000000..da06cc93 --- /dev/null +++ b/crates/shirabe/tests/plugin/fixtures/e2e-exception/project/composer.json @@ -0,0 +1,24 @@ +{ + "name": "shirabe/e2e-exception", + "description": "E2E fixture project: record the exceptions a plugin catches from Composer services.", + "repositories": [ + { + "type": "path", + "url": "../plugin", + "options": { + "symlink": false + } + }, + { + "packagist.org": false + } + ], + "require": { + "shirabe-test/exception-probe": "1.0.0" + }, + "config": { + "allow-plugins": { + "shirabe-test/exception-probe": true + } + } +} diff --git a/crates/shirabe/tests/plugin/main.rs b/crates/shirabe/tests/plugin/main.rs index 35f5beb5..3ef3f647 100644 --- a/crates/shirabe/tests/plugin/main.rs +++ b/crates/shirabe/tests/plugin/main.rs @@ -7,6 +7,7 @@ mod php_worker; mod alias_package_test; mod e2e_command_provider_test; +mod e2e_exception_test; mod e2e_extension_installer_test; mod e2e_installer_test; mod e2e_installers_test; |
