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

Fix RN frames on Android ... #875

Merged
merged 1 commit into from
Mar 23, 2017
Merged

Fix RN frames on Android ... #875

merged 1 commit into from
Mar 23, 2017

Conversation

benvinegar
Copy link
Contributor

@benvinegar benvinegar commented Mar 1, 2017

  1. Don't treat presence of "native" string everywhere as meaning "native code"
  2. Properly parse when not protocol / only leading forward slash

fixes #731

See also: csnover/TraceKit#69

cc @getsentry/javascript

1. Don't treat presence of "native" string everywhere as meaning "native code"
2. Properly parse when not protocol / only leading forward slash

fixes #731
@@ -396,7 +396,7 @@ TraceKit.computeStackTrace = (function computeStackTraceWrapper() {

for (var i = 0, j = lines.length; i < j; ++i) {
if ((parts = chrome.exec(lines[i]))) {
var isNative = parts[2] && parts[2].indexOf('native') !== -1;
var isNative = parts[2] && parts[2].indexOf('native') === 0; // start of line
Copy link
Contributor Author

@benvinegar benvinegar Mar 1, 2017

Choose a reason for hiding this comment

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

This is covered by the CHROME_48_BLOB test:

https://github.com/getsentry/raven-js/blob/master/test/vendor/fixtures/captured-errors.js#L327

I've also verified the format is the same in my Chrome 56 (on the console):

setTimeout(function callback() {
  [1].forEach(function iterate() { oops(); });
});

Emits:

Uncaught ReferenceError: oops is not defined
    at iterate (<anonymous>:2:36)
    at Array.forEach (native) # <-- bingo
    at callback (<anonymous>:2:7)

@benvinegar benvinegar requested a review from MaxBittker March 2, 2017 00:17
Copy link
Contributor

@MaxBittker MaxBittker left a comment

Choose a reason for hiding this comment

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

the test looks good to me and it seems to address the issue (don't have a android react native environment to confirm)

@benvinegar benvinegar merged commit f162c9e into master Mar 23, 2017
@benvinegar benvinegar deleted the fix-rn-android branch March 23, 2017 00:32
@f0rr0
Copy link

f0rr0 commented Mar 23, 2017

@benvinegar @MaxBittker when will this be released?

@benvinegar
Copy link
Contributor Author

I could try and cut 3.13.2 tomorrow morning.

@f0rr0
Copy link

f0rr0 commented Mar 23, 2017

@benvinegar that'll be awesome thanks

@f0rr0
Copy link

f0rr0 commented Mar 24, 2017

@benvinegar any word on that release?

@f0rr0
Copy link

f0rr0 commented Mar 31, 2017

@benvinegar ??

@benvinegar
Copy link
Contributor Author

@f0rr0 – sorry, have been busy. It's not as easy as hitting a button, alas.

@f0rr0
Copy link

f0rr0 commented Mar 31, 2017

@benvinegar sorry for bugging you, really appreciate this thanks

@benvinegar
Copy link
Contributor Author

We released 3.14.0 today.

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.

react-native on android in production make error that can't read
3 participants