solana-mobile / mobile-wallet-adapter

Other
249 stars 104 forks source link

RuntimeError when calling `wallet.authorize`. Floating bug #541

Closed letehaha closed 1 year ago

letehaha commented 1 year ago

Describe the bug

It's a floating bug. When calling wallet.authorize using transact from @solana-mobile/mobile-wallet-adapter-protocol-web3js lib, to authorize the session on the device that doesn't have wallets installed, it sometimes throws an RuntimeError.

To Reproduce

Fork of solana-mobile/expo-react-native-mwa-proof-of-concept with a reproducable code.

Since the bug is floating, it might not work from the first try, but overall the flow is next:

  1. Make sure your emulator or device doesn't have any wallet installed.
  2. Install APK from fork to emulator or device.
  3. Run npm run dev:mobile from forker repo.
  4. Connect from your emulator or device to Expo.
  5. Try to click "Authorize" fastly and at least 10 times (maybe more).

Expected behavior No RuntimeError, only expected one – [SolanaMobileWalletAdapterError: Found no installed wallet that supports the mobile wallet protocol.].

Smartphone:

Error log

Your app just crashed. See the error below.
java.lang.RuntimeException: Failure delivering result ResultInfo{who=null, request=0, result=0, data=null} to activity {bonfida.sns_manager/bonfida.sns_manager.MainActivity}: java.lang.NullPointerException: Attempt to invoke interface method 'void com.facebook.react.bridge.Callback.invoke(java.lang.Object[])' on a null object reference
  android.app.ActivityThread.deliverResults(ActivityThread.java:5323)
  android.app.ActivityThread.handleSendResult(ActivityThread.java:5362)
  android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:67)
  android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:45)
  android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
  android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
  android.app.ActivityThread$H.handleMessage(ActivityThread.java:2307)
  android.os.Handler.dispatchMessage(Handler.java:106)
  android.os.Looper.loopOnce(Looper.java:201)
  android.os.Looper.loop(Looper.java:288)
  android.app.ActivityThread.main(ActivityThread.java:7872)
  java.lang.reflect.Method.invoke(Native Method)
  com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
  com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)
Caused by java.lang.NullPointerException: Attempt to invoke interface method 'void com.facebook.react.bridge.Callback.invoke(java.lang.Object[])' on a null object reference
  com.facebook.react.bridge.PromiseImpl.reject(PromiseImpl.java:231)
  com.facebook.react.bridge.PromiseImpl.reject(PromiseImpl.java:81)
  com.solanamobile.mobilewalletadapter.reactnative.SolanaMobileWalletAdapterModule$startSession$1$1.invoke(SolanaMobileWalletAdapterModule.kt:84)
  com.solanamobile.mobilewalletadapter.reactnative.SolanaMobileWalletAdapterModule$startSession$1$1.invoke(SolanaMobileWalletAdapterModule.kt:81)
  com.solanamobile.mobilewalletadapter.reactnative.SolanaMobileWalletAdapterModule$mActivityEventListener$1.onActivityResult(SolanaMobileWalletAdapterModule.kt:53)
  com.facebook.react.bridge.ReactContext.onActivityResult(ReactContext.java:338)
  com.facebook.react.ReactInstanceManager.onActivityResult(ReactInstanceManager.java:821)
  com.facebook.react.ReactDelegate.onActivityResult(ReactDelegate.java:90)
  com.facebook.react.ReactActivityDelegate.onActivityResult(ReactActivityDelegate.java:133)
  expo.modules.ReactActivityDelegateWrapper.onActivityResult(ReactActivityDelegateWrapper.kt:170)
  com.facebook.react.ReactActivity.onActivityResult(ReactActivity.java:70)
  android.app.Activity.dispatchActivityResult(Activity.java:8628)
  android.app.ActivityThread.deliverResults(ActivityThread.java:5316)
  android.app.ActivityThread.handleSendResult(ActivityThread.java:5362)
  android.app.servertransaction.ActivityResultItem.execute(ActivityResultItem.java:67)
  android.app.servertransaction.ActivityTransactionItem.execute(ActivityTransactionItem.java:45)
  android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
  android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
  android.app.ActivityThread$H.handleMessage(ActivityThread.java:2307)
  android.os.Handler.dispatchMessage(Handler.java:106)
  android.os.Looper.loopOnce(Looper.java:201)
  android.os.Looper.loop(Looper.java:288)
  android.app.ActivityThread.main(ActivityThread.java:7872)
  java.lang.reflect.Method.invoke(Native Method)
  com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
  com.android.internal.os.ZygoteInit.main(ZygoteInit.java:936)

Video

https://github.com/solana-mobile/mobile-wallet-adapter/assets/12257282/a7434bf0-e7fb-4c80-903c-fc8a49b82866

sdlaver commented 1 year ago

@Michaelsulistio could you take a look at this please?

letehaha commented 1 year ago

Also noticed another RuntimeError. If the user doesn't respond inside the app, after like 70-90 seconds it will crash with a Timeout error from the native code. @sdlaver @Michaelsulistio

Video

This is a full-long video to show the amount of time needed for the crash, so you can rewind it to the end to see the error

https://github.com/solana-mobile/mobile-wallet-adapter/assets/12257282/3b922ae2-3c2c-4f02-be1b-61c481a47199

Error log

 ERROR  Your app just crashed. See the error below.
java.lang.RuntimeException: Could not invoke SolanaMobileWalletAdapter.invoke
  com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:383)
  com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
  com.facebook.jni.NativeRunnable.run(Native Method)
  android.os.Handler.handleCallback(Handler.java:942)
  android.os.Handler.dispatchMessage(Handler.java:99)
  com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
  android.os.Looper.loopOnce(Looper.java:201)
  android.os.Looper.loop(Looper.java:288)
  com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
  java.lang.Thread.run(Thread.java:1012)
Caused by java.lang.reflect.InvocationTargetException
  java.lang.reflect.Method.invoke(Native Method)
  com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
  com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
  com.facebook.jni.NativeRunnable.run(Native Method)
  android.os.Handler.handleCallback(Handler.java:942)
  android.os.Handler.dispatchMessage(Handler.java:99)
  com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
  android.os.Looper.loopOnce(Looper.java:201)
  android.os.Looper.loop(Looper.java:288)
  com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
  java.lang.Thread.run(Thread.java:1012)
Caused by java.util.concurrent.ExecutionException: java.util.concurrent.TimeoutException: Timed out waiting for response with id=1
  com.solana.mobilewalletadapter.common.util.NotifyingCompletableFuture.get(NotifyingCompletableFuture.java:86)
  com.solanamobile.mobilewalletadapter.reactnative.SolanaMobileWalletAdapterModule.invoke(SolanaMobileWalletAdapterModule.kt:124)
  java.lang.reflect.Method.invoke(Native Method)
  com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
  com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
  com.facebook.jni.NativeRunnable.run(Native Method)
  android.os.Handler.handleCallback(Handler.java:942)
  android.os.Handler.dispatchMessage(Handler.java:99)
  com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
  android.os.Looper.loopOnce(Looper.java:201)
  android.os.Looper.loop(Looper.java:288)
  com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
  java.lang.Thread.run(Thread.java:1012)
Caused by java.util.concurrent.TimeoutException: Timed out waiting for response with id=1
  com.solana.mobilewalletadapter.clientlib.protocol.JsonRpc20Client$PendingRequestTimeoutTask.run(JsonRpc20Client.java:279)
  java.util.TimerThread.mainLoop(Timer.java:563)
  java.util.TimerThread.run(Timer.java:513)
Michaelsulistio commented 1 year ago

Investigated these and have a couple of findings:

Bug #1 - Spam clicking authorize when no wallet is available

I tried doing this on a pure React Native app and found that it does not repro. This seems to be an Expo specific issue

Bug #2 - Timeout crash on connect screen

I was able to repro this on a pure React Native app, although it just restarted the app immediately. (This might be a development behavior)

letehaha commented 1 year ago

Thanks, @Michaelsulistio! That's sad about bug #1 and Expo. Maybe you have any idea if that is manageable? The problem is that, if I got it correctly, the error still comes from the lib, so even switching to bare workflow won't help in any way.

Michaelsulistio commented 1 year ago

The error does come from the lib, but I have a hunch it might simply be an issue with Expo + Dev Environment. If you publish a release build of the APK and try it out, it may not crash. I can try this later, but if you get the chance test it out.

@letehaha Is there a specific reason why you would be concerned about this bug? It seems like the repro chance is pretty small, as the user needs to have no wallet installed and needs to spam click.

letehaha commented 1 year ago

@Michaelsulistio sorry forgot to mention, but initially we faced that problem in APK built for prod, but not in Expo dev build. Even when I added throttle with 1000ms to prevent spamming, it was still reproducible in some way, after clicking it multiple times. For now, I increased it to 3000ms, and I personally cannot reproduce it anymore, but I still believe it's better to fix a root cause, but not to work around it. But yes for now surely no rush, cause the chance is really going to zero (I cannot imagine the case when a user installs a crypto-related app with no wallet installed, especially on Saga lol).

Michaelsulistio commented 1 year ago

@letehaha Gotcha, thats really helpful to know that its not just a development behavior. Bug #2 (timeout crash) imo is more likely to reproduce so we'll look into that one first.

Funkatronics commented 1 year ago

@Michaelsulistio this pr fixes the first issue.

Funkatronics commented 1 year ago

@Michaelsulistio and this fixes the second issue

letehaha commented 1 year ago

Thanks a lot, @Funkatronics!