Skip to content

Commit

Permalink
Merge pull request #734 from luketpeterson/main
Browse files Browse the repository at this point in the history
Propagating python errors in loader through to MeTTa
  • Loading branch information
vsbogd authored Jul 9, 2024
2 parents e027fe2 + dee834d commit fb08ded
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 12 deletions.
35 changes: 33 additions & 2 deletions c/src/metta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,19 +1180,33 @@ pub extern "C" fn metta_get_module_space(metta: *const metta_t, mod_id: module_i
pub struct run_context_t {
/// Internal. Should not be accessed directly
context: *mut RustRunContext,
err_string: *mut c_char,
}

//LP-TODO-NEXT I need to implement a solution to automatically retire run_context_t so we can throw a
// predictable error when a stale run_context_t is accessed. This is particularly important for the Python
// layer because it's harder to exercise lifecycle discipline in Python and a bug in Python shouldn't lead
// to invlid memory access

impl run_context_t {
fn take_err_string(&mut self) -> Option<String> {
if !self.err_string.is_null() {
let return_string = unsafe{ std::ffi::CString::from_raw(self.err_string) }.to_str().expect("UTF-8 error").to_string();
self.err_string = core::ptr::null_mut();
Some(return_string)
} else {
None
}
}
}

struct RustRunContext(RunContext<'static, 'static, 'static>);

impl From<&mut RunContext<'_, '_, '_>> for run_context_t {
fn from(context_ref: &mut RunContext<'_, '_, '_>) -> Self {
Self {
context: (context_ref as *mut RunContext<'_, '_, '_>).cast()
context: (context_ref as *mut RunContext<'_, '_, '_>).cast(),
err_string: core::ptr::null_mut(),
}
}
}
Expand Down Expand Up @@ -1260,6 +1274,20 @@ pub extern "C" fn run_context_get_tokenizer(run_context: *const run_context_t) -
context.module().tokenizer().clone().into()
}

/// @brief Sets a runtime error
/// @ingroup interpreter_group
/// @param[in] run_context A pointer to the `run_context_t` to access the runner API
/// @param[in] message A C-string specifying an error message
/// @note Raising an error through this function will cause the MeTTa interpreter to take an error pathway
///
#[no_mangle]
pub extern "C" fn run_context_raise_error(run_context: *mut run_context_t, message: *const c_char) {
let context = unsafe{ &mut *run_context };
let msg_str = unsafe{ std::ffi::CStr::from_ptr(message) };
let _ = context.take_err_string(); //Make sure an existing err string gets dropped.
context.err_string = std::ffi::CString::from(msg_str).into_raw();
}

/// @brief Represents the state of an in-flight MeTTa execution run
/// @ingroup interpreter_group
/// @note A `runner_state_t` is initially created by `runner_state_new_with_parser()`. Each call to `metta_run_step()`, in
Expand Down Expand Up @@ -1929,7 +1957,10 @@ impl ModuleLoader for CFsModFmtLoader {

(api.load)(self.payload, &mut c_context, self.callback_context);

Ok(())
match c_context.take_err_string() {
None => Ok(()),
Some(err_string) => Err(err_string),
}
}
fn get_resource(&self, _res_key: ResourceKey) -> Result<Vec<u8>, String> {
//TODO, add C API for providing resources
Expand Down
5 changes: 2 additions & 3 deletions python/hyperon/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,8 +362,7 @@ def loader_func(run_context):
if '__name__' in dir(obj) and obj.__name__ == 'metta_register':
obj(run_context)

except:
# LP-TODO-Next, need to create error pathway through C interface
raise RuntimeError("Error loading Python module: ", pymod_name)
except Exception as e:
raise RuntimeError("Error loading Python module: ", pymod_name, e)

return loader_func
8 changes: 7 additions & 1 deletion python/hyperonpy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,13 @@ void load_mod_fmt_callback(const void* payload, run_context_t* run_context, void
py::object* callback_context_obj = (py::object*)callback_context;
py::function py_func = fmt_interface_obj->attr("_load_called_from_c");
CRunContext c_run_context = CRunContext(run_context);
py_func(&c_run_context, callback_context_obj);
try {
py_func(&c_run_context, callback_context_obj);
} catch (py::error_already_set &e) {
char message[4096];
snprintf(message, lenghtof(message), "Exception caught:\n%s", e.what());
run_context_raise_error(run_context, message);
}
}

void free_mod_fmt_context(void* callback_context) {
Expand Down
8 changes: 2 additions & 6 deletions python/tests/test_extend.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,8 @@ def test_error_pyext(self):
This test verifies that an error from a Python extension is properly propagated
'''
metta = MeTTa(env_builder=Environment.custom_env(working_dir=os.getcwd(), disable_config=True, is_test=True))
try:
metta.run("!(import! &self error_pyext)")
except Exception as err:
pass
else:
raise Exception('error_pyext.py should raise an error when loading, so no-err is an error')
result = metta.run("!(import! &self error_pyext)")
self.assertEqual(S('Error'), result[0][0].get_children()[0])

if __name__ == "__main__":
unittest.main()

0 comments on commit fb08ded

Please sign in to comment.