search

tschoffelen / react-native-email-link

📭 Open an email client from React Native (for 'magic link' type functionality).
MIT License
409 stars 74 forks source link

Added additional null check to prevent App crash #28

Closed johnnyborg closed 5 years ago

johnnyborg commented 5 years ago

Whats this?

We've gotten a few reports in Sentry about our App crashing coming from this library. I've written a fix for this. The drawback of this change is that the user will not get any feedback, but the method will not crash the app anymore. No window displaying possible apps to run is shown anymore. If you want a good view of the changes, i suggest using the ignore whitespace option (use the gear icon).

Exception data:

java.lang.NullPointerException: Attempt to invoke virtual method 'int android.content.Intent.getFlags()' on a null object reference
    at android.content.Intent.createChooser(Intent.java:930)
    at android.content.Intent.createChooser(Intent.java:891)
    at com.facebook.react.modules.email.EmailModule.open(EmailModule.java:42)
    at java.lang.reflect.Method.invoke(Method.java)
    at java.lang.reflect.Method.invoke(Method.java:372)
    at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
    at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:158)
    at com.facebook.react.bridge.queue.NativeRunnable.run(NativeRunnable.java)
    at android.os.Handler.handleCallback(Handler.java:739)
    at android.os.Handler.dispatchMessage(Handler.java:95)
    at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
    at android.os.Looper.loop(Looper.java:145)
    at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:232)
    at java.lang.Thread.run(Thread.java:818)

How to test

  1. Use an Android simulator with API version 22 without Google Services.
  2. Run a project and make sure it calls openInbox(); somewhere.

The device familiy in the report was SM-J320FN (Samsung) with Android 5.1.1.

tschoffelen commented 5 years ago

Thanks so much, looks very good!

Suggested one change, once you've applied that (or disagreed with me, also possible 😄), I'll immediately merge this and cut a new release.

johnnyborg commented 5 years ago

Yeah, cutting down on cyclomatic complexity seems like a good idea.

The method queryIntentActivities() returns an list with one item: fallback activity/package. However a default Android mail app is installed but not configured. Maby it can not provide an Intent? I am not familiar enough with the Android eco system, is it possible that works like this?

  1. Fallback (failed to find anything for the default mail app)
  2. G-mail

Adding the return will block the request to the G-mail app to provide an Intent, but i am only assuming this and my knownledge is not sufficient enough. Can you elaborate on this, @tschoffelen?

johnnyborg commented 5 years ago

image Seems like the fallback is simulator only see:

Meaning getLaunchIntentForPackage() failed on the device on an unknown app. The return in this case will block the display of the App chooser. I you agree @tschoffelen , i'd rather leave it in.

tschoffelen commented 5 years ago

I think I understand what you mean. Alright, let's leave it as is!

tschoffelen commented 5 years ago

Released to NPM as react-native-email-link@1.5.1