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

devcorpio · 2022-06-07T14:56:47Z

Context

The RUM agent was capturing errors thrown in inline scripts with an empty stack trace. There is a nuance to highlight, though.

This only affects to errors whose type is SyntaxError and have been triggered because of a malformed JavaScript code. This tend to occur when the browser is parsing the document.

The following two examples will help to understand that nuance:

Example 1 (KO): SyntaxError because of malformed JavaScript while parsing the document:

<html>
  <head>Example 1: SyntaxError because of malformed JavaScript while parsing the document </head>
  <body>
     Hello, I am Example 1 page.
    <script>
         var a:      // This throws a Syntax Error
    </script>
  </body>
</html>

As we can see on the screenshot below the stacktrace was empty:

Example 2 (OK): SyntaxError because of bad usage of JSON.parse

<html>
  <head>Example 2: SyntaxError because of bad usage of JSON.parse </head>
  <body>
     Hello, I am Example 2 page.
    <script>
         JSON.parse("bad json") // this will throw a SyntaxError too
    </script>
  </body>
</html>

As we can see on the screenshot below the stack trace was not empty, since this kind of SyntaxError was being handled properly:

How are we generating stacktraces?

One of the steps of the error capturing flow is the one that generates the stacktrace from an error event. Given the complexity that some cases have (cross browsing support for instance) we rely on a third-party library named error-stack-parser. Unfortunately, that case is not being handled as expected.

Fix

The ErrorEvent that the browser dispatches when an uncaught error takes place contains all the information we need to generate the stacktrace. So, that's what we do now, we create a stack trace of a single element which contains the missing information.

After the fix, the stacktrace of example 1 looks this way: 🎉

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2022-06-07T14:57:00.526+0000
Duration: 90 min 51 sec

Test stats 🧪

Test	Results
Failed	0
Passed	4730
Skipped	62
Total	4792

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
run benchmark tests : Run the benchmark test.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

apmmachine · 2022-06-07T16:27:08Z

🌐 Coverage report

Name	Metrics % (`covered/total`)	Diff
Packages	100.0% (`8/8`)	💚
Files	98.148% (`53/54`)	👍 0.109
Classes	98.148% (`53/54`)	👍 0.109
Methods	97.406% (`413/424`)	👍 0.056
Lines	95.296% (`2107/2211`)	👎 -0.03
Conditionals	86.337% (`1049/1215`)	👎 -0.099

kyungeunni

LGTM!

kyungeunni · 2022-06-13T10:30:44Z

packages/rum-core/src/error-logging/stack-trace.js

+  return true
+}
+
+export function createStackTraces(stackParser, errorEvent) {


Is it correct that we are now passing stackParser here in order to mock this library when testing? :)

yes, that's right! 🎉 . In this particular scenario the option of injecting the dependency was the simplest strategy for making the testing experience smooth.

Any reason why you didnt go with mocking error-stack-parser using spy instead?

Unfortunately, with jasmine we cannot spy properly a third-party (node_modules one) dependency. Not with our project setup at least 😢

vigneshshanmugam

LGTM 🎉 Good catch Alberto.

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

20414d4

devcorpio linked an issue Jun 7, 2022 that may be closed by this pull request

Inline scripts errors does not have any references. #1140

Closed

devcorpio requested review from vigneshshanmugam and kyungeunni June 7, 2022 14:58

kyungeunni approved these changes Jun 13, 2022

View reviewed changes

vigneshshanmugam approved these changes Jun 14, 2022

View reviewed changes

devcorpio merged commit 4081941 into elastic:main Jun 14, 2022

devcorpio deleted the inline_error branch June 14, 2022 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

devcorpio commented Jun 7, 2022 •

edited

Loading

apmmachine commented Jun 7, 2022

apmmachine commented Jun 7, 2022

Build stats

Test stats 🧪

apmmachine commented Jun 7, 2022

kyungeunni left a comment

kyungeunni Jun 13, 2022

devcorpio Jun 13, 2022

vigneshshanmugam Jun 14, 2022

devcorpio Jun 14, 2022 •

edited

Loading

vigneshshanmugam left a comment

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

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

Conversation

devcorpio commented Jun 7, 2022 • edited Loading

Context

How are we generating stacktraces?

Fix

Next

apmmachine commented Jun 7, 2022

📦 Bundlesize report

apmmachine commented Jun 7, 2022

💚 Build Succeeded

Build stats

Test stats 🧪

🤖 GitHub comments

apmmachine commented Jun 7, 2022

🌐 Coverage report

kyungeunni left a comment

Choose a reason for hiding this comment

kyungeunni Jun 13, 2022

Choose a reason for hiding this comment

devcorpio Jun 13, 2022

Choose a reason for hiding this comment

vigneshshanmugam Jun 14, 2022

Choose a reason for hiding this comment

devcorpio Jun 14, 2022 • edited Loading

Choose a reason for hiding this comment

vigneshshanmugam left a comment

Choose a reason for hiding this comment

devcorpio commented Jun 7, 2022 •

edited

Loading

devcorpio Jun 14, 2022 •

edited

Loading