Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some crashes, turn them into actionable errors #1276

Merged
merged 11 commits into from
Oct 26, 2022
13 changes: 0 additions & 13 deletions crates/fj-host/src/evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,6 @@ impl Evaluator {

let evaluation = match model.evaluate() {
Ok(evaluation) => evaluation,
Err(Error::Compile { output }) => {
event_tx
.send(ModelEvent::StatusUpdate(format!(
"Failed to compile model:\n{}",
output
)))
.expect("Expected channel to never disconnect");

continue;
}
Err(err) => {
event_tx
.send(ModelEvent::Error(err))
Expand Down Expand Up @@ -65,9 +55,6 @@ impl Evaluator {

/// An event emitted by [`Evaluator`]
pub enum ModelEvent {
/// A status update about the model
StatusUpdate(String),

/// The model has been evaluated
Evaluation(Evaluation),

Expand Down
53 changes: 37 additions & 16 deletions crates/fj-host/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,11 @@ impl Model {
// to switch to a better technique:
// https://github.com/hannobraun/Fornjot/issues/71
let shape = unsafe {
let lib = libloading::Library::new(&self.lib_path)?;
let lib = libloading::Library::new(&self.lib_path)
.map_err(Error::LoadingLibrary)?;

let version_pkg: libloading::Symbol<fn() -> RawVersion> =
lib.get(b"version_pkg")?;
lib.get(b"version_pkg").map_err(Error::LoadingVersion)?;

let version_pkg = version_pkg();
if fj::version::VERSION_PKG != version_pkg.as_str() {
Expand All @@ -119,8 +120,9 @@ impl Model {
return Err(Error::VersionMismatch { host, model });
}

let init: libloading::Symbol<abi::InitFunction> =
lib.get(abi::INIT_FUNCTION_NAME.as_bytes())?;
let init: libloading::Symbol<abi::InitFunction> = lib
.get(abi::INIT_FUNCTION_NAME.as_bytes())
.map_err(Error::LoadingInit)?;

let mut host = Host::new(&self.parameters);

Expand Down Expand Up @@ -231,20 +233,28 @@ fn ambiguous_path_error(
/// An error that can occur when loading or reloading a model
#[derive(Debug, thiserror::Error)]
pub enum Error {
/// Model failed to compile
#[error("Error compiling model")]
Compile {
/// The compiler output
output: String,
},
/// Error loading model library
#[error(
"Failed to load model library\n\
This might be a bug in Fornjot, or, at the very least, this error \
message should be improved. Please report this!"
)]
LoadingLibrary(#[source] libloading::Error),

/// I/O error while loading the model
#[error("I/O error while loading model")]
Io(#[from] io::Error),
/// Error loading Fornjot version that the model uses
#[error(
"Failed to load the Fornjot version that the model uses\n\
- Is your model using the `fj` library? All models must!\n\
- Was your model created with a really old version of Fornjot?"
)]
LoadingVersion(#[source] libloading::Error),

/// Failed to load the model's dynamic library
#[error("Error loading model from dynamic library")]
LibLoading(#[from] libloading::Error),
/// Error loading the model's `init` function
#[error(
"Failed to load the model's `init` function\n\
- Did you define a model function using `#[fj::model]`?"
)]
LoadingInit(#[source] libloading::Error),

/// Host version and model version do not match
#[error("Host version ({host}) and model version ({model}) do not match")]
Expand All @@ -256,6 +266,17 @@ pub enum Error {
model: String,
},

/// Model failed to compile
#[error("Error compiling model\n{output}")]
Compile {
/// The compiler output
output: String,
},

/// I/O error while loading the model
#[error("I/O error while loading model")]
Io(#[from] io::Error),

/// Initializing a model failed.
#[error("Unable to initialize the model")]
InitializeModel(#[source] fj::models::Error),
Expand Down
17 changes: 13 additions & 4 deletions crates/fj-interop/src/status_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,19 @@ impl StatusReport {

/// Update the status
pub fn update_status(&mut self, status: &str) {
let date = Local::now();
let status =
format!("\n{} {}", date.format("[%H:%M:%S]"), status.to_owned());
self.status.push_back(status);
let date = {
let date = Local::now();
format!("{}", date.format("[%H:%M:%S]"))
};
let empty_space = " ".repeat(date.chars().count());

let mut rendered = String::new();
for (i, line) in status.lines().enumerate() {
let prefix = if i == 0 { &date } else { &empty_space };
rendered.push_str(&format!("\n{prefix} {line}"));
}

self.status.push_back(rendered);
if self.status.len() > 5 {
for _ in 0..(self.status.len() - 5) {
self.status.pop_front();
Expand Down
1 change: 1 addition & 0 deletions crates/fj-viewer/src/gui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ impl Gui {
ui.group(|ui| {
ui.add(egui::Label::new(
egui::RichText::new(format!("Status:{}", status.status()))
.monospace()
.color(egui::Color32::BLACK)
.background_color(egui::Color32::WHITE),
))
Expand Down
20 changes: 1 addition & 19 deletions crates/fj-window/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ pub fn run(
};

match event {
ModelEvent::StatusUpdate(status_update) => {
status.update_status(&status_update)
}
ModelEvent::Evaluation(evaluation) => {
status.update_status(&format!(
"Model compiled successfully in {}!",
Expand Down Expand Up @@ -100,22 +97,7 @@ pub fn run(
}
}
ModelEvent::Error(err) => {
// Can be cleaned up, once `Report` is stable:
// https://doc.rust-lang.org/std/error/struct.Report.html

println!("Error receiving updated shape: {}", err);

let mut current_err = &err as &dyn error::Error;
while let Some(err) = current_err.source() {
println!();
println!("Caused by:");
println!(" {}", err);

current_err = err;
}

*control_flow = ControlFlow::Exit;
return;
status.update_status(&err.to_string());
}
}
}
Expand Down