Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tolerate stacktrace generation error when in strict mode #584

Merged
merged 4 commits into from
Jul 26, 2019
Merged

Tolerate stacktrace generation error when in strict mode #584

merged 4 commits into from
Jul 26, 2019

Conversation

bengourley
Copy link
Contributor

When bundled inside a script that's running in strict mode, StackGenerator will throw an error. There's no better way to detect script mode than try to do what you need, and catch the error so that's what this PR does.

This adds an end-to-end test that confirms an error without a stack is correctly sent from strict mode.

Fixes #564.

@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Jul 12, 2019 •
edited
Loading

@bugsnag/browser bundle size diff

Minified Minfied + Gzipped
Before 40.79 kB 12.19 kB
After 40.82 kB 12.20 kB
± ⚠️ +33 bytes ⚠️ +7 bytes

Generated by 🚫 dangerJS against 9a70e46

@bengourley
Copy link
Contributor Author

bengourley commented Jul 15, 2019 •
edited
Loading

Here's the easiest way to test this out…

Firstly, verify that the current version of Bugsnag fails attempting to generate a stacktrace for a stack-less error in strict mode:

  1. clone the repo and checkout master or next (something that doesn't have this fix.

  2. install dependencies and build the notifier npm i && npm run boostrap && npm run build

  3. make a temporary directory mkdir -p packages/browser/scratch and insert the following HTML into packages/browser/scratch/strict.html

    <!DOCTYPE html>
<html>
  <head>
    <script type="module" src="../dist/bugsnag.min.js"></script>
    <script type="module">window.bugsnagClient = window.bugsnag('API_KEY')</script>
  </head>
  <body>
    <script type="module">
      console.log(bugsnagClient.BugsnagReport.getStacktrace(123))
    </script>
  </body>
</html>

  4. start a local webserver for the browser package npx serve packages/browser

  5. go to http://localhost:5000/scratch/strict.html and verify that you see the message TypeError: 'caller', 'callee', and 'arguments' properties may not be accessed on strict mode functions or the arguments objects for calls to them

Then to verify that this branch fixes the problem:

  1. checkout this branch git checkout tolerate-strict-mode
  2. go to http://localhost:5000/scratch/strict.html and verify that you see [] logged out and not the error message you saw before

@fractalwrench fractalwrench self-requested a review July 26, 2019 09:40
fractalwrench
fractalwrench approved these changes Jul 26, 2019
View reviewed changes
Copy link
Contributor

@fractalwrench fractalwrench left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with these changes based on the understanding that it's not feasible to collect a useful stacktrace using some alternative mechanism when Strict Mode is enabled.

@bengourley bengourley merged commit f695530 into next Jul 26, 2019
@bengourley bengourley deleted the tolerate-strict-mode branch July 26, 2019 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fractalwrench fractalwrench fractalwrench approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error 'caller', 'callee', and 'arguments' properties may not be accessed on strict mode
3 participants
@bengourley @bugsnagbot @fractalwrench