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

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

Merged
merged 1 commit into from
Sep 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion editor/src/communication/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion editor/src/frontend/frontend_message_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
24 changes: 13 additions & 11 deletions editor/src/lib.rs
Original file line number Diff line number Diff line change
@@ -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() }
Expand All @@ -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};
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/panels/Document.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/panels/LayerTree.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/widgets/inputs/MenuBarInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/widgets/inputs/SwatchPairInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/widgets/options/ToolOptions.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/utilities/documents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
71 changes: 63 additions & 8 deletions frontend/src/utilities/errors.ts
Original file line number Diff line number Diff line change
@@ -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 : ""}`;
Expand All @@ -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;

Expand All @@ -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 = `
Expand All @@ -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();

Expand All @@ -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"];
}
2 changes: 1 addition & 1 deletion frontend/src/utilities/input.ts
Original file line number Diff line number Diff line change
@@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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;
}
2 changes: 2 additions & 0 deletions frontend/src/utilities/response-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down
4 changes: 0 additions & 4 deletions frontend/wasm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading