fix!: keep Rust panics on the Rust side of the FFI boundary

Cargo.toml

@@ -71,6 +71,8 @@ members = [
     "crates/php-build",
     "crates/introspection",
     "tests",
+    "tests/broken-module",
+    "tests/broken-minit",
 ]
 
 [package.metadata.docs.rs]

allowed_bindings.rs

@@ -69,6 +69,7 @@ bind! {
     zend_ce_arithmetic_error,
     zend_ce_compile_error,
     zend_ce_division_by_zero_error,
+    zend_ce_error,
     zend_ce_error_exception,
     zend_ce_exception,
     zend_ce_parse_error,

crates/macros/src/function.rs

@@ -267,7 +267,7 @@ impl<'a> Function<'a> {
                 // The method returns &Self or &mut Self, use `this` directly
                 if let Err(e) = this.set_zval(retval, false) {
                     let e: ::ext_php_rs::exception::PhpException = e.into();
-                    e.throw().expect("Failed to throw PHP exception.");
+                    e.throw();
                 }
             }
         } else {
@@ -281,7 +281,7 @@ impl<'a> Function<'a> {
 
                 if let Err(e) = result.set_zval(retval, false) {
                     let e: ::ext_php_rs::exception::PhpException = e.into();
-                    e.throw().expect("Failed to throw PHP exception.");
+                    e.throw();
                 }
             }
         };
@@ -294,19 +294,9 @@ impl<'a> Function<'a> {
                         ex: &mut ::ext_php_rs::zend::ExecuteData,
                         retval: &mut ::ext_php_rs::types::Zval,
                     ) {
-                        use ::ext_php_rs::zend::try_catch;
-                        use ::std::panic::AssertUnwindSafe;
-
-                        // Wrap the handler body with try_catch to ensure Rust destructors
-                        // are called if a bailout occurs (issue #537)
-                        let catch_result = try_catch(AssertUnwindSafe(|| {
+                        ::ext_php_rs::zend::run_handler(::std::panic::AssertUnwindSafe(|| {
                             #handler_body
                         }));
-
-                        // try_catch already dropped the BailoutGuards of this frame; re-trigger the bailout
-                        if catch_result.is_err() {
-                            unsafe { ::ext_php_rs::zend::bailout(); }
-                        }
                     }
                 }
                 handler
@@ -400,7 +390,7 @@ impl<'a> Function<'a> {
         let arg_accessors = self.args.typed.iter().map(|arg| {
             arg.accessor(|e| {
                 quote! {
-                    #e.throw().expect("Failed to throw PHP exception.");
+                    #e.throw();
                     return;
                 }
             })
@@ -434,8 +424,7 @@ impl<'a> Function<'a> {
                             Some(this) => this,
                             None => {
                                 ::ext_php_rs::exception::PhpException::from_message("Failed to retrieve reference to `$this`".into())
-                                    .throw()
-                                    .unwrap();
+                                    .throw();
                                 return;
                             }
                         };
@@ -561,7 +550,7 @@ impl<'a> Function<'a> {
                     None => {
                         ::ext_php_rs::exception::PhpException::from_message(
                             concat!("Invalid value given for argument `", stringify!(#name), "`.").into()
-                        ).throw().expect("Failed to throw PHP exception.");
+                        ).throw();
                         return;
                     }
                 }
@@ -571,7 +560,7 @@ impl<'a> Function<'a> {
         let throw_invalid = quote! {
             ::ext_php_rs::exception::PhpException::from_message(
                 concat!("Invalid value given for argument `", stringify!(#name), "`.").into()
-            ).throw().expect("Failed to throw PHP exception.");
+            ).throw();
             return;
         };
 
@@ -580,7 +569,7 @@ impl<'a> Function<'a> {
                 concat!("Argument `$", stringify!(#name), "` must not be null").into(),
                 0,
                 ::ext_php_rs::zend::ce::type_error(),
-            ).throw().expect("Failed to throw PHP exception.");
+            ).throw();
             return;
         };
 
@@ -671,7 +660,7 @@ impl<'a> Function<'a> {
         let this_error = quote! {
             ::ext_php_rs::exception::PhpException::from_message(
                 "Failed to retrieve reference to `$this`".into()
-            ).throw().unwrap();
+            ).throw();
             return;
         };
 
@@ -731,7 +720,7 @@ impl<'a> Function<'a> {
 
                 if let Err(e) = __this.set_zval(retval, false) {
                     let e: ::ext_php_rs::exception::PhpException = e.into();
-                    e.throw().expect("Failed to throw PHP exception.");
+                    e.throw();
                 }
             }
         } else {
@@ -745,7 +734,7 @@ impl<'a> Function<'a> {
 
                 if let Err(e) = __result.set_zval(retval, false) {
                     let e: ::ext_php_rs::exception::PhpException = e.into();
-                    e.throw().expect("Failed to throw PHP exception.");
+                    e.throw();
                 }
             }
         }
@@ -823,32 +812,17 @@ impl<'a> Function<'a> {
             ::ext_php_rs::class::ConstructorMeta {
                 constructor: {
                     fn inner(ex: &mut ::ext_php_rs::zend::ExecuteData) -> ::ext_php_rs::class::ConstructorResult<#class> {
-                        use ::ext_php_rs::zend::try_catch;
-                        use ::std::panic::AssertUnwindSafe;
-
-                        // Wrap the constructor body with try_catch to ensure Rust destructors
-                        // are called if a bailout occurs (issue #537)
-                        let catch_result = try_catch(AssertUnwindSafe(|| {
-                            #(#arg_declarations)*
-                            let parse = ex.parser()
-                                #(.arg(&mut #required_arg_names))*
-                                .not_required()
-                                #(.arg(&mut #not_required_arg_names))*
-                                .parse();
-                            if parse.is_err() {
-                                return ::ext_php_rs::class::ConstructorResult::ArgError;
-                            }
-                            #(#variadic_bindings)*
-                            #class::#ident(#({#arg_accessors}),*).into()
-                        }));
-
-                        // try_catch already dropped the BailoutGuards of this frame; re-trigger the bailout
-                        match catch_result {
-                            Ok(result) => result,
-                            Err(_) => {
-                                unsafe { ::ext_php_rs::zend::bailout() }
-                            }
+                        #(#arg_declarations)*
+                        let parse = ex.parser()
+                            #(.arg(&mut #required_arg_names))*
+                            .not_required()
+                            #(.arg(&mut #not_required_arg_names))*
+                            .parse();
+                        if parse.is_err() {
+                            return ::ext_php_rs::class::ConstructorResult::ArgError;
                         }
+                        #(#variadic_bindings)*
+                        #class::#ident(#({#arg_accessors}),*).into()
                     }
                     inner
                 },

crates/macros/src/impl_interface.rs

@@ -232,8 +232,7 @@ fn generate_method_builder(
                         None => {
                             let msg = format!("Invalid value for argument `{}`", #php_name);
                             ::ext_php_rs::exception::PhpException::from_message(msg.into())
-                                .throw()
-                                .expect("Failed to throw PHP exception.");
+                                .throw();
                             return;
                         }
                     };
@@ -307,7 +306,7 @@ fn generate_method_builder(
         quote! {
             if let Err(e) = result.set_zval(retval, false) {
                 let e: ::ext_php_rs::exception::PhpException = e.into();
-                e.throw().expect("Failed to throw PHP exception.");
+                e.throw();
             }
         }
     };
@@ -354,8 +353,7 @@ fn generate_method_builder(
                 Some(this) => this,
                 None => {
                     ::ext_php_rs::exception::PhpException::from_message("Failed to get $this".into())
-                        .throw()
-                        .expect("Failed to throw PHP exception.");
+                        .throw();
                     return;
                 }
             };
@@ -380,8 +378,7 @@ fn generate_method_builder(
                 Some(this) => this,
                 None => {
                     ::ext_php_rs::exception::PhpException::from_message("Failed to get $this".into())
-                        .throw()
-                        .expect("Failed to throw PHP exception.");
+                        .throw();
                     return;
                 }
             };
@@ -411,18 +408,10 @@ fn generate_method_builder(
                         retval: &mut ::ext_php_rs::types::Zval,
                     ) {
                         use ::ext_php_rs::convert::IntoZval;
-                        use ::ext_php_rs::zend::try_catch;
-                        use ::std::panic::AssertUnwindSafe;
 
-                        let catch_result = try_catch(AssertUnwindSafe(|| {
+                        ::ext_php_rs::zend::run_handler(::std::panic::AssertUnwindSafe(|| {
                             #handler_body
                         }));
-
-                        if catch_result.is_err() {
-                            unsafe {
-                                ::ext_php_rs::zend::bailout();
-                            }
-                        }
                     }
                 }
                 handler

crates/macros/src/lib.rs

@@ -1705,6 +1705,25 @@ fn php_const_internal(args: TokenStream2, input: TokenStream2) -> TokenStream2 {
 /// Classes and constants are not registered with PHP in the `get_module`
 /// function. These are registered inside the extension startup function.
 ///
+/// ## Startup failures
+///
+/// If a constant, an interface, a class or an enum cannot be registered, the
+/// generated startup function logs the cause as an `E_CORE_WARNING` and returns
+/// `FAILURE`. PHP then reports `Unable to start <extension> module`. At engine
+/// startup PHP exits. From `dl()` the request fails. A panic during
+/// registration gets the same treatment. The process does not abort in either
+/// case.
+///
+/// `get_module` cannot report a failure, because PHP reads the returned entry
+/// without a check. If the `ModuleBuilder` cannot become a module entry, for
+/// example because a function name, an argument name or the module name
+/// contains a NUL byte, the macro returns a placeholder entry with the name of
+/// the crate. The startup function of that entry logs the build error and fails
+/// as described above.
+///
+/// The `startup` function that you name in `#[php_module(startup = ...)]` is
+/// your own `extern "C"` function. A panic inside it is not caught.
+///
 /// ## Usage
 ///
 /// ```rust,no_run,ignore
@@ -2235,6 +2254,9 @@ fn php_impl_interface_internal(args: TokenStream2, input: TokenStream2) -> Token
 /// * The actual function call failed internally.
 /// * The output [`Zval`] could not be parsed into the output type.
 ///
+/// Inside a function that PHP calls, these panics become a PHP `Error`. The
+/// process does not abort. See [Exceptions](../exceptions.md#panics).
+///
 /// The last point can be important when interacting with functions that return
 /// unions, such as [`strpos`] which can return an integer or a boolean. In this
 /// case, a [`Zval`] should be returned as parsing a boolean to an integer is

crates/macros/src/module.rs

@@ -74,22 +74,27 @@ fn parser_impl(input: ItemFn, crate_name: Option<&str>, static_ext: bool) -> Res
 
             extern "C" fn ext_php_rs_startup(ty: i32, mod_num: i32) -> i32 {
                 let a = unsafe { #startup };
-                let b = __EXT_PHP_RS_MODULE_STARTUP
-                    .lock()
-                    .take()
-                    .map(|startup| {
-                        ::ext_php_rs::internal::ext_php_rs_startup();
-                        startup.startup(ty, mod_num).map(|_| 0).unwrap_or(1)
-                    })
-                    .unwrap_or_else(|| {
-                        // Module already started, call ext_php_rs_startup for idempotent
-                        // initialization (e.g., Closure::build early-returns if already built)
-                        ::ext_php_rs::internal::ext_php_rs_startup();
-                        0
-                    });
+                let b = ::ext_php_rs::internal::startup_guard(|| {
+                    // ext_php_rs_startup is idempotent (Closure::build early-returns once
+                    // built), so it runs whether or not this is the first startup.
+                    ::ext_php_rs::internal::ext_php_rs_startup();
+                    match __EXT_PHP_RS_MODULE_STARTUP.lock().take() {
+                        Some(startup) => startup.startup(ty, mod_num),
+                        None => Ok(()),
+                    }
+                });
                 a | b
             }
 
+            static __EXT_PHP_RS_BUILD_ERROR: ::std::sync::OnceLock<::std::string::String> =
+                ::std::sync::OnceLock::new();
+
+            extern "C" fn ext_php_rs_failed_startup(_ty: i32, _mod_num: i32) -> i32 {
+                ::ext_php_rs::internal::failed_module_startup(
+                    __EXT_PHP_RS_BUILD_ERROR.get().map_or("unknown error", ::std::string::String::as_str),
+                )
+            }
+
             __EXT_PHP_RS_MODULE_ENTRY.get_or_init(|| {
                 #[inline]
                 fn internal(#inputs) #output {
@@ -107,7 +112,20 @@ fn parser_impl(input: ItemFn, crate_name: Option<&str>, static_ext: bool) -> Res
                         __EXT_PHP_RS_MODULE_STARTUP.lock().replace(startup);
                         (entry, owned)
                     },
-                    Err(e) => panic!("Failed to build PHP module: {:?}", e),
+                    Err(e) => {
+                        // get_module cannot report failure to the engine (dl() dereferences the
+                        // returned entry unchecked), so hand back a placeholder entry whose
+                        // MINIT logs the build error and fails.
+                        let _ = __EXT_PHP_RS_BUILD_ERROR.set(e.to_string());
+                        let (entry, _, owned) = ::ext_php_rs::builders::ModuleBuilder::new(
+                            env!("CARGO_PKG_NAME"),
+                            env!("CARGO_PKG_VERSION"),
+                        )
+                        .startup_function(ext_php_rs_failed_startup)
+                        .try_into()
+                        .unwrap_or_else(|_| ::std::unreachable!("a module with only env! strings always builds"));
+                        (entry, owned)
+                    }
                 }
             })
         }
@@ -163,6 +181,22 @@ mod tests {
         assert!(out.contains(r#"#[unsafe(no_mangle)]extern"C"fnmy_ext_get_module("#));
     }
 
+    #[test]
+    fn build_failure_yields_a_placeholder_entry_whose_minit_fails() {
+        let out = expand(Some("my_ext"), false);
+        assert!(!out.contains("panic!"));
+        assert!(out.contains(r#"extern"C"fnext_php_rs_failed_startup("#));
+        assert!(out.contains("::ext_php_rs::internal::failed_module_startup("));
+        assert!(out.contains(".startup_function(ext_php_rs_failed_startup)"));
+    }
+
+    #[test]
+    fn minit_runs_registration_under_startup_guard() {
+        let out = expand(Some("my_ext"), false);
+        assert!(out.contains("::ext_php_rs::internal::startup_guard(||"));
+        assert!(!out.contains("unwrap_or(1)"));
+    }
+
     #[test]
     fn missing_crate_name_skips_delegate_in_dynamic_build() {
         assert!(!expand(None, false).contains("_get_module"));

docsrs_bindings.rs

@@ -3597,6 +3597,9 @@ unsafe extern "C" {
 unsafe extern "C" {
     pub static mut zend_ce_error_exception: *mut zend_class_entry;
 }
+unsafe extern "C" {
+    pub static mut zend_ce_error: *mut zend_class_entry;
+}
 unsafe extern "C" {
     pub static mut zend_ce_compile_error: *mut zend_class_entry;
 }

guide/src/advanced/bailout_guard.md

@@ -141,3 +141,7 @@ let result = try_catch(|| {
 // On Err, try_catch dropped _tmp. connection is still valid here.
 connection.query("...");
 ```
+
+`try_catch` also catches a panic inside the closure and returns
+`Err(CatchError::Panic(message))`. It does not resume the panic. If you want the
+panic to continue, call `std::panic::resume_unwind` with your own payload.