Can't interpret line numbers with sourcemap.

[chetstone]

When I use this component, I get a log file in Crashlytics that looks something like this:

Non-fatal Exception: Can't find variable: foobar
0  ???                            0x0 onPress (main.jsbundle:9477)
1  ???                            0x0 _onPress (main.jsbundle:9125)
2  ???                            0x0 a (main.jsbundle:1653)
3  ???                            0x0 o (main.jsbundle:1605)

When I use sourcemap using the script given here, and using line number 9477 and column 1, I get this output:

{ source: null, line: null, column: null, name: null }

There are only 608 lines in my main.jsbundle. How to interpret the output I'm seeing on crashlytics?

Thanks

[mikelambert]

I haven't actually deployed and been using this yet myself. But as a wild guess, is it possible these numbers represent character offsets, not line offsets?

[chetstone]

Thanks for the suggestion. No, trying with line 0 and column 9477 gives nothing, line 1 and column 9477 gives an output in polyfills, not in my code. If they were character offsets from the beginning of the file they'd have to be much bigger. They may be column numbers, and the line number is just missing.

[mikelambert]

Ah interesting idea. Maybe my patch to react-native-fabric is buggy and missing column numbers?

What platform are you testing with? This is the code that converts JS exceptions to objects Crashlytics will understand:

Android: https://github.com/corymsmith/react-native-fabric/blob/master/android/src/main/java/com/smixx/fabric/SMXCrashlytics.java#L91

iOS: https://github.com/corymsmith/react-native-fabric/blob/master/ios/SMXCrashlytics/SMXCrashlytics.m#L93

[chetstone]

I'm on iOS. Probably something's going wrong with the translation from the JS stackframe to CLSStackFrame but I don't know enough Objective C to be able to see what's wrong.

[tianjianchn]

As I tested many cases, the lineNumber displayed in the Crashlytics web is not same with the actual one in the error stack.
Based on current implementation of recordCustomExceptionName in react-native-fabric, I use the code below to display the log:

import StackTrace from 'stacktrace-js';
var originalHandler = global.ErrorUtils.getGlobalHandler();
function errorHandler(e) {
  StackTrace.fromError(e).then((stack)=>{
    stack = stack.map(row=>{
      const {source, lineNumber} = row;
      return {fileName: e.message, lineNumber, functionName: source}
    })
    Crashlytics.recordCustomExceptionName(e.message, e.message, stack)
  });
  if (originalHandler) {
    originalHandler(e);
  }
}
global.ErrorUtils.setGlobalHandler(errorHandler);

The log will be:

Non-fatal Exception: Oh my ass 2016-07-05 20:30:30
0  ???                            0x0     at AwesomeProject.render (http://localhost:8081/index.ios.bundle?platform=ios&dev=true:1537:7) (Oh my ass 2016-07-05 20:30:30:24595)
1  ???                            0x0     at ReactCompositeComponentWrapper._renderValidatedComponentWithoutOwnerOrContext (http://localhost:8081/index.ios.bundle?platform=ios&dev=true:18090:28) (Oh my ass 2016-07-05 20:30:30:289443)
2  ???                            0x0     at ReactCompositeComponentWrapper._renderValidatedComponent (http://localhost:8081/index.ios.bundle?platform=ios&dev=true:18116:24) (Oh my ass 2016-07-05 20:30:30:289859)

[jondot]

Hey guys, I've been working on this problem, and after pulling most of my hair out, finally have a solution.
Sadly, I can now say that none of the above will really work, because there are a few fundamental bugs in how this library interprets positions, and how Crashlytics accepts data (might be on their side).

The fix is a combination of pulling sourcemaps, tweaking this library and the native crashlytics reporting as well. I need some time to pick up the pieces and make an elegant patch and for now, stay tuned :).

[jondot]

@mikelambert I'm trying to evaluate what's needed to be changed, and it looks like one PR for react-native-fabric-crashlytics with a major API change, and one PR for react-native-fabric for the native part (at least, iOS).

So the very least we'll have 2 PRs that depend upon each other which is a bummer (since we can't guarantee the upstream PR will happen on time). Any idea what to do here? perhaps fork react-native-fabric and depend on it for the meanwhile?

Further, we need a new API like this:

initWithSourceMap(..sourcemap..)

Where the exising init API isn't really helping and would become useless, unless you explicitly don't minify your bundle in release, but I think everyone do because its the default behavior, and therefore the regular init leads to mangled line numbers as mentioned in this thread. So should we drop init and force users to fetch and provide a sourcemap?

And also, we need a modified build step, or at least a README instruction that exports sourcemap as part of the regular RN build (I hacked it out manually in packager but I hope it comes built-in and does proper sourcemaps).

[chetstone]

Would it make sense to integrate this library (with your changes) into
react-native-fabric with a PR?

On Sun, Jul 10, 2016 at 5:12 AM Dotan J. Nahum [email protected]
wrote:

@mikelambert https://github.com/mikelambert I'm trying to evaluate
what's needed to be changed, and it looks like one PR for
react-native-fabric-crashlytics with a major API change, and one PR for
react-native-fabric for the native part (at least, iOS).

So the very least we'll have 2 PRs that depend upon each other which is a
bummer (since we can't guarantee the upstream PR will happen on time). Any
idea what to do here? perhaps fork react-native-fabric and depend on it
for the meanwhile?

Further, we need a new API like this:

initWithSourceMap(..sourcemap..)

Where the exising init API isn't really helping and would become useless,
unless you explicitly don't minify your bundle, but I think everyone do
because its the default behavior, and therefore the regular init leads to
mangled line numbers as mentioned in this thread. So should we drop init
and force users to fetch and provide a sourcemap?

And also, we need a modified build step, or at least a README instruction
that exports sourcemap as part of the regular RN build (I hacked it out
manually in packager but I hope it comes built-in and does proper
sourcemaps).

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA7l19Ibu7RFjGHzvDRQaWTc5TilQ9HZks5qUNO3gaJpZM4I_ZEI
.

[mikelambert]

I would be fine integrating this functionality entirely into react-native-fabric. I was unsure on which direction to go when building it originally. And I'm fine with a RN-fabric fork that we depend on until things are all upstreamed.

I would prefer an init() and initWithSourceMap(). Sometimes the exceptions are visible/useful to users without line numbers, and probably faster to load and have smaller filesizes, so I'm okay with offering that backup support.

Yeah, ideally we'd find a way to build sourcemaps as part of the building flow. But that might require manual changes to the ios/android build scripts, or changes to RN itself. I'm not as clear what was required here for your packager changes...but perhaps they could be enabled by a flag, and upstreamed to the RN codebase?

[jondot]

So I'm now after a full project setup with how I think this can work, and it works great. Building the sourcemap is done in a similar way to a bundle, so I guess this is a command a user would need to run (I hacked the default react-native runner - but I think we can drive the fact that the build will always generate source maps to the core react native repo with a PR, aside from little more CPU i don't see why not)

So here's how you engage with the new API:

function crashload(){
  const path = `${RNFS.MainBundlePath}/sourcemap.js`
  RNFS.readFile(path, 'utf8').then((contents)=>{
    crashlytics.init(JSON.parse(contents))
  })
}
crashload()

Unfortunately, anyone wanting to read a bundled source map has only one way to do that - with RNFS. Previous React Native releases had already read sourcemaps and had sourcemap exposed as a magic global variable but right now a React Native process comes up blank. It was made so in order to save memory - so we might need to warn users that using sourcemaps isn't for free.

Back to the code sample, here, init takes sourcemap. This is adjusted to @mikelambert comment about using init with or without sourcemap, I haven't yet did that - but we can make it not load and use sourcemap when the argument isn't given.

sourcemap.js is built the same way main.jsbundle is built, and resides in the main app the same way for it to be packaged (and read later):

And to sum it up, this uses the patch to this library and the react-native-fabric fork with the fixed type conversion bug.

Next up I'm going to submit a PR tomorrow (since I need to use this time right now for something else), and that will include the init which knows how to toggle modes. With that, and the snippet above that we can put in the README.md, we have a working solution.

[jondot]

I've submitted the PR.
Don't rush into merging it - I think it could use some eyeballs and testing. When I tested it on a fresh project I lost proper function names, though resolution of files, line number, column number, remained working great. The reason isn't clear to me yet since SourcemapConsumer should spit out the original names as well (they exist in the sourcemap).

To not hold this PR longer, I synthesized something that would make sense instead of a function name: file + line + col which you can take and go back to your source tree and find exactly what was the bit that was causing trouble.

There's quite a bit of prepwork to do to set up sourcemaps and I've tried lining it out on the README, but I'll be here to help anyone wanting to set it up.

The second issue with resolving proper line numbers was a bug in react-native-fabric that didn't use the real value behind a number in the bridging code, so it took the value of the pointer to the number, which is arbitrary - instead of the line number. see here:
https://github.com/corymsmith/react-native-fabric/pull/52/files

[chetstone]

So what this does is do the sourcemap conversion on the device before uploading the stack trace to crashlytics? Very cool!

However, I don't think I'd want to use it that way, first, because of the size issue in including the sourcemap in the app bundle, and second, I use code-push, and the sourcemap in the bundle is likely going to be out of date.

So I'd want to use this to upload a correct trace of the minified bundle, then download it and translate manually. Can it be used like that?

[jondot]

Yes, that's why the library gets a materialized source map already. Reading the source map is separated from using it, and in the given and suggested example it is packed with the app and read from the app bundle. It could by all means be fetched from network.

I have to say, that a real solution for this problem must be that crashlytics or any server side component should do the mapping and translation based on a copy of the source maps that is sent to them. That's how it should work, because really, the client doing this - that's wasting precious memory for little value.

In conclusion as long as Crashlytics don't provide this, we have to make up for it.

Another 2 ways I thought about and dismissed early on is to make a service that integrates with crashlytics and provides an alternative view that they don't provide, and to make a transparent crashlytics-like service that accepts an event, demangles it and forwards it to the real crashlytics on the fly, and as far as the client cares it talks to the real crashlytics.

Hope that makes sense. But really I'd hope crashlytics comes up with this feature soon, otherwise use this solution or move to more JavaScript friendly logging service early on.

[chetstone]

OK I'm trying this out using the fabric and crashlytics repos from github:jondot.

When I initialize with the sourcemap, it does work great--- the crashlytics dashboard shows the exact function, file, line number and column of the error. But when launching again after the crash, the app is unresponsive for several seconds while it processes the 1.7MB sourcemap. I don't want to use it this way, since I'm sure that my need to examine crashlogs will be very infrequent ;-)

So when I call crashlytics.init() with no arguments (I assume that's the way to use it if I want to do sourcemap translation later), the crashlog in my crashlytics dashboard looks like this:

Non-fatal Exception: Can't find variable: foobar
0  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:596)
1  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:576)
2  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:103)
3  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:100)
4  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:100)
5  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:97)
6  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:97)
7  ???                            0x0 undefined@undefined undefined:undefined (Can't find variable: foobar:106)

Finally, if I use your fix to react-native-fabric and the old version of react-native-fabric-crashlytics, I get this:

Non-fatal Exception: Can't find variable: foobar
0  ???                            0x0 onPress (main.jsbundle:592)
1  ???                            0x0 _onPress (main.jsbundle:572)
2  ???                            0x0 a (main.jsbundle:103)

Which does make some sense. When looking that up in the sourcemap, I get:

Looking for line  592
{ source: '.../components/settings.ios.js',
  line: 29,
  column: 0,
  name: '__d' }

It's finding the right file, anyway. The error is actually on line 291. So we need column information in the bundle.

[jondot]

Thanks @chetstone ! yes, I think I didn't give some love to the old path with init without sourcemap as much as needed. This should be it jondot@b5ad744#diff-168726dbe96b3ce427e7fedce31bb0bcR38
I'll make a smarter fallback there and make sure the init() flow is similar shortly.

Regarding loading of sourcemap - that's true, completely the tradeoff of doing the mapping this way. My only suggestion would be to load that in background and init only when ready, or wait to see if there are better ideas.

[jondot]

@chetstone fix is in, details here

[chetstone]

Thanks @jondot but sorry to say that doesn't really help much. It now shows the function name but I've got dozens of functions called onPress in my code. What I really need is the line number and column number in the bundle. Here's what I get now.

Non-fatal Exception: Can't find variable: foobar
0  ???                            0x0 onPress (Can't find variable: foobar:602)
1  ???                            0x0 _onPress (Can't find variable: foobar:582)

I guess 602 and 582 are line numbers but they shouldn't say foobar:602, the should be labeled with line: And column is still missing. It seems like if these are available when translating to a sourcemap they should be viewable here.

[chetstone]

I took a look... I can't figure out why the column number doesn't come through but if you use row.source all the info will be included in the "functionName". Like this:

      return {
        fileName: loc.source || row.fileName,
        columnNumber: loc.column || row.columnNumber,
        lineNumber: loc.line || row.lineNumber,
        functionName: loc.source ? `${loc.name}@${loc.source} ${loc.line}:${loc.column}` :
        `${(row.source || 'unknown_func')}`, //next best thing without a consistent function name
      };

That produces a report like this on the fabric dashboard:

Non-fatal Exception: Can't find variable: foobar
0  ???                            0x0 onPress@file:///var/containers/Bundle/Application/FDE716B8-8C83-4ACC-ADAA-D41788FC8F81/Accumulations.app/main.jsbundle:602:5757 (main.jsbundle:602)
1  ???                            0x0 _onPress@file:///var/containers/Bundle/Application/FDE716B8-8C83-4ACC-ADAA-D41788FC8F81/Accumulations.app/main.jsbundle:582:3283 (main.jsbundle:582)
2  ???                            0x0 a@file:///var/containers/Bundle/Application/FDE716B8-8C83-4ACC-ADAA-D41788FC8F81/Accumulations.app/main.jsbundle:103:72 (main.jsbundle:103)

Not perfect but good enough.

[mikelambert]

Thanks guys for all the investigation, I'm impressed how far you've taken this (yay open source!)

Some thoughts:

• I'm confused why @chetstone says loading the app after a crash takes so long...shouldn't the first load of the app also be taking awhile? The example code doesn't appear to be doing anything on second-load it wouldn't also do on first-load?
• Maybe we can load the sourcemap only when the exception actually occurs, versus loading it at startup? Perhaps by passing a "load the sourcemap" function instead of passing the actual sourcemap?

[chetstone]

Perhaps it also takes a long time on the first load and I didn't notice.

I won't have time to investigate further for a couple of days.
On Thu, Jul 14, 2016 at 00:50 Mike Lambert [email protected] wrote:

Thanks guys for all the investigation, I'm impressed how far you've taken
this (yay open source!)

Some thoughts:

• I'm confused why @chetstone https://github.com/chetstone says
  loading the app after a crash takes so long...shouldn't the first
  load of the app also be taking awhile? The example code doesn't appear to
  be doing anything on second-load it wouldn't also do on first-load?
• Maybe we can load the sourcemap only when the exception actually
  occurs, versus loading it at startup? Perhaps by passing a "load the
  sourcemap" function instead of passing the actual sourcemap?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA7l13_5Io0q2MWBwJhAFeCE5CeB7TKYks5qVdwugaJpZM4I_ZEI
.

[jbrodriguez]

@mikelambert , probably you should also take a look at this issue too.

facebook/react-native#7998

Not sure if it only affects Android (it happened to me on Android).

[mikelambert]

Thanks for the tip @jbrodriguez, I've just pushed 0.1.4 with an {offline: true} fix. (Note, jbrodriguez's issue is independent of the sourcemap issue...)

[chetstone]

@mikelambert, looks like you're missing a closing brace in your fix.

On Sun, Jul 17, 2016 at 7:47 PM Mike Lambert [email protected]
wrote:

Thanks for the tip @jbrodriguez https://github.com/jbrodriguez, I've
just pushed 0.1.4 with an {offline: true} fix. (Note, jbrodriguez's issue
is independent of the sourcemap issue...)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA7l1-a8B68TxH6ciD57qLX2uCjfw9Kfks5qWts1gaJpZM4I_ZEI
.

[mikelambert]

Yup, sorry about that. :/ I noticed and immediately pushed a 0.1.5 (before your message) to NPM. Looks like I forgot to push the github repo before heading out, though...I've pushed that now.

[wootwoot1234]

Thanks for the great work guys. I'm trying to understand how to use this correctly and all the option and pros and cons to the options (aka slower load times). @jondot can you explain the pros and cons to not passing a sourcemap to init? Should use your fork or this fork? What I'm hoping to do is use this to get some basic information about where the crash is occurring but not at the cost of slowing down the app very much. Sounds like I can call init without the source map and get what I'm looking for, right?

[jondot]

@wootwoot1234 sorry for the late response. If you do use this, it means on every crash that's sent out, the actual device will do the symbolication of the stack trace, which means a bit of CPU work for the device. The idea is that in normal life a backend does this (that's how it happens on native, and why you send out your symbols to apple).

[mikelambert]

Hey Jon, I just remember I never actually merged your change in. I know you said we shouldn't rush, but I think we "didn't rush" for too long. :)

It looks fully backwards compatible, so I'm fine to merge it in. But the "use sourcemap" case SourceMapConsumer does a lot of work at startup (json parsing, extracting all the metadata, etc). Ideally I'd like to make that lazily-done at exception time, as much as possible. Are you interested in making that change, or should I merge this in as-is, and consider my own attempt at improving the load times myself?

[jondot]

Hey Mike,
Well, the thing is this. I'm convinced sourcemaps should happen on the backend and the latest commit there in react native core is a hint of maybe the issue i've been experiencing with the sourcemaps being inaccurate even after this patch. So I guess we can wait a little more while, I plan to test out that change once it's hydrated.
Meanwhile we're testing Sentry for sourcemaps and it looks good so far (still some level of inaccuracy, like one-line off, and columns are completely off).

[arronmabrey]

@mikelambert I think it would be beneficial to merge the code as is, I agree with @jondot 's last comment about "sourcemaps being inaccurate even after this patch" but anything would be better than what's currently available.

Once it's merged it will easier to add a lazy function to do the JSON parsing and such at exception time.

Additionally all of this is opt-in behaviour, so it's unlikely to affect existing users, and people who do decide to opt-in will be aware of the caveats.

[oakis]

Any updates on this? I would love to have correct function names and line numbers in my reports..

[wootwoot1234]

@oakis Check out bugsnag.

[reyalpsirc]

Since I needed this, I made a pull request that enables line numbers on iOS and also on Android but, the Android ones are not correct due to facebook/react-native#6946.

[toearth]

@mikelambert When would merge the pr?