From f4cad2123b2af0de72bda4ce039e16e74f163f4e Mon Sep 17 00:00:00 2001 From: nsfisis Date: Sat, 8 Aug 2026 22:14:12 +0900 Subject: feat(php-shim): give ported exceptions PHP's class hierarchy Ported exceptions were flat structs reached with `downcast_ref`, so Composer's `catch (\RuntimeException $e)` only matched the exact leaf type and `get_class($e)` had nothing to report. Each exception now embeds an instance of the class it extends and travels inside an `AnyThrowable`; `Catch::catch`/`catch_mut` walk that chain, and `PhpClass::php_class_name` yields the PHP FQCN. Dropping the `std::error::Error` impls from the exception types leaves `AnyThrowable` as the only route into an `anyhow::Error`, so the walk cannot be bypassed. A `no_exception_downcast` linter catches the `downcast::()` calls that would now silently answer `None`. Three sites change behavior as a result: the `TransportException` exit-code override reaches `MaxFileSizeExceededException`, the `catch (\LogicException)` in findSimilar() reaches its subclasses, and rendered exception titles carry the real class name rather than a guess. `get_class_err()` is no longer a `todo!()`, which re-enables FilesystemRepositoryTest::testCorruptedRepositoryFile. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe/src/util/http/curl_downloader.rs | 58 ++++++++++++------------- crates/shirabe/src/util/http/proxy_item.rs | 34 ++++++--------- crates/shirabe/src/util/http/proxy_manager.rs | 6 +-- crates/shirabe/src/util/http/request_proxy.rs | 9 ++-- 4 files changed, 49 insertions(+), 58 deletions(-) (limited to 'crates/shirabe/src/util/http') diff --git a/crates/shirabe/src/util/http/curl_downloader.rs b/crates/shirabe/src/util/http/curl_downloader.rs index 604dae77..6409c9d5 100644 --- a/crates/shirabe/src/util/http/curl_downloader.rs +++ b/crates/shirabe/src/util/http/curl_downloader.rs @@ -60,7 +60,7 @@ static TIMEOUT_WARNING: AtomicBool = AtomicBool::new(false); enum Decision { Retry { url: String, delay_ms: Option }, Done(Response), - Failed(TransportException), + Failed(anyhow::Error), } impl CurlDownloader { @@ -108,7 +108,7 @@ impl CurlDownloader { url: &str, mut options: IndexMap, copy_to: Option<&str>, - ) -> anyhow::Result> { + ) -> anyhow::Result> { let mut attributes: IndexMap = { let mut m = IndexMap::new(); m.insert("retryAuthFailure".to_string(), PhpMixed::Bool(true)); @@ -178,7 +178,7 @@ impl CurlDownloader { .as_ref() .map(|pm| pm.get_proxy_for_request(url)) .transpose() - .map_err(|e| anyhow::anyhow!(e.message))? + .map_err(|e| anyhow::anyhow!(e.get_message().to_string()))? .and_then(|p| p.get_status(Some(" using proxy (%s)")).ok()) .unwrap_or_default(); // `attributes.redirects == 0 && attributes.retries == 0` in PHP is always true here since @@ -206,7 +206,7 @@ impl CurlDownloader { )?; let send_options = crate::util::StreamContextFactory::init_options(¤t_url, send_options, true) - .map_err(|e| anyhow::anyhow!(e.message))?; + .map_err(|e| anyhow::anyhow!(e.get_message().to_string()))?; let send_result = self .send_once(¤t_url, &send_options, copy_to, &attributes) @@ -304,26 +304,24 @@ impl CurlDownloader { if let Some(filename) = filename { unlink_silent(format!("{}~", filename)); } - // PHP throws a MaxFileSizeExceededException (a TransportException subclass) with - // the raw "Maximum allowed download size reached..." message verbatim rather than - // wrapping it in the generic curl-error text. + // The message carries the raw "Maximum allowed download size reached..." text + // rather than the generic curl-error wrapper used below. if transport_err.is_max_file_size { return Ok(Decision::Failed( - MaxFileSizeExceededException(TransportException::new( - transport_err.message, - 0, - )) - .0, + MaxFileSizeExceededException::new(transport_err.message).into(), )); } - return Ok(Decision::Failed(TransportException::new( - format!( - "curl error while downloading {}: {}", - Url::sanitize(url.to_string()), - transport_err.message - ), - 0, - ))); + return Ok(Decision::Failed( + TransportException::new( + format!( + "curl error while downloading {}: {}", + Url::sanitize(url.to_string()), + transport_err.message + ), + 0, + ) + .into(), + )); } }; @@ -373,7 +371,7 @@ impl CurlDownloader { }); } Ok(_) => {} - Err(e) => return Ok(Decision::Failed(e)), + Err(e) => return Ok(Decision::Failed((*e).into())), } // Handle 3xx redirects, 304 Not Modified excluded. @@ -401,7 +399,7 @@ impl CurlDownloader { if let Some(filename) = filename { unlink_silent(format!("{}~", filename)); } - return Ok(Decision::Failed(e)); + return Ok(Decision::Failed((*e).into())); } } } @@ -443,7 +441,7 @@ impl CurlDownloader { e.set_headers(curl_response.inner.get_headers().clone()); e.set_status_code(Some(curl_response.inner.get_status_code())); e.set_response(curl_response.inner.get_body().map(|s| s.to_string())); - return Ok(Decision::Failed(e)); + return Ok(Decision::Failed((*e).into())); } // storeAuth on success. @@ -625,7 +623,7 @@ impl CurlDownloader { url: &str, attributes: &IndexMap, response: &CurlResponse, - ) -> anyhow::Result> { + ) -> anyhow::Result>> { let mut target_url = String::new(); if let Some(location_header) = response.inner.get_header("location") && !location_header.is_empty() @@ -682,14 +680,14 @@ impl CurlDownloader { return Ok(Ok(target_url)); } - Ok(Err(TransportException::new( + Ok(Err(Box::new(TransportException::new( format!( "The \"{}\" file could not be downloaded, got redirect without Location ({})", url, response.inner.get_status_message().unwrap_or_default() ), 0, - ))) + )))) } fn is_authenticated_retry_needed( @@ -699,7 +697,7 @@ impl CurlDownloader { filename: Option<&str>, attributes: &IndexMap, response: &CurlResponse, - ) -> anyhow::Result> { + ) -> anyhow::Result>> { let retry_auth_failure = attributes .get("retryAuthFailure") .and_then(|b| b.as_bool()) @@ -808,7 +806,7 @@ impl CurlDownloader { filename: Option<&str>, response: &CurlResponse, error_message: &str, - ) -> TransportException { + ) -> Box { if let Some(filename) = filename { unlink_silent(format!("{}~", filename)); } @@ -836,13 +834,13 @@ impl CurlDownloader { ); } - TransportException::new( + Box::new(TransportException::new( format!( "The \"{}\" file could not be downloaded ({}){}", url, error_message, details ), response.inner.get_status_code(), - ) + )) } fn method_is_get(options: &IndexMap) -> bool { diff --git a/crates/shirabe/src/util/http/proxy_item.rs b/crates/shirabe/src/util/http/proxy_item.rs index 1a0b3ee8..73948f88 100644 --- a/crates/shirabe/src/util/http/proxy_item.rs +++ b/crates/shirabe/src/util/http/proxy_item.rs @@ -20,28 +20,22 @@ impl ProxyItem { let syntax_error = format!("unsupported `{}` syntax", env_name); if strpbrk(&proxy_url, "\r\n\t").is_some() { - return Err(RuntimeException { - message: syntax_error, - code: 0, - }); + return Err(RuntimeException::new(syntax_error)); } let proxy_parsed = parse_url_all(&proxy_url); let proxy = match proxy_parsed.as_array() { None => { - return Err(RuntimeException { - message: syntax_error, - code: 0, - }); + return Err(RuntimeException::new(syntax_error)); } Some(a) => a.clone(), }; if !proxy.contains_key("host") { - return Err(RuntimeException { - message: format!("unable to find proxy host in {}", env_name), - code: 0, - }); + return Err(RuntimeException::new(format!( + "unable to find proxy host in {}", + env_name + ))); } let scheme = if proxy.contains_key("scheme") { @@ -100,16 +94,16 @@ impl ProxyItem { // but is considered valid depending on the PHP or Curl version. let port = match port { None => { - return Err(RuntimeException { - message: format!("unable to find proxy port in {}", env_name), - code: 0, - }); + return Err(RuntimeException::new(format!( + "unable to find proxy port in {}", + env_name + ))); } Some(0) => { - return Err(RuntimeException { - message: format!("port 0 is reserved in {}", env_name), - code: 0, - }); + return Err(RuntimeException::new(format!( + "port 0 is reserved in {}", + env_name + ))); } Some(p) => p, }; diff --git a/crates/shirabe/src/util/http/proxy_manager.rs b/crates/shirabe/src/util/http/proxy_manager.rs index 82e8ebc5..13f0b521 100644 --- a/crates/shirabe/src/util/http/proxy_manager.rs +++ b/crates/shirabe/src/util/http/proxy_manager.rs @@ -71,12 +71,12 @@ impl ProxyManager { pub fn get_proxy_for_request( &self, request_url: &str, - ) -> Result { + ) -> Result> { if let Some(ref error) = self.error { - return Err(TransportException::new( + return Err(Box::new(TransportException::new( format!("Unable to use a proxy: {}", error), 0, - )); + ))); } let scheme = request_url.split("://").next().unwrap_or("").to_string(); diff --git a/crates/shirabe/src/util/http/request_proxy.rs b/crates/shirabe/src/util/http/request_proxy.rs index 1262622f..78a64078 100644 --- a/crates/shirabe/src/util/http/request_proxy.rs +++ b/crates/shirabe/src/util/http/request_proxy.rs @@ -48,7 +48,7 @@ impl RequestProxy { pub fn get_curl_options( &self, ssl_options: &IndexMap, - ) -> Result, TransportException> { + ) -> Result, Box> { // PHP guards an HTTPS proxy behind `is_secure() && !supports_secure_proxy()` because // libcurl < 7.52.0 cannot speak TLS to a proxy. Shirabe always can (see // supports_secure_proxy), so the guard is dropped. @@ -90,10 +90,9 @@ impl RequestProxy { return Ok(format.replace("%s", self.status.as_deref().unwrap())); } - Err(InvalidArgumentException { - message: "String format specifier is missing".to_string(), - code: 0, - }) + Err(InvalidArgumentException::new( + "String format specifier is missing".to_string(), + )) } pub fn is_excluded_by_no_proxy(&self) -> bool { -- cgit v1.3.1-4-g156e