From c89b948ab54b1fcceea9d41924f4fb7fa62abbd4 Mon Sep 17 00:00:00 2001 From: David Langhals Date: Tue, 24 Jan 2023 09:49:50 +0100 Subject: [PATCH] Return message string from model panic handler to be displayed in UI status --- crates/fj-proc/src/expand.rs | 6 +++--- crates/fj/src/abi/context.rs | 40 +++++++++++++++++++++++++++-------- crates/fj/src/abi/ffi_safe.rs | 2 +- crates/fj/src/abi/mod.rs | 10 ++++----- crates/fj/src/abi/model.rs | 32 +++++++++++++++++++++------- crates/fj/src/models/model.rs | 2 +- 6 files changed, 65 insertions(+), 27 deletions(-) diff --git a/crates/fj-proc/src/expand.rs b/crates/fj-proc/src/expand.rs index d5a887205..990b0f711 100644 --- a/crates/fj-proc/src/expand.rs +++ b/crates/fj-proc/src/expand.rs @@ -64,9 +64,9 @@ impl ToTokens for Metadata { let Self { name, arguments } = self; tokens.extend(quote! { - fn metadata(&self) -> fj::models::ModelMetadata { - fj::models::ModelMetadata::new(#name) - #( .with_argument(#arguments) )* + fn metadata(&self) -> std::result::Result> { + Ok(fj::models::ModelMetadata::new(#name) + #( .with_argument(#arguments) )*) } }); } diff --git a/crates/fj/src/abi/context.rs b/crates/fj/src/abi/context.rs index 76805a5e2..80d4bc0fa 100644 --- a/crates/fj/src/abi/context.rs +++ b/crates/fj/src/abi/context.rs @@ -5,8 +5,13 @@ use crate::abi::ffi_safe::StringSlice; #[repr(C)] pub struct Context<'a> { user_data: *const c_void, - get_argument: - unsafe extern "C" fn(*const c_void, StringSlice) -> StringSlice, + get_argument: unsafe extern "C" fn( + *const c_void, + StringSlice, + ) -> crate::abi::ffi_safe::Result< + StringSlice, + crate::abi::ffi_safe::String, + >, _lifetime: PhantomData<&'a ()>, } @@ -15,15 +20,24 @@ impl<'a> From<&'a &dyn crate::models::Context> for Context<'a> { unsafe extern "C" fn get_argument( user_data: *const c_void, name: StringSlice, - ) -> StringSlice { + ) -> crate::abi::ffi_safe::Result< + StringSlice, + crate::abi::ffi_safe::String, + > { let ctx = &*(user_data as *const &dyn crate::models::Context); match std::panic::catch_unwind(AssertUnwindSafe(|| { ctx.get_argument(&name) })) { - Ok(Some(arg)) => StringSlice::from_str(arg), - Ok(None) => StringSlice::from_str(""), - Err(payload) => crate::abi::on_panic(payload), + Ok(Some(arg)) => { + crate::abi::ffi_safe::Result::Ok(StringSlice::from_str(arg)) + } + Ok(None) => { + crate::abi::ffi_safe::Result::Ok(StringSlice::from_str("")) + } + Err(payload) => crate::abi::ffi_safe::Result::Err( + crate::abi::on_panic(payload), + ), } } @@ -47,9 +61,17 @@ impl crate::models::Context for Context<'_> { let name = StringSlice::from_str(name); - match get_argument(user_data, name).into_str() { - "" => None, - other => Some(other), + match name.trim().is_empty() { + true => None, + false => match get_argument(user_data, name) { + super::ffi_safe::Result::Ok(other) => { + match other.is_empty() { + true => None, + false => Some(other.into_str()), + } + } + super::ffi_safe::Result::Err(_) => None, + }, } } } diff --git a/crates/fj/src/abi/ffi_safe.rs b/crates/fj/src/abi/ffi_safe.rs index 0aa23a257..684f3e779 100644 --- a/crates/fj/src/abi/ffi_safe.rs +++ b/crates/fj/src/abi/ffi_safe.rs @@ -338,7 +338,7 @@ impl Deref for StringSlice { #[derive(Debug)] #[repr(C)] pub struct BoxedError { - msg: String, + pub(crate) msg: String, } impl Display for BoxedError { diff --git a/crates/fj/src/abi/mod.rs b/crates/fj/src/abi/mod.rs index 9309bfe07..5a237bc94 100644 --- a/crates/fj/src/abi/mod.rs +++ b/crates/fj/src/abi/mod.rs @@ -72,7 +72,7 @@ pub use self::{ /// struct MyModel { } /// /// impl Model for MyModel { -/// fn metadata(&self) -> ModelMetadata { todo!() } +/// fn metadata(&self) -> std::result::Result> { todo!() } /// /// fn shape(&self, ctx: &dyn Context) -> Result { /// todo!() @@ -111,12 +111,14 @@ macro_rules! register_model { pub type InitFunction = unsafe extern "C" fn(*mut Host<'_>) -> InitResult; pub type InitResult = ffi_safe::Result; pub type ShapeResult = ffi_safe::Result; +pub type ModelMetadataResult = + ffi_safe::Result; /// The name of the function generated by [`register_model`]. /// pub const INIT_FUNCTION_NAME: &str = "fj_model_init"; -fn on_panic(payload: Box) -> ! { +fn on_panic(payload: Box) -> crate::abi::ffi_safe::String { let msg: &str = if let Some(s) = payload.downcast_ref::() { s.as_str() @@ -126,7 +128,5 @@ fn on_panic(payload: Box) -> ! { "A panic occurred" }; - eprintln!("{msg}"); - // It's not ideal, but panicking across the FFI boundary is UB. - std::process::abort(); + crate::abi::ffi_safe::String::from(msg.to_string()) } diff --git a/crates/fj/src/abi/model.rs b/crates/fj/src/abi/model.rs index d4c14ee1e..892285b20 100644 --- a/crates/fj/src/abi/model.rs +++ b/crates/fj/src/abi/model.rs @@ -1,14 +1,14 @@ use std::{os::raw::c_void, panic::AssertUnwindSafe}; use crate::{ - abi::{Context, ModelMetadata, ShapeResult}, + abi::{Context, ModelMetadataResult, ShapeResult}, models::Error, }; #[repr(C)] pub struct Model { ptr: *mut c_void, - metadata: unsafe extern "C" fn(*mut c_void) -> ModelMetadata, + metadata: unsafe extern "C" fn(*mut c_void) -> ModelMetadataResult, shape: unsafe extern "C" fn(*mut c_void, Context<'_>) -> ShapeResult, free: unsafe extern "C" fn(*mut c_void), } @@ -30,23 +30,35 @@ impl crate::models::Model for Model { } } - fn metadata(&self) -> crate::models::ModelMetadata { + fn metadata(&self) -> Result { let Self { ptr, metadata, .. } = *self; - unsafe { metadata(ptr).into() } + let result = unsafe { metadata(ptr) }; + + match result { + super::ffi_safe::Result::Ok(meta) => Ok(meta.into()), + super::ffi_safe::Result::Err(err) => Err(err.into()), + } } } impl From> for Model { fn from(m: Box) -> Self { - unsafe extern "C" fn metadata(user_data: *mut c_void) -> ModelMetadata { + unsafe extern "C" fn metadata( + user_data: *mut c_void, + ) -> ModelMetadataResult { let model = &*(user_data as *mut Box); match std::panic::catch_unwind(AssertUnwindSafe(|| { model.metadata() })) { - Ok(meta) => meta.into(), - Err(payload) => crate::abi::on_panic(payload), + Ok(Ok(meta)) => ModelMetadataResult::Ok(meta.into()), + Ok(Err(err)) => ModelMetadataResult::Err(err.into()), + Err(payload) => { + ModelMetadataResult::Err(crate::abi::ffi_safe::BoxedError { + msg: crate::abi::on_panic(payload), + }) + } } } @@ -61,7 +73,11 @@ impl From> for Model { })) { Ok(Ok(shape)) => ShapeResult::Ok(shape), Ok(Err(err)) => ShapeResult::Err(err.into()), - Err(payload) => crate::abi::on_panic(payload), + Err(payload) => { + ShapeResult::Err(crate::abi::ffi_safe::BoxedError { + msg: crate::abi::on_panic(payload), + }) + } } } diff --git a/crates/fj/src/models/model.rs b/crates/fj/src/models/model.rs index 6a157fd3c..6a2e818e2 100644 --- a/crates/fj/src/models/model.rs +++ b/crates/fj/src/models/model.rs @@ -9,7 +9,7 @@ pub trait Model: Send + Sync { fn shape(&self, ctx: &dyn Context) -> Result; /// Get metadata for the model. - fn metadata(&self) -> ModelMetadata; + fn metadata(&self) -> Result; } #[cfg(test)]