From af19fdc0676164306bcba850cc6efd007f0e2474 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 10:59:21 +0200 Subject: [PATCH 01/11] Don't quit, if receiving error from model These errors are recoverable, if the model changes. --- crates/fj-window/src/run.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index b7066e928..4047c4475 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -113,9 +113,6 @@ pub fn run( current_err = err; } - - *control_flow = ControlFlow::Exit; - return; } } } From b5120028973269c52e2791af1087bdcd6f9e6a51 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:07:59 +0200 Subject: [PATCH 02/11] Show model loading errors to user --- crates/fj-window/src/run.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 4047c4475..1d6897a77 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -100,19 +100,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; - } + status.update_status(&err.to_string()); } } } From db59162401aca1a62dd42b93eadf45eb38524312 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:10:36 +0200 Subject: [PATCH 03/11] Update order of enum variants Start moving them into the order in which they can show up. --- crates/fj-host/src/model.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index d9f0f9b58..155ce8bd8 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -231,17 +231,6 @@ 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, - }, - - /// I/O error while loading the model - #[error("I/O error while loading model")] - Io(#[from] io::Error), - /// Failed to load the model's dynamic library #[error("Error loading model from dynamic library")] LibLoading(#[from] libloading::Error), @@ -256,6 +245,17 @@ pub enum Error { model: String, }, + /// Model failed to compile + #[error("Error compiling model")] + 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), From 9cd0b9c6c9a637799caf7f1f3f4c67cd82d5bc70 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:15:33 +0200 Subject: [PATCH 04/11] Show nice error, if version can't be loaded --- crates/fj-host/src/model.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 155ce8bd8..076893819 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -104,7 +104,7 @@ impl Model { let lib = libloading::Library::new(&self.lib_path)?; let version_pkg: libloading::Symbol 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() { @@ -231,6 +231,14 @@ fn ambiguous_path_error( /// An error that can occur when loading or reloading a model #[derive(Debug, thiserror::Error)] pub enum 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), From 4433a84590611d8d65a868320ada4d167cb6b46f Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:17:21 +0200 Subject: [PATCH 05/11] Refactor --- crates/fj-interop/src/status_report.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/fj-interop/src/status_report.rs b/crates/fj-interop/src/status_report.rs index d0e3ec58d..dfe8a6ceb 100644 --- a/crates/fj-interop/src/status_report.rs +++ b/crates/fj-interop/src/status_report.rs @@ -18,9 +18,12 @@ 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()); + let date = { + let date = Local::now(); + format!("{} ", date.format("[%H:%M:%S]")) + }; + + let status = format!("\n{} {}", date, status.to_owned()); self.status.push_back(status); if self.status.len() > 5 { for _ in 0..(self.status.len() - 5) { From 5ad43b58a7b15b523bd676cdf3d1b40cc1c415fd Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:29:16 +0200 Subject: [PATCH 06/11] Use monospace font for status messages This improves multiline status messages more practical. Especially those from a source that expect a monospace font in the first place, like compiler errors. --- crates/fj-viewer/src/gui.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/fj-viewer/src/gui.rs b/crates/fj-viewer/src/gui.rs index 1b21cc03f..056e5b92d 100644 --- a/crates/fj-viewer/src/gui.rs +++ b/crates/fj-viewer/src/gui.rs @@ -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), )) From ddb63f47184741335049c57ec6b08c3d78b2ab98 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 11:30:23 +0200 Subject: [PATCH 07/11] Improve rendering of multi-line status messages --- crates/fj-interop/src/status_report.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/fj-interop/src/status_report.rs b/crates/fj-interop/src/status_report.rs index dfe8a6ceb..9405ff862 100644 --- a/crates/fj-interop/src/status_report.rs +++ b/crates/fj-interop/src/status_report.rs @@ -20,11 +20,17 @@ impl StatusReport { pub fn update_status(&mut self, status: &str) { let date = { let date = Local::now(); - format!("{} ", date.format("[%H:%M:%S]")) + format!("{}", date.format("[%H:%M:%S]")) }; + let empty_space = " ".repeat(date.chars().count()); - let status = format!("\n{} {}", date, status.to_owned()); - self.status.push_back(status); + 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(); From 06638367896bce91bcbe2aa1ca563013e523b368 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 12:49:32 +0200 Subject: [PATCH 08/11] Improve error message --- crates/fj-host/src/model.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 076893819..423534dc9 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -101,7 +101,8 @@ 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 RawVersion> = lib.get(b"version_pkg").map_err(Error::LoadingVersion)?; @@ -231,6 +232,14 @@ fn ambiguous_path_error( /// An error that can occur when loading or reloading a model #[derive(Debug, thiserror::Error)] pub enum Error { + /// 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), + /// Error loading Fornjot version that the model uses #[error( "Failed to load the Fornjot version that the model uses\n\ From cb8bc3bc1aeb4f5212e82ac095b30b44f6f407fc Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 12:53:20 +0200 Subject: [PATCH 09/11] Improve error message, if model has no `init` --- crates/fj-host/src/model.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 423534dc9..20148e786 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -120,8 +120,9 @@ impl Model { return Err(Error::VersionMismatch { host, model }); } - let init: libloading::Symbol = - lib.get(abi::INIT_FUNCTION_NAME.as_bytes())?; + let init: libloading::Symbol = lib + .get(abi::INIT_FUNCTION_NAME.as_bytes()) + .map_err(Error::LoadingInit)?; let mut host = Host::new(&self.parameters); @@ -248,9 +249,12 @@ pub enum Error { )] 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")] From 71ddd5d179112b9598a754b9186ef082e10392e1 Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 12:58:20 +0200 Subject: [PATCH 10/11] Improve error message --- crates/fj-host/src/model.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/fj-host/src/model.rs b/crates/fj-host/src/model.rs index 20148e786..02c17d40e 100644 --- a/crates/fj-host/src/model.rs +++ b/crates/fj-host/src/model.rs @@ -267,7 +267,7 @@ pub enum Error { }, /// Model failed to compile - #[error("Error compiling model")] + #[error("Error compiling model\n{output}")] Compile { /// The compiler output output: String, From 0721390318861277e971646b6fb33824e7559fba Mon Sep 17 00:00:00 2001 From: Hanno Braun Date: Wed, 26 Oct 2022 12:58:41 +0200 Subject: [PATCH 11/11] Don't use special code path for compile errors --- crates/fj-host/src/evaluator.rs | 13 ------------- crates/fj-window/src/run.rs | 3 --- 2 files changed, 16 deletions(-) diff --git a/crates/fj-host/src/evaluator.rs b/crates/fj-host/src/evaluator.rs index 70e61d8c7..1ba7eeb5e 100644 --- a/crates/fj-host/src/evaluator.rs +++ b/crates/fj-host/src/evaluator.rs @@ -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)) @@ -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), diff --git a/crates/fj-window/src/run.rs b/crates/fj-window/src/run.rs index 1d6897a77..a47c38ec5 100644 --- a/crates/fj-window/src/run.rs +++ b/crates/fj-window/src/run.rs @@ -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 {}!",