Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Basic integration of Crashlytics for iOS... #549

Merged
merged 26 commits into from
Apr 22, 2018

Conversation

DickSmith
Copy link

...including automatic dSYM uploads. Based on @hypery2k 's nativescript-fabric plugin.

Since it is only for iOS right now, both original Firebase crash_reporting and crashlytics can be enabled. If crashlytics is enabled, then crash_reporting is disabled for iOS, but still enabled for Android.
I prioritized Crashlytics for iOS since the original crash_reporting works well enough for Android issues (even with uglify on, but no obfuscation on proguard), but for iOS without dSYM it's almost impossible to decode most of them. The Crashlytics for Firebase offered the easiest solution for automatic dSYM uploading. For Crashlytics dSYM to be properly generated, bitcode must be disabled, which is done via the podfile for all targets.

I have this running in Prod with 30,000+ daily users.

TODOs: implement Android; integrate custom logging

Dick Smith added 4 commits November 21, 2017 12:57
…loads. Based on @hypery2k 's `nativescript-fabric` plugin.

Since it is only for iOS right now, both original Firebase `crash_reporting` and `crashlytics` can be enabled. If `crashlytics` is enabled, then `crash_reporting` is disabled for iOS, but still enabled for Android.
I prioritized Crashlytics for iOS since the original `crash_reporting` works well enough for Android issues (even with uglify on, but no obfuscation on proguard), but for iOS without dSYM it's almost impossible to decode most of them. The Crashlytics for Firebase offered the easiest solution for automatic dSYM uploading. For Crashlytics dSYM to be properly generated, bitcode must be disabled, which is done via the podfile for all targets.

I have this running in Prod with 30,000+ users

TODO: implement Android; integrate custom logging
@EddyVerbruggen
Copy link
Owner

Hi @DickSmith this is awesome!

Do you feel like this is ready to be merged? If so, I'll test it and merge to master so we can ship it with release 5.0.0. It will take a little while to get all tasks for 5.0.0 completed, so there is no rush or anything.

Thanks,
Eddy

@hypery2k
Copy link
Contributor

@DickSmith this looks great. I could help on this. I currently working on the Android part on my plugin. Should me maybe merge our work somehow?
@EddyVerbruggen Not sure if it would be feature to make a branch directly on your plugin and work together there. What do you think?
I could break CI builds to the table, also with snapshots builds ;)

@EddyVerbruggen
Copy link
Owner

If there's anything I can do to help let me know. But perhaps it's easiest to push changes to Dick's fork so all of it ends up in this PR?

@DickSmith
Copy link
Author

DickSmith commented Nov 22, 2017

@EddyVerbruggen Yes it appears to be very stable, other than no ability to add the custom logs yet (https://firebase.google.com/docs/crashlytics/customize-crash-reports), which I hadn't been using yet with the original crash_reporting, so haven't explored much yet.

Other than that it is currently building correctly both locally (debug) and on my TravisCI (release with webpack/AoT/uglify; including the automatic dSYM upload), and I currently have 9000 iOS users already using the version with Crashlytics since I released it yesterday with no issues (other than the preexisting crashes that I can now actually make sense of with the symbols 😁 ). I will probably have several thousand more on it by the end of the day, too.

@EddyVerbruggen @hypery2k I'm open to whatever works best for you guys. This would have definitely been far more difficult without being able to "borrow" from @hypery2k 's plugin. 😀

The Android I think should be easy enough to add in the Gradle pieces. I haven't looked into uploading the Proguard mappings, although I don't think that is probably as essential for Android as the dSYM is for iOS. I probably use more Proguard than most {N} apps (it's how I avoid multidexing), but I don't apply the obfuscation since everything in Java is from Open Source anyway, so no real need to hide it like you would with your own private Java code in a regular Android app. (obfuscation could obviously mess up the native calls from {N} if you aren't careful about keeping the names used by the JS/TS)

@EddyVerbruggen I guess the big question is how we would want to implement the custom logs? Should we just use the same methods for both crash_reporting and crashlytics and just check to see which ones are enabled/included? That way people can easily switch between the 2 crash reports without changing all their usage?

@hypery2k
Copy link
Contributor

you can take a look at my android branch here
But I'm currently facing some issues:

12-15 12:52:27.920  2827  2827 I TNS.Native: NativeScript Runtime Version 3.3.1, commit 368c29d75ca1cb632aa1efd8f822bacdd3c4da41
12-15 12:52:27.920  2827  2827 D TNS.Native: JNI_ONLoad
12-15 12:52:27.920  2827  2827 D TNS.Native: JNI_ONLoad END
12-15 12:52:27.945  1961  2128 E Surface : getSlotFromBufferLocked: unknown buffer: 0x7f8bb267a4a0
12-15 12:52:28.059  1584  1584 I free_storage_changed: 1709887488
12-15 12:52:28.404  1584  1959 I dvm_lock_sample: [system_server,1,Binder_6,51,WindowManagerService.java,11874,WindowAnimator.java,121,10]
12-15 12:52:28.525  1232  1232 I sf_frame_dur: [com.android.launcher3/com.android.launcher3.Launcher,0,1,6,2,1,2,7]
12-15 12:52:28.526  2801  2815 E QueryController: Got null root node from accessibility - Retrying...
12-15 12:52:28.778  2801  2815 E QueryController: Got null root node from accessibility - Retrying...
12-15 12:52:28.872  2827  2827 D TNS.Native: Failed to load snapshot: dlopen failed: library "libsnapshot.so" not found
12-15 12:52:28.916  2827  2827 D TNS.Native: V8 version 5.5.372.32
12-15 12:52:29.029  2801  2815 E QueryController: Got null root node from accessibility - Retrying...
12-15 12:52:29.052  1247  1247 I SELinux : SELinux: Loaded file_contexts contexts from /file_contexts.
12-15 12:52:29.052  1247  1247 I auditd  : SELinux: Loaded file_contexts contexts from /file_contexts.
12-15 12:52:29.099  1247  1247 E DEBUG   : AM write failed: Broken pipe
12-15 12:52:29.100  1584  2848 I am_crash: [1584,0,org.nativescript.demo,950582854,Native crash,Aborted,unknown,0]
12-15 12:52:29.100  1584  1601 I BootReceiver: Copying /data/tombstones/tombstone_00 to DropBox (SYSTEM_TOMBSTONE)
12-15 12:52:29.101  1584  2848 W ActivityManager:   Force finishing activity org.nativescript.demo/com.tns.NativeScriptActivity
12-15 12:52:29.101  1584  2848 I am_finish_activity: [0,110625689,10,org.nativescript.demo/com.tns.NativeScriptActivity,force-crash]
12-15 12:52:29.153  1263  1263 I Zygote  : Process 2827 exited due to signal (6)
12-15 12:52:29.279  2801  2815 E QueryController: Cannot proceed when root node is null. Aborted search
12-15 12:52:29.280  2801  2815 E QueryController: Got null root node from accessibility - Retrying...
12-15 12:52:29.402  1584  2848 I WindowManager: Screenshot max retries 4 of Token{128565e ActivityRecord{6980399 u0 org.nativescript.demo/com.tns.NativeScriptActivity t10 f}} appWin=Window{36f33a4 u0 Starting org.nativescript.demo} drawState=4
12-15 12:52:29.402  1584  2848 I am_pause_activity: [0,110625689,org.nativescript.demo/com.tns.NativeScriptActivity]
12-15 12:52:29.403  1584  2848 W ActivityManager: Exception thrown during pause
12-15 12:52:29.403  1584  2848 W ActivityManager: android.os.DeadObjectException
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at android.os.BinderProxy.transactNative(Native Method)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at android.os.BinderProxy.transact(Binder.java:503)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at android.app.ApplicationThreadProxy.schedulePauseActivity(ApplicationThreadNative.java:727)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityStack.startPausingLocked(ActivityStack.java:867)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityStack.finishActivityLocked(ActivityStack.java:2907)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityStack.finishTopRunningActivityLocked(ActivityStack.java:2763)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityStackSupervisor.finishTopRunningActivityLocked(ActivityStackSupervisor.java:2755)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityManagerService.handleAppCrashLocked(ActivityManagerService.java:11971)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityManagerService.makeAppCrashingLocked(ActivityManagerService.java:11867)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityManagerService.crashApplication(ActivityManagerService.java:12556)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.ActivityManagerService.handleApplicationCrashInner(ActivityManagerService.java:12063)
12-15 12:52:29.403  1584  2848 W ActivityManager: 	at com.android.server.am.NativeCrashListener$NativeCrashReporter.run(NativeCrashListener.java:86)
12-15 12:52:29.404  1584  2848 I am_home_stack_moved: [0,1,0,0,noMoreActivities setFocusedActivity]
12-15 12:52:29.404  1584  2848 I wm_task_moved: [9,1,0]

you try yourself with my demo app here and adding npm i nativescript-fabric@1.0.6-bugfix-android-23 --E --S

After adding the plugin to my demo project i had to do an additional `tns build android`, otherwise the app crashes, see here: https://github.com/NativeScript/android-runtime/issues/358#issuecomment-352002965

@DickSmith
Copy link
Author

@EddyVerbruggen @hypery2k
K, so I finally got this working on Android; was a bit more complicated since it requires some bleeding edge as described here:

// Crashlytics requires Firebase >= 11.8.0
// Firebase 11.8.0 requires google-services plugin >= 3.1.2
// google-services plugin 3.1.2 requires gradle-plugin >= 3.0.1
// gradle-plugin 3.0.1 requires buildToolsVersion >=26.0.3

Tested with both PAN-demo and ng-demo (with and without Crashlytics enabled), as well as in my own ns+ng app.

Same as iOS before, if crashlytics is enabled in the config then the original crash_reporting is disabled.

However, getting the same issue as @hypery2k , somewhat different output, but second build works while first doesn't.

@hypery2k
Copy link
Contributor

hypery2k commented Jan 3, 2018

I'm waiting for feedback from @Plamen5kov, see here

@DickSmith: Maybe this helps here, too

@manojdcoder
Copy link

Thank you guys for making this. So it should be functional now with TNS >= 3.4?

@hypery2k
Copy link
Contributor

no, it's currently having issues, see here. I can not solve the class path issue for now. So currently not able to fix

@EddyVerbruggen
Copy link
Owner

@DickSmith Yeah nasty issue that one.

Awesome that this will actually report an issue like that!

So you think it's ready for prime time?

@hypery2k
Copy link
Contributor

hypery2k commented Mar 5, 2018

@DickSmith So is currently iOS and Android working for you?

I still struggling on my nativescript-fabric plugin with the Android Build and I currently think I should quit my plugin in favour to integrate all in the firebase plugin

```
ERROR Error: Uncaught (in promise): Error: java.lang.IllegalStateException: GoogleApiClient is not connected yet.
    com.google.android.gms.common.api.internal.zzba.zze(Unknown Source)
    com.google.android.gms.auth.api.signin.internal.zze.zzb(Unknown Source)
    com.google.android.gms.auth.api.signin.internal.zzc.revokeAccess(Unknown Source)
    com.tns.Runtime.callJSMethodNative(Native Method)
    com.tns.Runtime.dispatchCallJSMethodNative(Runtime.java:1088)
    com.tns.Runtime.callJSMethodImpl(Runtime.java:970)
    com.tns.Runtime.callJSMethod(Runtime.java:957)
    com.tns.Runtime.callJSMethod(Runtime.java:941)
    com.tns.Runtime.callJSMethod(Runtime.java:933)
    com.tns.gen.java.lang.Object_frnal_ts_helpers_l58_c38__ClickListenerImpl.onClick(Object_frnal_ts_helpers_l58_c38__ClickListenerImpl.java:12)
    android.view.View.performClick(View.java:5637)
    android.view.View$PerformClick.run(View.java:22429)
    android.os.Handler.handleCallback(Handler.java:751)
    android.os.Handler.dispatchMessage(Handler.java:95)
    android.os.Looper.loop(Looper.java:154)
    android.app.ActivityThread.main(ActivityThread.java:6119)
    java.lang.reflect.Method.invoke(Native Method)
    com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
    com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
Error: java.lang.IllegalStateException: GoogleApiClient is not connected yet.
    com.google.android.gms.common.api.internal.zzba.zze(Unknown Source)
    com.google.android.gms.auth.api.signin.internal.zze.zzb(Unknown Source)
    com.google.android.gms.auth.api.signin.internal.zzc.revokeAccess(Unknown Source)
    com.tns.Runtime.callJSMethodNative(Native Method)
    com.tns.Runtime.dispatchCallJSMethodNative(Runtime.java:1088)
    com.tns.Runtime.callJSMethodImpl(Runtime.java:970)
    com.tns.Runtime.callJSMethod(Runtime.java:957)
    com.tns.Runtime.callJSMethod(Runtime.java:941)
    com.tns.Runtime.callJSMethod(Runtime.java:933)
    com.tns.gen.java.lang.Object_frnal_ts_helpers_l58_c38__ClickListenerImpl.onClick(Object_frnal_ts_helpers_l58_c38__ClickListenerImpl.java:12)
    android.view.View.performClick(View.java:5637)
    android.view.View$PerformClick.run(View.java:22429)
    android.os.Handler.handleCallback(Handler.java:751)
    android.os.Handler.dispatchMessage(Handler.java:95)
    android.os.Looper.loop(Looper.java:154)
    android.app.ActivityThread.main(ActivityThread.java:6119)
    java.lang.reflect.Method.invoke(Native Method)
    com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:886)
    com.android.internal.os.ZygoteInit.main(ZygoteInit.java:776)
    at file:///data/data/__REDACTED__/files/app/tns_modules/nativescript-plugin-firebase/firebase.js:799:70
    at new ZoneAwarePromise (file:///data/data/__REDACTED__/files/app/tns_modules/nativescript-angular/zone-js/dist/zone-nativescript.js:777:29)
    at Object.firebase_common_1.firebase.logout (file:///data/data/__REDACTED__/files/app/tns_modules/nativescript-plugin-firebase/firebase.js:795:12)
    ...
```
@sitefinitysteve
Copy link
Contributor

How much of this changes with the out of beta announcement? https://firebase.googleblog.com/2018/03/firebase-crashlytics-graduates-from-beta.html

@DickSmith
Copy link
Author

@EddyVerbruggen
Yes. The core of this (SDK integration and the dSYM upload) has been stable since end of November in my Christmas app (PNP 2017 Portable North Pole, on App/Play Stores). And the additional logging has been in release for about 2-3 weeks now (about a million users in December and still a few thousand a month, even now).

@hypery2k
So I haven't looked at Android again since end of January, so I don't know if the situation has improved or not, but with the official announcement that @sitefinitysteve mentioned I guess I'll give it another look in the coming weeks. It also looks like with that official announcement that the Firebase Console UI is now pushing hard for people to transition from the old Crash Reporting to Firebase/Crashlytics now. They don't give a timeline for when they plan to remove the old Crash Reporting, so I would be guessing maybe at least a few months, but can't be certain. When I look at the old Fabric/Crashlytics dashboard (my old non-{N} apps were using Fabric/Crashlytics directly), I don't see a mention of when they plan to retire it, but I'm guessing it will be at some time in the next few months as well?

@sitefinitysteve
It appears that my current iOS implementation is still fully supported, including the additional analytics integration which I've been seeing for a few weeks now. From what I can see there haven't been any changes to the documentation for Firebase/Crashlytics that would affect what I've done (other than it mentioning specific versions to pin to in the CocoaPod now, so it'd be a question for @EddyVerbruggen about if we want to pin these pod versions or not per his usual preferences there.)

@EddyVerbruggen
In short, this PR is stable and well proven for enabling Crashlytics on iOS, so should be good to get merged and I'll make another PR once I figure out the Android situation now that the disadvantage of Firebase/Crashlytics not including the analytics has been resolved with the official release.

I'll post another PR in the coming weeks that will include the full removal of the old crash_reporting stuff (while maintaining backward compatibility with the firebase.nativescript.json so that having either crashlytics or crash_reporting set to true will enable Crashlytics for both iOS and Android).

I still have a branch with the Android changes I had been working on, so I should be able to pick up where I left off easily enough, but it may still take some massaging to get it to work. I am also not sure if the changes coming in {N} 4.0 will make this easier or harder as last I checked some of the big changes to the tns-android template project seemed to affect the way some of this will need to work, especially in the *.gradle files. So the PR I end up making may only be compatible with one or the other for now (depending on what I find).

If you have any questions, feel free to send me a message on the {N} Slack, where my username is also dicksmith .

@DickSmith
Copy link
Author

@EddyVerbruggen
It also looks like this commit ended up on this PR which fixes an issue I was seeing in Android: bb5bb5e

If you want I can revert it and submit it in another PR, or leave it.

@EddyVerbruggen
Copy link
Owner

@DickSmith You can leave it in here, I'll be merging it as soon as I've finished a few other things. Won't be too long. Thanks so much for your work and perseverance!

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Apr 24, 2018

@DickSmith This seems to work perfectly fine indeed!

Just curious: I have no idea how one could verify whether or not those dSYM files are actually uploaded and used in the firebase console. I can force a crash just fine and it turns up in the Firebase console after the app is restarted and Firebase.init runs... but how can you tell those dSYM files are applied?

@hypery2k
Copy link
Contributor

did you have a preview build of the plugin? I could verify it in our beta test setup

@DickSmith
Copy link
Author

DickSmith commented Apr 24, 2018

@EddyVerbruggen
If the crash log has stack has stuff like
NativeScript NativeScript::reportFatalErrorBeforeShutdown(JSC::ExecState*, JSC::Exception*, bool) + 1784
Then the dSym is successful.
Otherwise you would just see anonymous memory addresses. Stuff like NativeScript+88499 or whatever... might not even see the NativeScript part.

@hypery2k If you want I could send you the .tgz I've been using either via Slack or email.

@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Apr 24, 2018

@DickSmith Yep, I see the former so it's working 😍

@hypery2k I was just chatting with @DickSmith on Slack while narrowing down what causes the crash on Android in both your Fabric plugin and this one (as I was working on adding the Android side of Crsahlytics to Firebase today).

The good news: found it! 🎉

The bad news: both our plugins need an ugly build step to copy the Android runtime metadata (3 .dat files) from the src folder (the highlighted one at the bottom of the screenshot) to the build folder (the one at the top, note that for release builds this is a different folder) because somehow the NativeScript build system and Crashlytics bite each other (Crashlytics writes a file called crashlytics-build.properties (also highlighted) to the same folder as where the metadata needs to be copied to).

I can't tell for sure this is the cause, I only see the symptom. But that symptom will be cured by a build step which I can also PR to the Fabric plugin if you like.

screen shot 2018-04-24 at 21 08 19

@hypery2k
Copy link
Contributor

great for the hint. Feel free to send a PR in this branch. Glad for your support

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants