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

Keavon · Keavon · commit 6e546aa72628 · 2021-09-01T04:09:23.000-07:00
Part of #357
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/editor/src/communication/dispatcher.rs b/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());
diff --git a/editor/src/frontend/frontend_message_handler.rs b/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 },
diff --git a/editor/src/lib.rs b/editor/src/lib.rs
@@ -1,39 +1,35 @@
-// 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;
 
 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 misc::EditorError;
-
 #[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 graphene::document::Document as SvgDocument;
+pub use misc::EditorError;
 
 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};
diff --git a/frontend/src/components/panels/Document.vue b/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";
diff --git a/frontend/src/components/panels/LayerTree.vue b/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";
diff --git a/frontend/src/components/widgets/inputs/MenuBarInput.vue b/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";
diff --git a/frontend/src/components/widgets/inputs/SwatchPairInput.vue b/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";
diff --git a/frontend/src/components/widgets/options/ToolOptions.vue b/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";
diff --git a/frontend/src/utilities/documents.ts b/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);
 
diff --git a/frontend/src/utilities/errors.ts b/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,30 +37,39 @@ 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(),
 		props: { label: "Reload", emphasized: true, minWidth: 96 },
 	};
 	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];
 
 	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"];
+}
diff --git a/frontend/src/utilities/input.ts b/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);
 
diff --git a/frontend/src/utilities/panic-proxy.ts b/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;
-}
diff --git a/frontend/src/utilities/response-handler.ts b/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,
 	};
diff --git a/frontend/wasm/Cargo.toml b/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"
diff --git a/frontend/wasm/src/lib.rs b/frontend/wasm/src/lib.rs
diff --git a/frontend/wasm/src/utils.rs b/frontend/wasm/src/utils.rs

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ impl Dispatcher {`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`pub fn collect_actions(&self) -> ActionList {`
`56`		`- //TODO: reduce the number of heap allocations`
	`56`	`+ // TODO: Reduce the number of heap allocations`
`57`	`57`	`let mut list = Vec::new();`
`58`	`58`	`list.extend(self.input_preprocessor.actions());`
`59`	`59`	`list.extend(self.input_mapper.actions());`