From a9af3c2dcbf82edcd2f258aef355d11303af82ca Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Tue, 17 Oct 2017 00:54:56 +0900 Subject: [PATCH 1/8] Show uncaughtException as an internal error and quit the app --- src/main.js | 23 +++++++----- src/main/CriticalErrorHandler.js | 64 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 src/main/CriticalErrorHandler.js diff --git a/src/main.js b/src/main.js index 4dbd0f330d7..357d9c2b3f1 100644 --- a/src/main.js +++ b/src/main.js @@ -10,15 +10,18 @@ const { systemPreferences, session } = require('electron'); +const os = require('os'); +const path = require('path'); const isDev = require('electron-is-dev'); const installExtension = require('electron-devtools-installer'); const squirrelStartup = require('./main/squirrelStartup'); +const CriticalErrorHandler = require('./main/CriticalErrorHandler'); const protocols = require('../electron-builder.json').protocols; -process.on('uncaughtException', (error) => { - console.error(error); -}); +const criticalErrorHandler = new CriticalErrorHandler(); + +process.on('uncaughtException', criticalErrorHandler.processUncaughtExceptionHandler.bind(criticalErrorHandler)); global.willAppQuit = false; @@ -27,9 +30,6 @@ if (squirrelStartup()) { global.willAppQuit = true; } -const os = require('os'); -const path = require('path'); - var settings = require('./common/settings'); var certificateStore = require('./main/certificateStore').load(path.resolve(app.getPath('userData'), 'certificate.json')); const {createMainWindow} = require('./main/mainWindow'); @@ -304,6 +304,10 @@ app.on('certificate-error', (event, webContents, url, error, certificate, callba } }); +app.on('gpu-process-crashed', () => { + throw new Error('The GPU process has crached'); +}); + const loginCallbackMap = new Map(); ipcMain.on('login-credentials', (event, request, user, password) => { @@ -392,11 +396,10 @@ app.on('ready', () => { // when you should delete the corresponding element. mainWindow = null; }); - mainWindow.on('unresponsive', () => { - console.log('The application has become unresponsive.'); - }); + criticalErrorHandler.setMainWindow(mainWindow); + mainWindow.on('unresponsive', criticalErrorHandler.windowUnresponsiveHandler.bind(criticalErrorHandler)); mainWindow.webContents.on('crashed', () => { - console.log('The application has crashed.'); + throw new Error('webContents \'crashed\' event has been emitted'); }); ipcMain.on('notified', () => { diff --git a/src/main/CriticalErrorHandler.js b/src/main/CriticalErrorHandler.js new file mode 100644 index 00000000000..54965f526b0 --- /dev/null +++ b/src/main/CriticalErrorHandler.js @@ -0,0 +1,64 @@ +const {app, dialog, shell} = require('electron'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +function createErrorReport(err) { + return `Application: ${app.getName()} ${app.getVersion()}\n` + + `Platform: ${os.type()} ${os.release()} ${os.arch()}\n` + + `${err.stack}`; +} + +function bindWindowToShowMessageBox(win) { + if (win && win.isVisible()) { + return dialog.showMessageBox.bind(null, win); + } + return dialog.showMessageBox; +} + +class CriticalErrorHandler { + constructor() { + this.mainWindow = null; + } + + setMainWindow(mainWindow) { + this.mainWindow = mainWindow; + } + + windowUnresponsiveHandler() { + const result = dialog.showMessageBox(this.mainWindow, { + type: 'warning', + title: `Unresponsive - ${app.getName()}`, + message: 'The window is no longer responsive.\nDo you wait until the window becomes responsive again?', + buttons: ['No', 'Yes'], + defaultId: 0 + }); + if (result === 0) { + throw new Error('BrowserWindow \'unresponsive\' event has been emitted'); + } + } + + processUncaughtExceptionHandler(err) { + const file = path.join(app.getPath('userData'), `uncaughtException-${Date.now()}.txt`); + const report = createErrorReport(err); + fs.writeFileSync(file, report.replace(new RegExp('\\n', 'g'), os.EOL)); + fs.writeSync(2, `See "${file}" to report the problem.\n`); + + if (app.isReady()) { + const showMessageBox = bindWindowToShowMessageBox(this.mainWindow); + const result = showMessageBox({ + type: 'error', + title: `Error - ${app.getName()}`, + message: `An internal error has occurred: ${err.message}\nThe application will quit.`, + buttons: ['OK', 'Show detail'], + defaultId: 0 + }); + if (result === 1) { + shell.openExternal(file); + } + } + throw err; + } +} + +module.exports = CriticalErrorHandler; From 02c63395b59518ed30521fdcf44b27bdac99075f Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Fri, 20 Oct 2017 22:08:47 +0900 Subject: [PATCH 2/8] Fix error case where tray icon is already destroyed --- src/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.js b/src/main.js index 357d9c2b3f1..cff5aa9fc18 100644 --- a/src/main.js +++ b/src/main.js @@ -471,7 +471,7 @@ app.on('ready', () => { } } - if (trayIcon) { + if (trayIcon && !trayIcon.isDestroyed()) { if (arg.mentionCount > 0) { trayIcon.setImage(trayImages.mention); if (process.platform === 'darwin') { From 00b7a806adccab3ff2152062d9886c1026873d85 Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Tue, 24 Oct 2017 22:21:53 +0900 Subject: [PATCH 3/8] Open the error report in detached process --- src/main/CriticalErrorHandler.js | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/main/CriticalErrorHandler.js b/src/main/CriticalErrorHandler.js index 54965f526b0..383ae8be880 100644 --- a/src/main/CriticalErrorHandler.js +++ b/src/main/CriticalErrorHandler.js @@ -1,4 +1,5 @@ -const {app, dialog, shell} = require('electron'); +const {app, dialog} = require('electron'); +const {spawn} = require('child_process'); const fs = require('fs'); const os = require('os'); const path = require('path'); @@ -9,6 +10,20 @@ function createErrorReport(err) { `${err.stack}`; } +function openDetachedExternal(url) { + const spawnOption = {detached: true, stdio: 'ignore'}; + switch (process.platform) { + case 'win32': + return spawn('cmd', ['/C', 'start', url], spawnOption); + case 'darwin': + return spawn('open', [url], spawnOption); + case 'linux': + return spawn('xdg-open', [url], spawnOption); + default: + return null; + } +} + function bindWindowToShowMessageBox(win) { if (win && win.isVisible()) { return dialog.showMessageBox.bind(null, win); @@ -54,7 +69,13 @@ class CriticalErrorHandler { defaultId: 0 }); if (result === 1) { - shell.openExternal(file); + const child = openDetachedExternal(file); + if (child) { + child.on('error', (spawnError) => { + console.log(spawnError); + }); + child.unref(); + } } } throw err; From 0e1e4228f01ea7ca19753e8fb9efb514d9fe4a19 Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Thu, 2 Nov 2017 00:30:19 +0900 Subject: [PATCH 4/8] Update error dialog style --- src/main/CriticalErrorHandler.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/CriticalErrorHandler.js b/src/main/CriticalErrorHandler.js index 383ae8be880..1bb5b8078d4 100644 --- a/src/main/CriticalErrorHandler.js +++ b/src/main/CriticalErrorHandler.js @@ -43,7 +43,7 @@ class CriticalErrorHandler { windowUnresponsiveHandler() { const result = dialog.showMessageBox(this.mainWindow, { type: 'warning', - title: `Unresponsive - ${app.getName()}`, + title: app.getName(), message: 'The window is no longer responsive.\nDo you wait until the window becomes responsive again?', buttons: ['No', 'Yes'], defaultId: 0 @@ -63,12 +63,13 @@ class CriticalErrorHandler { const showMessageBox = bindWindowToShowMessageBox(this.mainWindow); const result = showMessageBox({ type: 'error', - title: `Error - ${app.getName()}`, - message: `An internal error has occurred: ${err.message}\nThe application will quit.`, - buttons: ['OK', 'Show detail'], - defaultId: 0 + title: app.getName(), + message: `The ${app.getName()} app quit unexpectedly. Click "Show Details" to learn more.\n\n Internal error: ${err.message}`, + buttons: ['Show details', 'OK'], + defaultId: 1, + noLink: true }); - if (result === 1) { + if (result === 0) { const child = openDetachedExternal(file); if (child) { child.on('error', (spawnError) => { From 19ced85bc88b2caac3366a3380b534b769e9aa93 Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Thu, 2 Nov 2017 00:57:51 +0900 Subject: [PATCH 5/8] Use app.exit() instead of app.quit() in makeSingleInstance --- src/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.js b/src/main.js index cff5aa9fc18..9a178a63acb 100644 --- a/src/main.js +++ b/src/main.js @@ -172,7 +172,7 @@ if (app.makeSingleInstance((commandLine/*, workingDirectory*/) => { } } })) { - app.quit(); + app.exit(); } function shouldShowTrayIcon() { From f794ac729e382fc7f80dee65f773c61b7d2241f7 Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Sun, 5 Nov 2017 21:02:15 +0900 Subject: [PATCH 6/8] Remove fs.writeFileSync(2) to avoid crashing on Windows --- src/main/CriticalErrorHandler.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/CriticalErrorHandler.js b/src/main/CriticalErrorHandler.js index 1bb5b8078d4..747ada517ab 100644 --- a/src/main/CriticalErrorHandler.js +++ b/src/main/CriticalErrorHandler.js @@ -57,7 +57,6 @@ class CriticalErrorHandler { const file = path.join(app.getPath('userData'), `uncaughtException-${Date.now()}.txt`); const report = createErrorReport(err); fs.writeFileSync(file, report.replace(new RegExp('\\n', 'g'), os.EOL)); - fs.writeSync(2, `See "${file}" to report the problem.\n`); if (app.isReady()) { const showMessageBox = bindWindowToShowMessageBox(this.mainWindow); From f9cb2370ac7fdcfc835d8190a0c7a2f6d056c73a Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Mon, 6 Nov 2017 23:03:57 +0900 Subject: [PATCH 7/8] Relaunch the app by Reopen button and tweak for macOS --- src/main/CriticalErrorHandler.js | 35 +++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/main/CriticalErrorHandler.js b/src/main/CriticalErrorHandler.js index 747ada517ab..9d76792529f 100644 --- a/src/main/CriticalErrorHandler.js +++ b/src/main/CriticalErrorHandler.js @@ -4,6 +4,10 @@ const fs = require('fs'); const os = require('os'); const path = require('path'); +const BUTTON_OK = 'OK'; +const BUTTON_SHOW_DETAILS = 'Show Details'; +const BUTTON_REOPEN = 'Reopen'; + function createErrorReport(err) { return `Application: ${app.getName()} ${app.getVersion()}\n` + `Platform: ${os.type()} ${os.release()} ${os.arch()}\n` + @@ -59,23 +63,34 @@ class CriticalErrorHandler { fs.writeFileSync(file, report.replace(new RegExp('\\n', 'g'), os.EOL)); if (app.isReady()) { + const buttons = [BUTTON_SHOW_DETAILS, BUTTON_OK, BUTTON_REOPEN]; + if (process.platform === 'darwin') { + buttons.reverse(); + } const showMessageBox = bindWindowToShowMessageBox(this.mainWindow); const result = showMessageBox({ type: 'error', title: app.getName(), - message: `The ${app.getName()} app quit unexpectedly. Click "Show Details" to learn more.\n\n Internal error: ${err.message}`, - buttons: ['Show details', 'OK'], - defaultId: 1, + message: `The ${app.getName()} app quit unexpectedly. Click "Show Details" to learn more or "Reopen" to open the application again.\n\n Internal error: ${err.message}`, + buttons, + defaultId: buttons.indexOf(BUTTON_REOPEN), noLink: true }); - if (result === 0) { - const child = openDetachedExternal(file); - if (child) { - child.on('error', (spawnError) => { - console.log(spawnError); - }); - child.unref(); + switch (result) { + case buttons.indexOf(BUTTON_SHOW_DETAILS): + { + const child = openDetachedExternal(file); + if (child) { + child.on('error', (spawnError) => { + console.log(spawnError); + }); + child.unref(); + } } + break; + case buttons.indexOf(BUTTON_REOPEN): + app.relaunch(); + break; } } throw err; From 94a4b968b95b33d08a284167c1cbef8ab646d15e Mon Sep 17 00:00:00 2001 From: Yuya Ochiai Date: Tue, 7 Nov 2017 01:09:52 +0900 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4665d19b9e0..cf88d32abde 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Release date: TBD #### All Platforms - Now "Saved" indicators appear for each preferences section. [#500](https://github.com/mattermost/desktop/issues/500) + - Added the dialog to reopen the application when it quits unexpectedly. + [#626](https://github.com/mattermost/desktop/pull/626) #### Windows - Added the feature to open the application via `mattermost://` link.