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

Better error message if supported ABI's cannot be found inside the APK. #65

Merged
merged 1 commit into from Jan 6, 2020

Conversation

cmelchior
Copy link
Contributor

@cmelchior cmelchior commented Nov 27, 2019

This PR adds a better error message if an ABI cannot be successfully located.

This is helpful as some app users are side-loading APKs outside the Play Store. If those apps have been built using App Bundle or APK Split, it could result in users loading an APK that doesn't support their device, which results in a rather unhelpful error message like com.getkeepsafe.relinker.MissingLibraryException: libtest.so. This isn't particularly helpful for an app developer tracking their crashes. Especially if they are using a 3rd party library that includes native code. In that case, it could look like it is a bug in the 3rd party library.

With this PR, the error message now tries to be more descriptive about what has been tried and what is actually supported by the device, so the error message will now look like this:

Could not find 'libtest.so'. Looking for: [armeabi-v7a], but only [x86, arm64-v8a] are supported.

@Tweener
Copy link

Tweener commented Dec 27, 2019

Hi there,

Any idea when this PR will be merged/released?
It is apparently blocking the mentioned PR for Realm Java (realm/realm-java#6673), which crashes the app for a significant percentage of users.

Cheers

Copy link
Contributor

@emarc-m emarc-m left a comment

Choose a reason for hiding this comment

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

Hi @cmelchior, Thank you for this PR. I apologize for not getting into this sooner. Please see my comments and request another review once they have been addressed.

I will take care of merging this PR and releasing a new binary with these changes. Tentative release would be around the 2nd week of January.

@emarc-m
Copy link
Contributor

emarc-m commented Dec 28, 2019

Hi there,

Any idea when this PR will be merged/released?
It is apparently blocking the mentioned PR for Realm Java (realm/realm-java#6673), which crashes the app for a significant percentage of users.

Cheers

Please see: #65 (review) for tentative release with these changes.

@cmelchior
Copy link
Contributor Author

Ready for 2nd round @emarc-m

@emarc-m
Copy link
Contributor

emarc-m commented Jan 6, 2020

Ready for 2nd round @emarc-m

Thank you for addressing the comments.

@emarc-m emarc-m merged commit 0e19739 into KeepSafe:master Jan 6, 2020
@emarc-m emarc-m mentioned this pull request Jan 6, 2020
@emarc-m
Copy link
Contributor

emarc-m commented Jan 6, 2020

@cmelchior (cc: @Tweener) version 1.4.0 has been released which includes these changes. Thank you for the contribution.

@cmelchior
Copy link
Contributor Author

Awesome. Thanks 👏

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.

None yet

3 participants