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

Shall we add a global exception handler? #304

Closed
julianharty opened this issue Dec 2, 2017 · 6 comments
Closed

Shall we add a global exception handler? #304

julianharty opened this issue Dec 2, 2017 · 6 comments

Comments

@julianharty
Copy link
Contributor

We've various open issues (particularly #240 #287 kiwix/java-libkiwix#24 ) related to crashes affecting the Android app, some percolate from the native code, others may be triggered in the Java code.

@ISNIT0 and I are exploring (with the help of some new volunteers) possible ways to address kiwix/java-libkiwix#24 Meanwhile we wondered about the benefits of adding a global exception handler to the app. @ISNIT0 found the following app https://github.com/Ereza/CustomActivityOnCrash which might form the basis of this enhancement (it's licensed using Apache 2.0 which AFAIK is OK to incorporate) or we could write one ourselves.

I'll try incorporating https://github.com/Ereza/CustomActivityOnCrash in a local test build as in theory it's as simple as adding

 dependencies {
    compile 'cat.ereza:customactivityoncrash:2.2.0'
} 

to our Gradle build file. I'll update this issue with the results of my testing.

There are various discussions on ways to add a Global Exception Handler to minimise potential runtime problems e.g. https://stackoverflow.com/questions/2764394/ideal-way-to-set-global-uncaught-exception-handler-in-android

BTW: It'd be cool if we could allow & enable users who want to to send us a log along the same lines as https://stackoverflow.com/questions/19897628/need-to-handle-uncaught-exception-and-send-log-file I'll raise this as a separate issue so we can discuss the implications.

@julianharty
Copy link
Contributor Author

julianharty commented Dec 2, 2017

Well it works well so far,
I've committed a simple p-o-c in a branch https://github.com/kiwix/kiwix-android/tree/julianharty/custom_crash_poc in ff77895

This generates a runtime exception when deleting a bookmark using the bookmark manager UI. Of course, this code shouldn't be merged as-is. Firstly for the obvious reason that it includes a forced runtime exception, second as we'd want to agree (a) adding a global exception handler makes sense (b) we like this library (c) we're OK with incorporating code released using the Apache 2.0 license.

PS: We'd probably want to tweak the library if we decide we'd like to use it.
Thoughts?

@mhutti1
Copy link
Contributor

mhutti1 commented Dec 2, 2017

I think anything to help catch errors makes sense. I don't have any experience with this kind of library though. As for licences I think that that is more @kelson42 domain.

@kelson42
Copy link
Collaborator

kelson42 commented Dec 4, 2017

I think the ticket should be more clear about which kind of additional information will or should be gathered that way. That said, having an instrumented build of Kiwix has my vote. I would tend to use standard solutions provided by big players to do that. I trust you to choose something robust and maintained properly. Last but not least, first, I'm quite against having this is the published app and would like to have it build - beside the standard one, as a nightly.

@kelson42
Copy link
Collaborator

@julianharty I think this is now pretty much a duplicate of #305 and #335!? Should we close that ticket?

@stale
Copy link

stale bot commented Jun 26, 2019

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.

@stale stale bot added the stale label Jun 26, 2019
@mhutti1
Copy link
Contributor

mhutti1 commented Aug 15, 2019

I think we should close this as it is covered by other issues at this point.

@mhutti1 mhutti1 closed this as completed Aug 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants