fix(rum-core): capture stack trace from syntax errors properly (#1239)

elastic · Jun 14, 2022 · 4081941 · 4081941
1 parent c83c586
commit 4081941
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 6 deletions.
diff --git a/packages/rum-core/src/error-logging/error-logging.js b/packages/rum-core/src/error-logging/error-logging.js
@@ -27,6 +27,7 @@ import { createStackTraces, filterInvalidFrames } from './stack-trace'
 import { generateRandomId, merge, extend } from '../common/utils'
 import { getPageContext } from '../common/context'
 import { truncateModel, ERROR_MODEL } from '../common/truncate'
+import stackParser from 'error-stack-parser'
 
 /**
  * List of keys to be ignored from getting added to custom error properties
@@ -76,7 +77,7 @@ class ErrorLogging {
    * errorEvent = { message, filename, lineno, colno, error }
    */
   createErrorDataModel(errorEvent) {
-    const frames = createStackTraces(errorEvent)
+    const frames = createStackTraces(stackParser, errorEvent)
     const filteredFrames = filterInvalidFrames(frames)
 
     // If filename empty, assume inline script

diff --git a/packages/rum-core/src/error-logging/stack-trace.js b/packages/rum-core/src/error-logging/stack-trace.js
@@ -23,8 +23,6 @@
  *
  */
 
-import stackParser from 'error-stack-parser'
-
 function filePathToFileName(fileUrl) {
   const origin =
     window.location.origin ||
@@ -91,7 +89,21 @@ function normalizeFunctionName(fnName) {
   return fnName
 }
 
-export function createStackTraces(errorEvent) {
+function isValidStackTrace(stackTraces) {
+  if (stackTraces.length === 0) {
+    return false
+  }
+
+  if (stackTraces.length === 1) {
+    const stackTrace = stackTraces[0]
+
+    return 'lineNumber' in stackTrace
+  }
+
+  return true
+}
+
+export function createStackTraces(stackParser, errorEvent) {
   const { error, filename, lineno, colno } = errorEvent
 
   let stackTraces = []
@@ -106,7 +118,11 @@ export function createStackTraces(errorEvent) {
     }
   }
 
-  if (stackTraces.length === 0) {
+  // error-stack-parser library doesn't generate proper stack traces from Syntax Errors
+  // thrown during the browser document parsing process. The outcome of the library depends on the browser:
+  // 1. on non-chromium browsers it throws an exception.
+  // 2. on chromium browsers it returns a malformed single stack trace element not containing the lineNumber property
+  if (!isValidStackTrace(stackTraces)) {
     stackTraces = [
       {
         fileName: filename,

diff --git a/packages/rum-core/test/error-logging/stack-trace.spec.js b/packages/rum-core/test/error-logging/stack-trace.spec.js
@@ -24,6 +24,7 @@
  */
 
 import { createStackTraces } from '../../src/error-logging/stack-trace'
+import stackParser from 'error-stack-parser'
 
 describe('StackTraceService', function () {
   it('should produce correct number of frames', function (done) {
@@ -34,10 +35,56 @@ describe('StackTraceService', function () {
       try {
         generateError()
       } catch (error) {
-        var stackTraces = createStackTraces({ error })
+        var stackTraces = createStackTraces(stackParser, { error })
         expect(stackTraces.length).toBeGreaterThan(1)
         done()
       }
     }, 1)
   })
+
+  describe('integration error-stack-parser library', () => {
+    it('should generate stack trace from errorEvent when stackParser throws an error parsing it', () => {
+      const stackParserFake = {
+        parse: () => {
+          throw new Error()
+        }
+      }
+
+      const testErrorEvent = new Error()
+      testErrorEvent.error = 'error event'
+      testErrorEvent.lineno = 1
+      testErrorEvent.colno = 30
+      testErrorEvent.filename = '(inline script)'
+
+      const [stackTrace] = createStackTraces(stackParserFake, testErrorEvent)
+
+      expect(stackTrace.lineno).toBe(testErrorEvent.lineno)
+      expect(stackTrace.colno).toBe(testErrorEvent.colno)
+      expect(stackTrace.filename).toBe(testErrorEvent.filename)
+    })
+
+    it('should generate stack trace from errorEvent when the one returned from stackParser does not contain lineNumber', () => {
+      const stackParserFake = {
+        parse: () => {
+          return [
+            {
+              filename: 'malformed parsing filename'
+            }
+          ]
+        }
+      }
+
+      const testErrorEvent = new Error()
+      testErrorEvent.error = 'error event'
+      testErrorEvent.lineno = 4
+      testErrorEvent.colno = 23
+      testErrorEvent.filename = '(inline script)'
+
+      const [stackTrace] = createStackTraces(stackParserFake, testErrorEvent)
+
+      expect(stackTrace.lineno).toBe(testErrorEvent.lineno)
+      expect(stackTrace.colno).toBe(testErrorEvent.colno)
+      expect(stackTrace.filename).toBe(testErrorEvent.filename)
+    })
+  })
 })