feat: Don't process exit on oncaught error (#93)

* feat: Don't process exit on oncaught error

* fix: Don't show dialog

CHANGELOG.md

@@ -1,5 +1,26 @@
 # Changelog
 
+## v0.7.0
+
+**Breaking Changes**:
+
+* We no longer exit your app by default. Before `0.7.0` we would call
+  `process.exit(1)` after we caught and uncaught error. As of version `0.7.0` we
+  no longer change electrons default behaviour which is showing a dialog with
+  the error and keep the app alive. If you want to exit the app or do something
+  else if a global uncaught error happens you have to set the `onFatalError`
+  option in init like (we will send the error to Sentry before this is called):
+
+  ```javascript
+  init({
+    dsn: 'DSN',
+    onFatalError: error => {
+      // I really want to crash
+      process.exit(1);
+    },
+  });
+  ```
+
 ## v0.6.0
 
 **Breaking Changes**:

docs/javascript.rst

@@ -188,11 +188,10 @@ Uncaught Exceptions
 The default behavior for dealing with globally unhandled exceptions and Promise
 rejections differs by process.
 
-In the main process, such unhandled errors will cause the app to exit
-immediatly. This is in accordance with the strategy recommended by Node. By
-default, Sentry will capture these events and send them to Sentry prior to
-exiting. To override this behavior, declare a custom ``onFatalError`` callback
-when configuring the SDK:
+In the main process, such unhandled errors Sentry will capture before showing the
+default dialog electron always shows. To override this behavior, declare a
+custom ``onFatalError`` callback when configuring the SDK, for example if you
+want your app to exit do:
 
 .. code-block:: javascript
 

src/main/integrations/onuncaughtexception.ts

@@ -1,5 +1,9 @@
-import { getDefaultHub, Handlers, NodeClient } from '@sentry/node';
+import { getDefaultHub, NodeClient } from '@sentry/node';
 import { Integration, SentryEvent, Severity } from '@sentry/types';
+import {
+  dialog,
+  // tslint:disable-next-line:no-implicit-dependencies
+} from 'electron';
 
 /** Capture unhandled erros. */
 export class OnUncaughtException implements Integration {
@@ -34,7 +38,16 @@ export class OnUncaughtException implements Integration {
         if (this.options.onFatalError) {
           this.options.onFatalError(error);
         } else {
-          Handlers.defaultOnFatalError(error);
+          console.error('Uncaught Exception:');
+          console.error(error);
+          const ref = error.stack;
+          const stack =
+            ref !== undefined ? ref : `${error.name}: ${error.message}`;
+          const message = `Uncaught Exception:\n${stack}`;
+          dialog.showErrorBox(
+            'A JavaScript error occurred in the main process',
+            message,
+          );
         }
       });
     });

test/e2e/index.test.ts

@@ -64,14 +64,6 @@ tests.forEach(([version, arch]) => {
       const event = context.testServer.events[0];
       const breadcrumbs = event.data.breadcrumbs || [];
       const lastFrame = getLastFrame(event.data);
-      // wait for the main process to exit (default behavior)
-      await context.waitForTrue(
-        async () =>
-          context.mainProcess
-            ? !(await context.mainProcess.isRunning())
-            : false,
-        'Timeout: Waiting for app to die',
-      );
 
       expect(context.testServer.events.length).to.equal(1);
       expect(lastFrame.filename).to.equal('app:///fixtures/javascript-main.js');
@@ -86,14 +78,6 @@ tests.forEach(([version, arch]) => {
       const event = context.testServer.events[0];
       const breadcrumbs = event.data.breadcrumbs || [];
       const lastFrame = getLastFrame(event.data);
-      // wait for the main process to exit (default behavior)
-      await context.waitForTrue(
-        async () =>
-          context.mainProcess
-            ? !(await context.mainProcess.isRunning())
-            : false,
-        'Timeout: Waiting for app to die',
-      );
 
       expect(context.testServer.events.length).to.equal(1);
       expect(lastFrame.filename).to.equal(
@@ -105,7 +89,7 @@ tests.forEach(([version, arch]) => {
     });
 
     it('onFatalError can be overridden', async () => {
-      await context.start('sentry-onfatal-dont-exit', 'javascript-main');
+      await context.start('sentry-onfatal-exit', 'javascript-main');
       await context.waitForEvents(1);
       const event = context.testServer.events[0];
       const breadcrumbs = event.data.breadcrumbs || [];
@@ -117,11 +101,12 @@ tests.forEach(([version, arch]) => {
       expect(event.sentry_key).to.equal(SENTRY_KEY);
       expect(breadcrumbs.length).to.greaterThan(4);
 
-      // Ensure the main process is still alive
       await context.waitForTrue(
         async () =>
-          context.mainProcess ? context.mainProcess.isRunning() : false,
-        'Timeout: Ensure app is still alive',
+          context.mainProcess
+            ? !(await context.mainProcess.isRunning())
+            : false,
+        'Timeout: Waiting for app to die',
       );
     });
 

test/e2e/test-app/fixtures/sentry-basic.js

@@ -2,4 +2,7 @@ const { init } = require('../../../../');
 
 init({
   dsn: process.env.DSN,
+  onFatalError: error => {
+    // We need this here otherwise we will get a dialog and travis will be stuck
+  },
 });

test/e2e/test-app/fixtures/sentry-onfatal-dont-exit.js → test/e2e/test-app/fixtures/sentry-onfatal-exit.js

@@ -4,6 +4,6 @@ const { app } = require('electron');
 init({
   dsn: process.env.DSN,
   onFatalError: error => {
-    // Do nothing
+    process.exit(1);
   },
 });