fix(common): CHECKOUT-4418 Discard the error event if none of the fra…

…mes in its stack trace contains a file name
bigcommerce · Oct 10, 2019 · 4fb1112 · 4fb1112
1 parent 5cf02db
commit 4fb1112
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 4 deletions.
diff --git a/src/app/common/error/SentryErrorLogger.spec.ts b/src/app/common/error/SentryErrorLogger.spec.ts
@@ -128,7 +128,7 @@ describe('SentryErrorLogger', () => {
             .toEqual(null);
     });
 
-    it('does not log exeception event if it does not contain stacktrace', () => {
+    it('does not log exception event if it does not contain stacktrace', () => {
         // tslint:disable-next-line:no-unused-expression
         new SentryErrorLogger(config);
 
@@ -143,6 +143,48 @@ describe('SentryErrorLogger', () => {
             .toEqual(null);
     });
 
+    it('does not log exception event if all frames in stacktrace are missing filename', () => {
+        // tslint:disable-next-line:no-unused-expression
+        new SentryErrorLogger(config);
+
+        const clientOptions: BrowserOptions = (init as jest.Mock).mock.calls[0][0];
+        const event = {
+            exception: {
+                values: [{
+                    stacktrace: { frames: [{ filename: '' }] },
+                    type: 'Error',
+                    value: 'Unexpected error',
+                }],
+            },
+        };
+        const hint = { originalException: new Error('Unexpected error') };
+
+        // tslint:disable-next-line:no-non-null-assertion
+        expect(clientOptions.beforeSend!(event, hint))
+            .toEqual(null);
+    });
+
+    it('logs exception event if some frames in stacktrace contain filename', () => {
+        // tslint:disable-next-line:no-unused-expression
+        new SentryErrorLogger(config);
+
+        const clientOptions: BrowserOptions = (init as jest.Mock).mock.calls[0][0];
+        const event = {
+            exception: {
+                values: [{
+                    stacktrace: { frames: [{ filename: '' }, { filename: 'js/app-123.js' }] },
+                    type: 'Error',
+                    value: 'Unexpected error',
+                }],
+            },
+        };
+        const hint = { originalException: new Error('Unexpected error') };
+
+        // tslint:disable-next-line:no-non-null-assertion
+        expect(clientOptions.beforeSend!(event, hint))
+            .toEqual(event);
+    });
+
     it('configures client to rewrite filename of error frames', () => {
         // tslint:disable-next-line:no-unused-expression
         new SentryErrorLogger(config, { publicPath: 'https://cdn.foo.bar' });

diff --git a/src/app/common/error/SentryErrorLogger.ts b/src/app/common/error/SentryErrorLogger.ts
@@ -1,7 +1,7 @@
 import { captureException, init, withScope, BrowserOptions, Event, Integrations, Severity, StackFrame } from '@sentry/browser';
 import { RewriteFrames } from '@sentry/integrations';
-import { EventHint } from '@sentry/types';
-import { every, includes, isEmpty } from 'lodash';
+import { EventHint, Exception } from '@sentry/types';
+import { every, includes, isEmpty, some } from 'lodash';
 
 import computeErrorCode from './computeErrorCode';
 import DEFAULT_ERROR_TYPES from './defaultErrorTypes';
@@ -93,6 +93,24 @@ export default class SentryErrorLogger implements ErrorLogger {
         }
     }
 
+    private hasUsefulStacktrace(exceptions: Exception[]): boolean {
+        return some(exceptions, exception => {
+            if (!exception.stacktrace) {
+                return false;
+            }
+
+            if (isEmpty(exception.stacktrace.frames)) {
+                return false;
+            }
+
+            if (every(exception.stacktrace.frames, frame => !frame.filename)) {
+                return false;
+            }
+
+            return true;
+        });
+    }
+
     private handleBeforeSend: (event: Event, hint?: EventHint) => Event | null = (event, hint) => {
         if (event.exception) {
             const { originalException = null } = hint || {};
@@ -101,7 +119,7 @@ export default class SentryErrorLogger implements ErrorLogger {
                 return null;
             }
 
-            if (every(event.exception.values, value => !value.stacktrace || isEmpty(value.stacktrace.frames))) {
+            if (!event.exception.values || !this.hasUsefulStacktrace(event.exception.values)) {
                 return null;
             }