Error tooltips and logging 🤕 #32

bengreenier · 2018-10-02T15:06:51Z

This adds all those fancy debugging and safety features you've been looking for! ✨

Handle plugins that fail to load (require('./path/to/plugin') fails)
Handle plugins that fail to render (React.createElement(Plugin) fails)
Log critical events
Default log level to verbose when NODE_ENV !== 'production'
fixes error handling when custom overlays fail #7

The logging uses electron-log which writes to:

on Linux: ~/.config/<app name>/log.log
on OS X: ~/Library/Logs/<app name>/log.log
on Windows: %USERPROFILE%\AppData\Roaming\<app name>\log.log

Plugin failures are communicated using the Tray.displayBalloon() electron api, as well as being written to the log.

Examples:

showing tooltip Plugin Render Error
[2018-10-02 10:52:38.229] [error] Failed to render clock located at V:\vs_workspace\overlayed\dist\app\plugin\Clock

showing tooltip Plugin Load Error
[2018-10-02 10:46:26.621] [error] Failed to load clock from V:\vs_workspace\overlayed\dist\app\plugin\Clock

Ethan-Arrowood · 2018-10-03T17:20:58Z

src/app/helpers/serialization.tsx

-        component: require(componentPath).default as React.ComponentType<any>
-      }} as IInstalledPlugin
+      try {
+        return {...plugin, ...{


You can rewrite this to:

return { ...plugin, component: require(componentPath).default as React.ComponentType<any> } as IInstalledPlugin

this is cool - i'm going to punt on this for now though, as there's quite a few places in the codebase that do this that deserve to be refactored at the same time. see #36

Ethan-Arrowood · 2018-10-03T17:27:56Z

src/app/main/ErrorBoundry.tsx

+
+  public componentDidCatch(error : Error) {
+    this.setState({ hasError: true })
+    this.props.onError(error)


Is this onError function dependent on state in anyway? If so you should consider moving line 11 into a callback function of the setState method on line 10.

not dependent, as such keeping as is for now

Ethan-Arrowood · 2018-10-03T17:31:26Z

src/app/main/Plugin.tsx

@@ -20,6 +24,7 @@ export class Plugin extends React.Component<IPluginProps, any> {
    super(props)

    this.onLabelLinkClicked = this.onLabelLinkClicked.bind(this)
+    this.onError = this.onError.bind(this)


To avoid binding the component this you can declare your onError method differently. See how I do it here

Also check out the next comment on the onError function where I show you the proper rewrite.

makes sense, but punting for now as it requires a larger change across the codebase for consistency: #37

Ethan-Arrowood · 2018-10-03T17:32:04Z

src/app/main/Plugin.tsx

        open(uri)
      }
    }
  }
+
+  private onError(err : Error) {


rewrite to

private onError = (err : Error) => { ... }

Ethan-Arrowood · 2018-10-03T17:38:09Z

src/app/main/PluginGrid.tsx

    // install the uninstalled plugins
    Promise.all(this.installPlugins(installNeeded)).then((installedPlugins) => {
      // when these load, merge them in
      this.setState({
        plugins: this.state.plugins.concat(installedPlugins)
      })
+    }, (error) => {
+      log.error(`failed installation: ${error}`)


if error is an object it may be printed like so: failed installation: [object Object]
if it is an instance of Error then it will print like so: failed installation: Error: <message>

I recommend using error.message when logging errors through template strings

would prefer to refactor template content to expect Error: message than rely on a specific object shape here. will make the change - good catch!

this looks like the only outstanding comment then its good to merge ✨

Ethan-Arrowood · 2018-10-03T17:38:33Z

src/app/main/PluginGrid.tsx

-            component: require(plugin.component).default as React.ComponentType<any>
-          }} as IInstalledPlugin)
+          try {
+            resolve({...plugin, ...{


see above comment on better obj destructuring

Ethan-Arrowood · 2018-10-03T17:39:14Z

src/electron/Bootstrap.ts

@@ -0,0 +1,146 @@
+import { app, ipcMain, Menu, MenuItem, nativeImage, Tray } from "electron"
+import log from 'electron-log'


inconsistent quote characters. Surprised linter didn't yell at you for this.

Ethan-Arrowood · 2018-10-03T17:43:50Z

src/electron/CompositeWindow.ts

@@ -30,12 +31,14 @@ export class CompositeWindow extends BrowserWindow {

    // notify react side
    this.webContents.send(IPCMessageNames.ToggleEditMode, this.isInteractive)
+    log('toggled compositor interactivity')


Throughout the previous code you are using log.info or log.error; why nothing here?

bengreenier added 3 commits October 2, 2018 11:01

add logging

a51b54b

add plugin load tooltips

24bcafe

add error boundry and render tooltip

18a9112

bengreenier added the enhancement New feature or request label Oct 2, 2018

bengreenier added this to the 1.0.0 Release milestone Oct 2, 2018

bengreenier self-assigned this Oct 2, 2018

Ethan-Arrowood suggested changes Oct 3, 2018

View reviewed changes

This was referenced Oct 4, 2018

refactor object spreading logic #36

Open

refactor react+es6 method syntax #37

Open

bengreenier added 3 commits October 3, 2018 20:38

correct quotes

3a2f2c0

correct log method usage

f2e992a

improve template string for error

ddd16a1

bengreenier merged commit 18b49b2 into master Oct 4, 2018

bengreenier deleted the feature/error-tooltips branch October 4, 2018 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Error tooltips and logging 🤕 #32

Error tooltips and logging 🤕 #32

bengreenier commented Oct 2, 2018 •

edited

Loading

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 4, 2018

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 4, 2018

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 4, 2018

Ethan-Arrowood Oct 3, 2018

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 4, 2018

Ethan-Arrowood Oct 4, 2018

Ethan-Arrowood Oct 3, 2018

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 4, 2018

Ethan-Arrowood Oct 3, 2018

bengreenier Oct 3, 2018

		@@ -0,0 +1,146 @@
		import { app, ipcMain, Menu, MenuItem, nativeImage, Tray } from "electron"
		import log from 'electron-log'

Error tooltips and logging 🤕 #32

Error tooltips and logging 🤕 #32

Conversation

bengreenier commented Oct 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bengreenier commented Oct 2, 2018 •

edited

Loading