Kiwix-android: Configure Fabric to automatically post crash reports

Created on 17 Feb 2018  Â·  43Comments  Â·  Source: kiwix/kiwix-android

Is it a ?

  • [ ] Bug Report

  • [x] Feature Request

Feature Request

Feature Description

Currently, we are unable to map our user's Oh, crap! moments when our app crashes or behaves erroneously. Thus, we can configure Fabric library (Crashlytics) to automatically post crash reports to us, the developers. And the best thing about Fabric is that it can provide all the fields in reports such as APP_VERSION, ANDROID_VERSION, PHONE_MODEL, CUSTOM_DATA, STACK_TRACE, LOGCAT, etc.

  • Medium to receive crash reports:

    • [x] _Email_

    • [x] _Fabric Dashboard (back-end)_

  • Why should be go with Fabric?

    • Free of cost

    • Provides facility to Invite Teammate allowing more multiple maintainers to access the resources.

    • Accomplishes one of the Improvements to be made, suggested in the GSoC Ideas list. (Requesting features directly from the user can always be implemented by a providing a simple form fill up)

I have already tested it and works just awesome.

@mhutti1 @kelson42 Your reviews please.

enhancement good first issue question stale

All 43 comments

@brijeshshah13 Have you seen https://github.com/kiwix/kiwix-android/issues/335 ? It seems to be quite the same to me.

@kelson42 @mhutti1 I didn't knew about that, but I do want to ask this question: Why go through all this pain and extra code which leads to increase in apk size only to provide facility which can be achieved by adding just a few lines of code? I mean, Crashlytics is free of cost and provides all the data at a moment's notice.

Why am I insisting on using Crashlytics?

  • Free of cost

  • Most of the times, the users will select the No Thanks button as they do not want to get into all the trouble of sending some data they don't understand to people they don't know, leaving our app prone to those bugs and crashes, deteriorating user experience.

  • We can invite as many people to join our Team at Crashlytics to monitor and fix the reported crashes.

  • Crashlytics provides the feature to download the text file of the Log at which the crash occurred acknowledging us the exact location of the bug.

  • Get real time analysis of the performance of the app.

  • _Crash Insights_, a feature provided by Fabric, groups issues together based on their common cause. This makes it easier for you to understand what‘s behind your crashes.

  • Map crashes for different versions of the app separately.

And much more.... Infact, I analyzed the apk after adding Fabric, and it didn't even increase the size of the apk by 0.1 Mb, which we reduced by crushing the PNG files. So, overall it's a win-win for both the user and the developers.

@mhutti1 @ISNIT0 @julianharty @EladKeyshawn ?

As far as I am aware #335 is the first step towards addressing user crashes. The main point being we do not want to start sending user data anywhere without their explicit permission. As such we will need to take the time to ensure external libraries fully meet our needs.

@mhutti1 Well, that's the best thing. Fabric doesn't report any user data, only the crash data. No private info of the user is exposed thus preserving their right to privacy. And to acknowledge users about sending crash reports, we can display a Toast using the current Global Crash Handler. You can examine the data collected and analytics from the dashboard and take a decision whether or not we want to incorporate this in our app. I have invited you to join the Team at Fabric. Please check your email.

@mhutti1
Update Fabric also provides this: _When a crash occurs in any version of your app, this will ask your users if that crash should be reported. If you’d like to enable it, you can do it in your app settings page._

However, when I tried to check it out, I was shown this dialog box informing the same point I mentioned above.

fabric

Even a simple crash report could contain identifying information that in certain circumstances could identify a user. If we get their permission as you suggest it could work however I think we will need to think this over a bit.

@mhutti1 No problem. However, I myself have gone through the whole procedure and can't locate any instance where the identity of the user is exposed. Please take your time to analyze the Fabric dashboard and share what you conclude.

@brijeshshah13 At a previous hackathon, we looked at a few different solutions, but decided that email was the simplest and easiest to control. We did start using crashlytics, and ended up changing.

Another reason to go with email, is that... It's implemented :)

@ISNIT0 I agree that it may be easy to control and it's already implemented, but asking the users to select which data is to be sent (which they no nothing about) and manually send it from an email client will be quite cumbersome for them which will result in almost no crash reports being reported.

¯_(ツ)_/¯ I personally am not fussed, and the discussion would be very different if we had a PR for each solution.

I seem to remember with Fabric we were unable to allow users to decide how much info they send us (or even if they send it). Email is a well understood medium, and users are well aware of what to expect from sending one (it uses mobile data, exposes their email address, sends the content of the email).

Something else that Fabric doesn't give us for free is a crash page, so that would need to be implemented in order to be sensibly compared to the existing open PR

@mhutti1 @ISNIT0 Agreed. But, Fabric doesn't expose the email address of the user at the least as @mhutti1 emphasized on prevention of user's identity in any way. Also, Fabric does provide the facility to present the user with a dialog box asking for the permission and the crash data is sent only upon acceptance by the user. I personally examined the data Fabric collects and reached to a conclusion that it only collects the data which might be required by us any how to deal with any issue.

Any conclusions drawn regarding this issue? @mhutti1 @kelson42 @ISNIT0

While we're chatting about this, it may be worth looking at Count.ly...
I've built a simple integration for other purposes in this branch: https://github.com/kiwix/kiwix-android/tree/ISNIT0/instrumentation
(it has a dead API key in there)

It's been quite nice/easy to use and integrate. It's very obvious what's going on, and is open source (where Fabric is not). @julianharty and I have collected lots of nice data from a few devices with the test integration.

It has the capability to do all the things we're talking about, plus it's open source, self hosted, and very transparent.

I made both a Crashlytics and Fabric integration branch a while ago to evaluate them:

I found that Fabric's "ask user for permission" was set on the server, which means the app clearly has to communicate with the server to get that information beforehand... I personally don't trust Fabric (Google), especially considering this is a highly open source project.

At the end of the day though, these are just my mostly un-educated opinions, happy to be wrong/countered.

@ISNIT0 @mhutti1 @kelson42 I will dig deep and study Count.ly carefully and at the end, if we finalize using it then I think #351 can be used for allowing the user to create a custom report and request features. What are your views on that?

Fine by me!

@ISNIT0 Does Count.ly provide the facility to display a dialog box or notification to ask the user if they want to send the crash data? If yes, can you please direct me to some documentation that describes it. I tried to crash the app after integrating it but it didn't ask for any approval.

No, that would be for us to do manually (easy enough to do)

On Fri, 16 Mar 2018 at 12:47 Brijesh Shah notifications@github.com wrote:

@ISNIT0 https://github.com/isnit0 Does Count.ly provide the facility to
display a dialog box or notification to ask the user if they want to send
the crash data?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kiwix/kiwix-android/issues/443#issuecomment-373702927,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE7vi37Scr56yilvweinq9rq9JZlSY-Qks5te7RngaJpZM4SJXPP
.

>

Joseph E. Reeve

@ISNIT0 Can you please verify that is indeed possible because the enableCrashReporting() automatically catches uncaught Java exceptions and reports them to server. The problem arose when I tried to set up a dialog box to display on the screen asking for the approval before the crash data is sent. However, the files provided by Count.ly are read-only files, thus preventing us from editing them and incorporating our feature.

Instead of using their global crash handler, register a custom one as per
https://github.com/kiwix/kiwix-android/pull/351
You can then use Countly.logException and log everything manually :)

On Fri, 16 Mar 2018 at 13:28 Brijesh Shah notifications@github.com wrote:

@ISNIT0 https://github.com/isnit0 Can you please verify that is indeed
possible because the enableCrashReporting() automatically catches
uncaught Java exceptions and reports them to server. The problem arose when
I tried to set up a dialog box to display on the screen asking for the
approval before the crash data is sent. However, the files provided by
Count.ly are read-only files, thus preventing us from editing them and
incorporating our feature.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kiwix/kiwix-android/issues/443#issuecomment-373712939,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE7vi7b27aKvQ7wVvXctbI_BC3xwvc5wks5te73rgaJpZM4SJXPP
.

>

Joseph E. Reeve

@ISNIT0 From what I understand, you are saying that we use the global crash handler implemented in #351 to catch the crash and instead of using Countly.sharedInstance().enableCrashReporting(), we manually catch every exception in the project using Countly.sharedInstance().logException(Exception exception) which will allow us to send the crash report to Count.ly as logException() method contains sendCrashReport() as well as redirect the user to the email app so that any other issues or feature requests can be added. Please correct if I'm wrong.

To be clear, this would be an alternative to email.
But yes, we'd implement the global crash handler on our end, not using the
Countly global crash handler.

On Fri, 16 Mar 2018 at 14:56 Brijesh Shah notifications@github.com wrote:

@ISNIT0 https://github.com/isnit0 From what I understand, you are
saying that we use the global crash handler implemented in #351
https://github.com/kiwix/kiwix-android/pull/351 to catch the crash and
instead of using Countly.sharedInstance().enableCrashReporting(), we
manually catch every exception in the project using Countly.sharedInstance().logException(Exception
exception) which will allow us to send the crash report to Count.ly as
logException() method contains sendCrashReport() as well as redirect the
user to the email app so that any other issues or feature requests can be
added. Please correct if I'm wrong.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/kiwix/kiwix-android/issues/443#issuecomment-373738715,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AE7vi0Ww-fN0qaj3niPRQOLCF0QlIi-Sks5te9KBgaJpZM4SJXPP
.

>

Joseph E. Reeve

@ISNIT0 If email feature will be removed, how will the user be able to add the feature requests/suggestions or other issues? I think we must provide redirecting to the email app so that the former can be added by the user if they wish. This is will accomplish both the tasks:

  • We will be able to get the crash reports on Count.ly without worrying about the spam e-mails or false data being sent to us.

  • We can check e-mails for feature requests/suggestions or other issues sent by the users.

Sounds reasonable? @mhutti1 Your views?

Could be a good solution.

@mhutti1 What I'm thinking is that we divide this into 2 parts:

  • Global Crash Handler will handle the crashes and redirect the user to an email app to send the crash report. We do not change it's structure or body of the email.

  • Report an issue option will provide the user an almost replica of the global crash handler except that it won't collect any _Details of the crash_ as there won't be any. Here, we tweak the email body of the crash report to ask for the issues/feature requests, thus providing transparency and nice interface for writing them without the cost of implementing it manually.

Sample

screenshot_20180319-183712

Please share your views on the same.

That sounds like a fairly decent solution. We are a bit flooded at the moment with the GSoC application process so I don't think any work will be able to be made till a bit after the application deadline.

@ISNIT0 I'm a bit confused here. How will we integrate Countly.sharedInstance().logException(Exception exception) with our global crash handler? Because Countly.sharedInstance().logException(Exception exception) will log handled exceptions whereas the global crash handler will handle uncaught exceptions. Please elaborate.

@brijeshshah13 It could be implemented as shown in the below example.

@Override public void uncaughtException (Thread thread, Throwable e) {
Countly.sharedInstance().logException(new Exception(e))
}

@ISNIT0 Link you provided isn't working. Can you give it a check please?

@ISNIT0 Alright. So we manually catch uncaught exceptions in each activity and before passing the Intent to the KiwixErrorActivity, we log the handled exception using Countly, correct?

@GeVic Thank you for the reference.

@GeVic the link still works for me, could you try again?
@brijeshshah13 All activities inherit from BaseActivity, so we can just catch them there.

@ISNIT0 Sorry for the late response, I have been quite occupied lately. I checked and found that KiwixSettingsActivity, SearchActivity, SplashActivity and ZimManageActivity do not inherit BaseActivity. How do we want to cover those activities?

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@mhutti1 @macgills @abdulwd Is that still a valid ticket? What should we do with it?

Well the email reporting is implemented?

@macgills Yes AFAIK, even if I don't know exactly when this will appear (I don't have experienced it myself). You should have access to the mailbox to read the emails.

It appears anytime there is a crash, you not seeing it is a good thing. I have already received and triaged multiple emails to us regarding crashes

@macgills yes... sod we should close this ticket to your opinion?

email is pretty clunky for crash reporting, I would be in favour of using a more extensive solution somewhere down the line

I agree with macgills

@macgills @mhutti1 I would then recommend to open a new ticket to superseed this one, kind of last track of what should exactly done in this ticket!

This issue has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@macgills @mhutti1 I hope the necessary tickets have been open... I have unfortunately not much visibility on this part of the app as I don't know how to trigger it. What seems clear is that nothing more can be done for that ticket. I'll close it.

Was this page helpful?
0 / 5 - 0 ratings