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

Android Only: closing share bottom sheet causes app crash #1291

Closed
princefishthrower opened this issue Oct 13, 2022 · 5 comments
Closed

Android Only: closing share bottom sheet causes app crash #1291

princefishthrower opened this issue Oct 13, 2022 · 5 comments
Labels

Comments

@princefishthrower
Copy link
Contributor

princefishthrower commented Oct 13, 2022

Steps to reproduce

  1. Trigger a Share.open() call on an Android device.
  2. Close the bottom sheet that appears without sharing or tapping anything on it
  3. The android app will crash and close

Expected behaviour

The app should not crash.

Actual behaviour

The app crashes.

Environment

  • React Native version: 0.67.2
  • React Native platform + platform version: iOS 9.0, Android 5.0, etc 0.67.2 - Android 10-13 (at least that I have tested, maybe more android versions are affected by this)
  • Typescript version (if using typescript): 3.8+ required, what version is in your environment? 4.4.3

react-native-share

Version: npm version or "master" npm 8.19.2

Link to repo (highly encouraged)

I could put this together but I would need time. Good evidence that this is still a bug is shown in #1024 but was marked eventually as stale

Wish I could provide more info, but I don't get any logs from react native, just the immediate crash. Also as mentioned we have tested this on numerous iOS and Android devices and it seems to effect only Android (both emulator and physical devices)

@princefishthrower princefishthrower changed the title Android Only: closing sheet Android Only: closing share bottom sheet causes app crash Oct 13, 2022
@mikehardy
Copy link
Collaborator

Interesting...

The app crashes.

All reports of crashes should be accompanied by the crash stack trace, can you paste it in here as text?

@princefishthrower
Copy link
Contributor Author

Hmmm in the metro console I don't get anything... is there a flag I can pass to it to get a higher amount / level of logs?

@mikehardy
Copy link
Collaborator

adb logcat

@princefishthrower
Copy link
Contributor Author

princefishthrower commented Oct 14, 2022

Ah perfect, thank you! Uh oh, looks like this could be a problem from our side. I will double-check what we were passing as the message, but I thought it was just a plain string... Will update shortly with investigation:

10-14 08:15:27.842 23452 23548 E AndroidRuntime: FATAL EXCEPTION: mqt_native_modules
10-14 08:15:27.842 23452 23548 E AndroidRuntime: Process: myapp.com, PID: 23452
10-14 08:15:27.842 23452 23548 E AndroidRuntime: com.facebook.react.bridge.UnexpectedNativeTypeException: Value for message cannot be cast from ReadableNativeMap to String
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.ReadableNativeMap.checkInstance(ReadableNativeMap.java:140)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.ReadableNativeMap.getNullableValue(ReadableNativeMap.java:128)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.ReadableNativeMap.getString(ReadableNativeMap.java:162)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.modules.dialog.DialogModule.showAlert(DialogModule.java:195)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at java.lang.reflect.Method.invoke(Native Method)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at android.os.Handler.handleCallback(Handler.java:751)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at android.os.Handler.dispatchMessage(Handler.java:95)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at android.os.Looper.loop(Looper.java:154)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:226)
10-14 08:15:27.842 23452 23548 E AndroidRuntime: 	at java.lang.Thread.run(Thread.java:761)
10-14 08:15:27.844  1718  2664 W ActivityManager:   Force finishing activity myapp.com/.MainActivity

UPDATE: the variable we are using as message is indeed a string, looks something like this:

Check out this free app and join my experience.✨ 🌱️
https://myapp.page.link/share

and I console log out typeof message as a double check, it's a string. I wonder though, because we have a call to an i18n translate function... though this returns a string... then my only remaining guess is that it can be the newline character or the emojis throwing it off? WIll do some more experiments.

UPDATE: unfortunately this same exception occurs even when I pass an empty options object, i.e.:

await Share.open({})

princefishthrower added a commit to princefishthrower/react-native-share that referenced this issue Oct 14, 2022
As the title states, this small doc change would have saved me from the issue I raised in react-native-share#1291

Totally flexible about the wording or arrangement, I think it's just important to mention somewhere in the docs that closing the sheet throws an error.
@princefishthrower
Copy link
Contributor Author

princefishthrower commented Oct 14, 2022

Finally found the true cause here: I switched our code around the Share() call from then / catch to async / await and simply forgot to handle the case where a sheet is closed. I didn't realize that closing the sheet throws an error, which essentially requires you to wrap a try / catch around the await Share() call. Not sure if it makes the most sense that closing the share sheet without sharing throws an error, but anyway as for the scope of this issue it's resolved :)

In summary, I believe the docs could be updated to mention this thrown error and show:

try {
    const shareResponse = await Share.open(options);
} catch (error) {
    // when user closes share sheet, other errors etc
}

since some people may assume that closing the sheet just affects what is inside shareResponse. I added this to a pull request on the docs here: #1292

MateusAndrade pushed a commit that referenced this issue Oct 14, 2022
#1292)

As the title states, this small doc change would have saved me from the issue I raised in #1291

Totally flexible about the wording or arrangement, I think it's just important to mention somewhere in the docs that closing the sheet throws an error.
mobiledev7 added a commit to mobiledev7/react-native-share that referenced this issue Dec 12, 2022
…d (#1292)

As the title states, this small doc change would have saved me from the issue I raised in react-native-share/react-native-share#1291

Totally flexible about the wording or arrangement, I think it's just important to mention somewhere in the docs that closing the sheet throws an error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants