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.

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

[Fixes #269] get rid of rollbar wrapper stack frames #335

Merged
merged 2 commits into from
Jul 14, 2017
Merged

Conversation

rokob
Copy link
Contributor

@rokob rokob commented Jul 11, 2017

We wrap certain built-ins in the browser and rethrow exceptions. When such an error occurs, our wrapper ends up in the stack trace because technically it is running, but it isn't really part of the error so there is no reason for it to be in the stack trace.

There are a few different ways I looked at fixing this, but in the end just checking for our f._wrapped method signature in the frame is the simplest and actually works about as well as any other approach. Threading state around is a bit of a mess, and actually munging the exception at the rethrow site is impossible due to browser differences.

@@ -172,6 +172,9 @@ function addBodyTrace(item, options, callback) {
method: (!stackFrame.func || stackFrame.func === '?') ? '[anonymous]' : stackFrame.func,
colno: stackFrame.column
};
if (frame.method && frame.method.indexOf('f._wrapped') != -1) {
Copy link
Member

Choose a reason for hiding this comment

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

This seems fragile - it might break if:

  • this library changes, so the minified frame is no longer called f._wrapped
  • this library is included in a larger application build, and the minified name there is something else

How about if we change the _wrapped part to be more identifiable? e.g. _rollbar_wrapped -- and then we can just check that the method name ends with the exact string ._rollbar_wrapped

@brianr
Copy link
Member

brianr commented Jul 11, 2017

👍

Will this require people to update their snippet?

@rokob
Copy link
Contributor Author

rokob commented Jul 11, 2017

Yeah, the snippet will change

@rokob rokob merged commit 2dc8eea into master Jul 14, 2017
@rokob rokob deleted the event-wrapper branch February 1, 2018 23:02
mudetroit pushed a commit that referenced this pull request Mar 14, 2024
[Fixes #269] get rid of rollbar wrapper stack frames
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants