Improve and clean up panic dialog code and wasm wrapper (#368)

Part of #357

Cargo.lock

@@ -130,7 +130,6 @@ dependencies = [
 name = "graphite-wasm"
 version = "0.1.0"
 dependencies = [
- "console_error_panic_hook",
  "graphite-editor",
  "graphite-graphene",
  "log",

editor/src/communication/dispatcher.rs

@@ -53,7 +53,7 @@ impl Dispatcher {
 	}
 
 	pub fn collect_actions(&self) -> ActionList {
-		//TODO: reduce the number of heap allocations
+		// TODO: Reduce the number of heap allocations
 		let mut list = Vec::new();
 		list.extend(self.input_preprocessor.actions());
 		list.extend(self.input_mapper.actions());

editor/src/frontend/frontend_message_handler.rs

@@ -13,7 +13,7 @@ pub enum FrontendMessage {
 	SetActiveDocument { document_index: usize },
 	UpdateOpenDocumentsList { open_documents: Vec<String> },
 	DisplayError { title: String, description: String },
-	DisplayPanic { title: String, description: String },
+	DisplayPanic { panic_info: String, title: String, description: String },
 	DisplayConfirmationToCloseDocument { document_index: usize },
 	DisplayConfirmationToCloseAllDocuments,
 	UpdateCanvas { document: String },

editor/src/lib.rs

@@ -1,4 +1,4 @@
-// since our policy is tabs, we want to stop clippy from warning about that
+// Since our policy is tabs, we want to stop clippy from warning about that
 #![allow(clippy::tabs_in_doc_comments)]
 
 extern crate graphite_proc_macros;
@@ -6,34 +6,30 @@ extern crate graphite_proc_macros;
 mod communication;
 #[macro_use]
 pub mod misc;
+pub mod consts;
 mod document;
 mod frontend;
 mod global;
 pub mod input;
 pub mod tool;
 
-pub mod consts;
+#[doc(inline)]
-
+pub use graphene::color::Color;
+#[doc(inline)]
+pub use graphene::document::Document as SvgDocument;
+#[doc(inline)]
+pub use graphene::LayerId;
 #[doc(inline)]
 pub use misc::EditorError;
 
-#[doc(inline)]
-pub use graphene::color::Color;
-
-#[doc(inline)]
-pub use graphene::LayerId;
-
-#[doc(inline)]
-pub use graphene::document::Document as SvgDocument;
-
 use communication::dispatcher::Dispatcher;
+use message_prelude::*;
+
 // TODO: serialize with serde to save the current editor state
 pub struct Editor {
 	dispatcher: Dispatcher,
 }
 
-use message_prelude::*;
-
 impl Editor {
 	pub fn new() -> Self {
 		Self { dispatcher: Dispatcher::new() }
@@ -49,6 +45,12 @@ impl Editor {
 	}
 }
 
+impl Default for Editor {
+	fn default() -> Self {
+		Self::new()
+	}
+}
+
 pub mod message_prelude {
 	pub use crate::communication::generate_uuid;
 	pub use crate::communication::message::{AsMessage, Message, MessageDiscriminant};

frontend/src/components/panels/Document.vue

@@ -227,7 +227,7 @@ import { defineComponent } from "vue";
 import { ResponseType, registerResponseHandler, Response, UpdateCanvas, UpdateScrollbars, SetActiveTool, SetCanvasZoom, SetCanvasRotation } from "@/utilities/response-handler";
 import { SeparatorDirection, SeparatorType } from "@/components/widgets/widgets";
 import { comingSoon } from "@/utilities/errors";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 
 import LayoutRow from "@/components/layout/LayoutRow.vue";
 import LayoutCol from "@/components/layout/LayoutCol.vue";

frontend/src/components/panels/LayerTree.vue

@@ -182,7 +182,7 @@
 import { defineComponent } from "vue";
 
 import { ResponseType, registerResponseHandler, Response, BlendMode, ExpandFolder, CollapseFolder, UpdateLayer, LayerPanelEntry, LayerType } from "@/utilities/response-handler";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 import { SeparatorType } from "@/components/widgets/widgets";
 
 import LayoutRow from "@/components/layout/LayoutRow.vue";

frontend/src/components/widgets/inputs/MenuBarInput.vue

@@ -54,7 +54,7 @@
 import { defineComponent } from "vue";
 
 import { comingSoon } from "@/utilities/errors";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 
 import IconLabel from "@/components/widgets/labels/IconLabel.vue";
 import { ApplicationPlatform } from "@/components/window/MainWindow.vue";

frontend/src/components/widgets/inputs/SwatchPairInput.vue

@@ -69,7 +69,7 @@
 import { defineComponent } from "vue";
 
 import { rgbToDecimalRgb, RGB } from "@/utilities/color";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 import { ResponseType, registerResponseHandler, Response, UpdateWorkingColors } from "@/utilities/response-handler";
 
 import ColorPicker from "@/components/widgets/floating-menus/ColorPicker.vue";

frontend/src/components/widgets/options/ToolOptions.vue

@@ -32,7 +32,7 @@
 import { defineComponent, PropType } from "vue";
 
 import { comingSoon } from "@/utilities/errors";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 import { WidgetRow, SeparatorType, IconButtonWidget } from "@/components/widgets/widgets";
 
 import Separator from "@/components/widgets/separators/Separator.vue";

frontend/src/utilities/documents.ts

@@ -12,7 +12,7 @@ import {
 	SaveDocument,
 } from "@/utilities/response-handler";
 import { download, upload } from "@/utilities/files";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 
 const wasm = import("@/../wasm/pkg").then(panicProxy);
 

frontend/src/utilities/errors.ts

@@ -1,8 +1,8 @@
 import { createDialog, dismissDialog } from "@/utilities/dialog";
 import { TextButtonWidget } from "@/components/widgets/widgets";
-import { getPanicDetails } from "@/utilities/panic";
 import { ResponseType, registerResponseHandler, Response, DisplayError, DisplayPanic } from "@/utilities/response-handler";
 
+// Coming soon dialog
 export function comingSoon(issueNumber?: number) {
 	const bugMessage = `— but you can help add it!\nSee issue #${issueNumber} on GitHub.`;
 	const details = `This feature is not implemented yet${issueNumber ? bugMessage : ""}`;
@@ -23,6 +23,7 @@ export function comingSoon(issueNumber?: number) {
 	createDialog("Warning", "Coming soon", details, buttons);
 }
 
+// Graphite error dialog
 registerResponseHandler(ResponseType.DisplayError, (responseData: Response) => {
 	const data = responseData as DisplayError;
 
@@ -36,9 +37,18 @@ registerResponseHandler(ResponseType.DisplayError, (responseData: Response) => {
 	createDialog("Warning", data.title, data.description, buttons);
 });
 
+// Code panic dialog and console error
 registerResponseHandler(ResponseType.DisplayPanic, (responseData: Response) => {
 	const data = responseData as DisplayPanic;
 
+	// eslint-disable-next-line @typescript-eslint/no-explicit-any
+	(Error as any).stackTraceLimit = Infinity;
+	const stackTrace = new Error().stack || "";
+	const panicDetails = `${data.panic_info}\n\n${stackTrace}`;
+
+	// eslint-disable-next-line no-console
+	console.error(panicDetails);
+
 	const reloadButton: TextButtonWidget = {
 		kind: "TextButton",
 		callback: async () => window.location.reload(),
@@ -46,12 +56,12 @@ registerResponseHandler(ResponseType.DisplayPanic, (responseData: Response) => {
 	};
 	const copyErrorLogButton: TextButtonWidget = {
 		kind: "TextButton",
-		callback: async () => navigator.clipboard.writeText(getPanicDetails()),
+		callback: async () => navigator.clipboard.writeText(panicDetails),
 		props: { label: "Copy Error Log", emphasized: false, minWidth: 96 },
 	};
 	const reportOnGithubButton: TextButtonWidget = {
 		kind: "TextButton",
-		callback: async () => window.open(githubUrl(), "_blank"),
+		callback: async () => window.open(githubUrl(panicDetails), "_blank"),
 		props: { label: "Report Bug", emphasized: false, minWidth: 96 },
 	};
 	const buttons = [reloadButton, copyErrorLogButton, reportOnGithubButton];
@@ -59,7 +69,7 @@ registerResponseHandler(ResponseType.DisplayPanic, (responseData: Response) => {
 	createDialog("Warning", data.title, data.description, buttons);
 });
 
-function githubUrl() {
+function githubUrl(panicDetails: string) {
 	const url = new URL("https://github.com/GraphiteEditor/Graphite/issues/new");
 
 	const body = `
@@ -74,17 +84,17 @@ Describe precisely how the crash occurred, step by step, starting with a new edi
 4. 
 5. 
 
-**Browser and OS*
-List of your browser and its version, as well as your operating system.
-
 **Additional Details**
 Provide any further information or context that you think would be helpful in fixing the issue. Screenshots or video can be linked or attached to this issue.
 
+**Browser and OS**
+${browserVersion()}, ${operatingSystem()}
+
 **Stack Trace**
 Copied from the crash dialog in the Graphite Editor:
 
 \`\`\`
-${getPanicDetails()}
+${panicDetails}
 \`\`\`
 `.trim();
 
@@ -104,3 +114,48 @@ ${getPanicDetails()}
 
 	return url.toString();
 }
+
+function browserVersion(): string {
+	const agent = window.navigator.userAgent;
+	let match = agent.match(/(opera|chrome|safari|firefox|msie|trident(?=\/))\/?\s*(\d+)/i) || [];
+
+	if (/trident/i.test(match[1])) {
+		const browser = /\brv[ :]+(\d+)/g.exec(agent) || [];
+		return `IE ${browser[1] || ""}`.trim();
+	}
+
+	if (match[1] === "Chrome") {
+		let browser = agent.match(/\bEdg\/(\d+)/);
+		if (browser !== null) return `Edge (Chromium) ${browser[1]}`;
+
+		browser = agent.match(/\bOPR\/(\d+)/);
+		if (browser !== null) return `Opera ${browser[1]}`;
+	}
+
+	match = match[2] ? [match[1], match[2]] : [navigator.appName, navigator.appVersion, "-?"];
+
+	const browser = agent.match(/version\/(\d+)/i);
+	if (browser !== null) match.splice(1, 1, browser[1]);
+
+	return `${match[0]} ${match[1]}`;
+}
+
+function operatingSystem(): string {
+	const osTable: Record<string, string> = {
+		"Windows NT 11": "Windows 11",
+		"Windows NT 10": "Windows 10",
+		"Windows NT 6.3": "Windows 8.1",
+		"Windows NT 6.2": "Windows 8",
+		"Windows NT 6.1": "Windows 7",
+		"Windows NT 6.0": "Windows Vista",
+		"Windows NT 5.1": "Windows XP",
+		"Windows NT 5.0": "Windows 2000",
+		Mac: "Mac",
+		X11: "Unix",
+		Linux: "Linux",
+		Unknown: "YOUR OPERATING SYSTEM",
+	};
+
+	const userAgentOS = Object.keys(osTable).find((key) => window.navigator.userAgent.includes(key));
+	return osTable[userAgentOS || "Unknown"];
+}

frontend/src/utilities/input.ts

@@ -1,6 +1,6 @@
 import { toggleFullscreen } from "@/utilities/fullscreen";
 import { dialogIsVisible, dismissDialog, submitDialog } from "@/utilities/dialog";
-import { panicProxy } from "@/utilities/panic";
+import { panicProxy } from "@/utilities/panic-proxy";
 
 const wasm = import("@/../wasm/pkg").then(panicProxy);
 

frontend/src/utilities/panic-proxy.ts

@@ -1,20 +1,19 @@
+/* eslint-disable @typescript-eslint/no-explicit-any, func-names */
+
 // Import this function and chain it on all `wasm` imports like: const wasm = import("@/../wasm/pkg").then(panicProxy);
 // This works by proxying every function call wrapping a try-catch block to filter out redundant and confusing `RuntimeError: unreachable` exceptions sent to the console
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function panicProxy(module: any) {
 	const proxyHandler = {
-		// eslint-disable-next-line @typescript-eslint/no-explicit-any
 		get(target: any, propKey: any, receiver: any) {
 			const targetValue = Reflect.get(target, propKey, receiver);
 
 			// Keep the original value being accessed if it isn't a function or it is a class
-			// TODO: Figure out how to also wrap (class) constructor functions instead of skipping them for now
+			// TODO: Figure out how to also wrap class constructor functions instead of skipping them for now
 			const isFunction = typeof targetValue === "function";
 			const isClass = isFunction && /^\s*class\s+/.test(targetValue.toString());
 			if (!isFunction || isClass) return targetValue;
 
 			// Replace the original function with a wrapper function that runs the original in a try-catch block
-			// eslint-disable-next-line @typescript-eslint/no-explicit-any, func-names
 			return function (...args: any) {
 				let result;
 				try {
@@ -31,20 +30,3 @@ export function panicProxy(module: any) {
 
 	return new Proxy(module, proxyHandler);
 }
-
-// Intercept console.error() for panic messages sent by code in the WASM toolchain
-let panicDetails = "";
-// eslint-disable-next-line no-console
-const error = console.error.bind(console);
-// eslint-disable-next-line no-console
-console.error = (...args) => {
-	const details = "".concat(...args).trim();
-	if (details.startsWith("panicked at")) panicDetails = details;
-
-	error(...args);
-};
-
-// Get the body of the panic's exception that was printed in the console
-export function getPanicDetails(): string {
-	return panicDetails;
-}

frontend/src/utilities/response-handler.ts

@@ -160,11 +160,13 @@ function newDisplayError(input: any): DisplayError {
 }
 
 export interface DisplayPanic {
+	panic_info: string;
 	title: string;
 	description: string;
 }
 function newDisplayPanic(input: any): DisplayPanic {
 	return {
+		panic_info: input.panic_info,
 		title: input.title,
 		description: input.description,
 	};

frontend/wasm/Cargo.toml

@@ -12,11 +12,7 @@ publish = false
 [lib]
 crate-type = ["cdylib", "rlib"]
 
-[features]
-default = ["console_error_panic_hook"]
-
 [dependencies]
-console_error_panic_hook = { version = "0.1.6", optional = true }
 editor = { path = "../../editor", package = "graphite-editor" }
 graphene = { path = "../../graphene", package = "graphite-graphene" }
 log = "0.4"

frontend/wasm/src/lib.rs

@@ -5,63 +5,67 @@ pub mod wrappers;
 
 use editor::{message_prelude::*, Editor};
 use std::cell::RefCell;
+use std::panic;
 use std::sync::atomic::AtomicBool;
 use utils::WasmLog;
 use wasm_bindgen::prelude::*;
 
-// The thread_local macro provides a way to initialize static variables with non-constant functions
+// Set up the persistent editor backend state (the thread_local macro provides a way to initialize static variables with non-constant functions)
-thread_local! {
+thread_local! { pub static EDITOR_STATE: RefCell<Editor> = RefCell::new(Editor::new()); }
-	pub static EDITOR_STATE: RefCell<Editor> = RefCell::new(Editor::new());
-}
 static LOGGER: WasmLog = WasmLog;
 static EDITOR_HAS_CRASHED: AtomicBool = AtomicBool::new(false);
 
+// Initialize the backend
 #[wasm_bindgen(start)]
 pub fn init() {
-	utils::set_panic_hook();
+	panic::set_hook(Box::new(panic_hook));
+
 	log::set_logger(&LOGGER).expect("Failed to set logger");
 	log::set_max_level(log::LevelFilter::Debug);
 }
 
-// Sends FrontendMessages to JavaScript
+// When a panic occurs, close up shop before the backend dies
+fn panic_hook(info: &panic::PanicInfo) {
+	let panic_info = info.to_string();
+	let title = "The editor crashed — sorry about that".to_string();
+	let description = "An internal error occurred. Reload the editor to continue. Please report this by filing an issue on GitHub.".to_string();
+
+	handle_response(FrontendMessage::DisplayPanic { panic_info, title, description });
+
+	EDITOR_HAS_CRASHED.store(true, std::sync::atomic::Ordering::SeqCst);
+}
+
+// Sends a message to the dispatcher in the Editor Backend
 fn dispatch<T: Into<Message>>(message: T) {
 	// Process no further messages after a crash to avoid spamming the console
 	if EDITOR_HAS_CRASHED.load(std::sync::atomic::Ordering::SeqCst) {
 		return;
 	}
 
-	match EDITOR_STATE.with(|state| state.try_borrow_mut().ok().map(|mut state| state.handle_message(message.into()))) {
+	// Dispatch the message and receive a vector of FrontendMessage responses
-		Some(messages) => {
+	let responses = EDITOR_STATE.with(|state| state.try_borrow_mut().ok().map(|mut state| state.handle_message(message.into())));
-			for message in messages.into_iter() {
+	for response in responses.unwrap_or_default().into_iter() {
-				handle_response(message);
+		// Send each FrontendMessage to the JavaScript frontend
-			}
+		handle_response(response);
-		}
-		None => {
-			EDITOR_HAS_CRASHED.store(true, std::sync::atomic::Ordering::SeqCst);
-
-			let title = "The editor crashed — sorry about that".to_string();
-			let description = "An internal error occurred. Reload the editor to continue. Please report this by filing an issue on GitHub.".to_string();
-
-			handle_response(FrontendMessage::DisplayPanic { title, description });
-		}
 	}
 }
 
 // Sends a FrontendMessage to JavaScript
 fn handle_response(message: FrontendMessage) {
 	let message_type = message.to_discriminant().local_name();
-	let message_data = JsValue::from_serde(&message).expect("Failed to serialize response");
+	let message_data = JsValue::from_serde(&message).expect("Failed to serialize FrontendMessage");
 
-	let _ = handleResponse(message_type, message_data).map_err(|error| {
+	let js_return_value = handleResponse(message_type, message_data);
+	if let Err(error) = js_return_value {
 		log::error!(
 			"While handling FrontendMessage \"{:?}\", JavaScript threw an error: {:?}",
 			message.to_discriminant().local_name(),
-			error
+			error,
 		)
-	});
+	}
 }
 
-// The JavaScript function to call into
+// The JavaScript function to call into with each FrontendMessage
 #[wasm_bindgen(module = "/../src/utilities/response-handler-binding.ts")]
 extern "C" {
 	#[wasm_bindgen(catch)]

frontend/wasm/src/utils.rs

@@ -1,14 +1,4 @@
 use wasm_bindgen::prelude::*;
-pub fn set_panic_hook() {
-	// When the `console_error_panic_hook` feature is enabled, we can call the
-	// `set_panic_hook` function at least once during initialization, and then
-	// we will get better error messages if our code ever panics.
-	//
-	// For more details see
-	// https://github.com/rustwasm/console_error_panic_hook#readme
-	#[cfg(feature = "console_error_panic_hook")]
-	console_error_panic_hook::set_once();
-}
 
 #[wasm_bindgen]
 extern "C" {