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 crashes in many devices after release #315

Closed
umutpiri opened this issue Nov 9, 2018 · 45 comments · Fixed by #397 or #540
Closed

Android crashes in many devices after release #315

umutpiri opened this issue Nov 9, 2018 · 45 comments · Fixed by #397 or #540
Assignees
Labels
🤖 android Related to android 🐛 bug Something isn't working

Comments

@umutpiri
Copy link

umutpiri commented Nov 9, 2018

Version of react-native-iap

"react-native": "0.55.4"
"react-native-iap": "^2.3.2"

Platforms you faced the error (IOS or Android or both?)

Android

Expected behavior

Please help me, I need to fix this immediately. Because there are already +2k users

Actual behavior

App crashes because of rniap in many devices. In play console, I saw this error causes the crash :
java.lang.RuntimeException:

  1. at com.facebook.react.bridge.CallbackImpl.invoke (CallbackImpl.java:28)
  2. at com.facebook.react.bridge.PromiseImpl.resolve (PromiseImpl.java:30)
  3. at com.dooboolab.RNIap.RNIapModule$4.run (RNIapModule.java:154)
  4. at com.dooboolab.RNIap.RNIapModule$3.onBillingSetupFinished (RNIapModule.java:123)
  5. at com.android.billingclient.api.BillingClientImpl$BillingServiceConnection.onServiceConnected (BillingClientImpl.java:903)
  6. at android.app.LoadedApk$ServiceDispatcher.doConnected (LoadedApk.java:1264)
  7. at android.app.LoadedApk$ServiceDispatcher$RunConnection.run (LoadedApk.java:1281)
  8. at android.os.Handler.handleCallback (Handler.java:815)
  9. at android.os.Handler.dispatchMessage (Handler.java:104)
  10. at android.os.Looper.loop (Looper.java:207)
  11. at android.app.ActivityThread.main (ActivityThread.java:5692)
  12. at java.lang.reflect.Method.invoke (Native Method)
  13. at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:908)
  14. at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:769)

Tested environment (Emulator? Real Device?)

here is the report from play console :
https://ibb.co/gDxZQA

Steps to reproduce the behavior

I don't know what to do, I used rniap like in example.
in componentDidMount :
try {
const result = await RNIap.initConnection();
} catch (err) {
console.log(err);
}
in componentWillUnmount:
RNIap.endConnection();

@hyochan hyochan added the 🤖 android Related to android label Nov 11, 2018
@hyochan
Copy link
Member

hyochan commented Nov 11, 2018

Could you try our recent version 2.3.19 and comeback?

@umutpiri
Copy link
Author

I can't. Because it was an immediate situation. So, I changed the iap package. Now there are no crashes in +3k users. But, thank you. I will try it in my next project.

@Ilario17
Copy link

Ilario17 commented Dec 16, 2018

Hi, can you reopen this issue? I have the same problem and I'm using version 2.3.21

java.lang.RuntimeException:
at com.facebook.react.bridge.CallbackImpl.invoke (CallbackImpl.java:28)
at com.facebook.react.bridge.PromiseImpl.resolve (PromiseImpl.java:30)
at com.dooboolab.RNIap.RNIapModule$4.run (RNIapModule.java:155)
at com.dooboolab.RNIap.RNIapModule$3.onBillingSetupFinished (RNIapModule.java:124)
at com.android.billingclient.api.BillingClientImpl$BillingServiceConnection.onServiceConnected (BillingClientImpl.java:903)
at android.app.LoadedApk$ServiceDispatcher.doConnected (LoadedApk.java:1658)
at android.app.LoadedApk$ServiceDispatcher$RunConnection.run (LoadedApk.java:1687)
at android.os.Handler.handleCallback (Handler.java:789)
at android.os.Handler.dispatchMessage (Handler.java:98)
at android.os.Looper.loop (Looper.java:164)
at android.app.ActivityThread.main (ActivityThread.java:6938)
at java.lang.reflect.Method.invoke (Native Method)
at com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:327)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1374)

this is my componentDidMount

           const itemSKus = Platform.select({
			ios: [
				...
			],
			android: [
				...
			]
		});

		try {
			const message = await RNIap.initConnection();
			//console.log(`message = ${message}`);
			const items = await RNIap.getProducts(itemSKus);
			//console.log(`items = ${items.length}`);
			//console.log(items);
			this.props.storeInAppPurchaseList(items);
		} catch (errorCode) {
			console.log(`error rniap = ${errorCode}`);
		}

@hyochan hyochan reopened this Dec 16, 2018
@hyochan
Copy link
Member

hyochan commented Dec 16, 2018

@LinusU Could you help us out here? Why would callback.run() crashes in release? Should we check whether callback is null in ensureConnection? Hm. it is a runtime exception.

@hyochan hyochan added the 🐛 bug Something isn't working label Dec 16, 2018
@LinusU
Copy link
Member

LinusU commented Dec 16, 2018

Hmmm, what does RuntimeException mean here, is there no more info? @hyochan why do you think that callback was null? 🤔

@hyochan
Copy link
Member

hyochan commented Dec 16, 2018

@LinusU Thanks for your feedback. I've no idea currently. I've just saw issue line below.
image

In RNIapModule.java line 124,

callback.run();

Do you have any further idea?

@hyochan
Copy link
Member

hyochan commented Dec 17, 2018

@LinusU If you don't have any idea about this one, I'd consider rolling this back to where we were at previously.

@LinusU
Copy link
Member

LinusU commented Dec 17, 2018

Okay, here is the problem:

https://github.com/facebook/react-native/blob/370bcffba748e895ad8afa825bfef40bff859c95/ReactAndroid/src/main/java/com/facebook/react/bridge/CallbackImpl.java#L27-L31

For some reason we are calling the callback more than once, should be easy to fix!

@hyochan
Copy link
Member

hyochan commented Dec 17, 2018

@LinusU Looks great because you are in a track! Could you give us a PR for this?

@Ilario17
Copy link

Why is there a RuntimeException in that method?

if (!mInvoked) {
   mJSInstance.invokeCallback(mCallbackId, Arguments.fromJavaArgs(args));
   mInvoked = true;
}

is not something like this enough?

@LinusU
Copy link
Member

LinusU commented Dec 18, 2018

@hyochan sorry, didn't get time to look at this yesterday, hopefully I can take a look soon

@Ilario17 It's probably to flag that there is a logic error somewhere, to be fair, we shouldn't call that method multiple times

edit: ah, the problem comes when onBillingSetupFinished happens more than once. In the ensureConnection we need to remove the listener when we are done (i.e. when we call callback.run() or reject the promise passed in). Should be an easy PR if anyone is up for some free karma! Otherwise I'll try to get to it soon

@hyochan
Copy link
Member

hyochan commented Dec 18, 2018

@LinusU So do you mean onBillingSetupFinished, we should remove the billingClientStateListener? Would someone try this one out to be part of us?

@LinusU
Copy link
Member

LinusU commented Dec 20, 2018

If you want a quick and dirty solution, I would add a local variable to that function, didCallCallback = false. Then guard the call to callback.run() behind if (didCallCallback) { didCallCallback = true; callback.run() }.

A cleaner solution would simply be to deregister the listener after the first call altogether...

@Ilario17
Copy link

Ilario17 commented Dec 20, 2018

Log.d(TAG, "billing client ready");
callback.run();
//deregister the listener here?

hyochan added a commit that referenced this issue Dec 26, 2018
Fixes for #315. Need more tests.
@hyochan
Copy link
Member

hyochan commented Dec 26, 2018

I've just released 2.4.0-beta2 to make fixes on this. Please try.

@Ilario17
Copy link

@hyochan I will try this new version

@hyochan
Copy link
Member

hyochan commented Dec 27, 2018

Please try 2.4.0-beta3 since beta2 had regression issue.

@Ilario17
Copy link

@hyochan after the update the error is still there :(

@hyochan
Copy link
Member

hyochan commented Dec 30, 2018

@Ilario17 What about 2.4.0-beta5? Since I can't reproduce this in my side, I need your test :(

@hyochan hyochan reopened this Dec 30, 2018
@Ilario17
Copy link

Ilario17 commented Jan 4, 2019

@hyochan @LinusU with version 2.4.0-beta5 the error is still there.

@mitchellmcm27
Copy link
Contributor

mitchellmcm27 commented Jan 6, 2019

I have some information relevant to this discussion.

On 2.4.0-beta5, I am getting the following error

Attempt to invoke virtual method void com.android.billingclient.api.BillingClient.querySkuDetailsAsync(...) on a null object reference

pointing to RNIapModule.java line 223. This implies that mBillingClient is unexpectedly null here.

To reproduce: mount a react-native component that calls RNIap.getProducts() on componentDidMount and RNIap.endConnection() on componentWillUnmount. Unmount the component and mount it again. The above error should occur.

The variable clientReady is being used to track if the billing client setup has been completed, by setting it to false in the onBillingServiceDisconnected() callback. But mBillingClient itself is set to null immediately when endConnection() is called. This is a mismatch. It's possible for clientReady to remain set to true, even if mBillingClient is null. I believe this is what is happening, at least for my error. It seems that something is preventing onBillingServiceDisconnected() from being called.

I can confirm that I'm not getting "billing client disconnected" in logcat on unmount as expected, so onBillingServiceDisconnected() is indeed not being called.

Potential Fix: set clientReady = false within the endConnection() method (e.g. after line 179). I've tested this on device and it seems to work. Re-mountoing the in-app purchase component multiple times doesn't seem to result in any errors or crashes. Also, think about if it's necessary to set mBillingClient to null? I don't have any experience here so I can't say, but it does seem a little odd. I also don't see any other projects on Github nulling out the billing client in this callback.

@hyochan
Copy link
Member

hyochan commented Jan 7, 2019

@mitchellmcm27 Thanks for your catch. This is very promising and I agree with you. It was hard for me to fix the problem without any minimal reproduction. Also, I'd like to solve the problems together.

Please give us any PR if you have any thought of enhancing this problem.

Best, regards.

hyochan added a commit that referenced this issue Jan 7, 2019
@mitchellmcm27
Copy link
Contributor

I've released 2.4.0-beta6 in the latest version of my app, with a few thousand upgrades so far. No crashes yet 👍

@hyochan
Copy link
Member

hyochan commented Jan 9, 2019

Sounds great. Thank you so much for the fix! I will close this issue for now then.

@hyochan hyochan closed this as completed Jan 9, 2019
@Ilario17
Copy link

Ilario17 commented Jan 9, 2019

I already have some crash with the same stacktrace but much less than before.


java.lang.RuntimeException: 
  at com.facebook.react.bridge.CallbackImpl.invoke (CallbackImpl.java:28)
  at com.facebook.react.bridge.PromiseImpl.resolve (PromiseImpl.java:30)
  at com.dooboolab.RNIap.RNIapModule$4.run (RNIapModule.java:155)
  at com.dooboolab.RNIap.RNIapModule$3.onBillingSetupFinished (RNIapModule.java:124)
  at com.android.billingclient.api.BillingClientImpl$BillingServiceConnection.onServiceConnected (BillingClientImpl.java:903)
  at android.app.LoadedApk$ServiceDispatcher.doConnected (LoadedApk.java:1658)
  at android.app.LoadedApk$ServiceDispatcher$RunConnection.run (LoadedApk.java:1687)
  at android.os.Handler.handleCallback (Handler.java:789)
  at android.os.Handler.dispatchMessage (Handler.java:98)
  at android.os.Looper.loop (Looper.java:164)
  at android.app.ActivityThread.main (ActivityThread.java:6944)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.Zygote$MethodAndArgsCaller.run (Zygote.java:327)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1374)

@levpopov
Copy link

Also still getting same stacktrace on 2.4.0-beta6

@Ilario17
Copy link

Ilario17 commented Jan 11, 2019

I think this bug should be reopened.

schermata 2019-01-11 alle 10 51 20

120 crash in less than 24 hours.

@LinusU
Copy link
Member

LinusU commented Jan 11, 2019

I'm not sure that the recent changes didn't cause their own problems, e.g. if mBillingClient isn't null, but it isn't yet ready, the ensureConnection function will simply overwrite mBillingClient with a new client, throwing away the old one 🤔

@wasedaigo
Copy link
Contributor

I would like to know which version started showing this error, so that I can roll back to a stable version.

@mitchellmcm27
Copy link
Contributor

I too think the issue should be reopened. The original issue had to do with the callback, and the fixes for that surfaced the "null object reference" error. Although the latter may have been fixed (I agree with @LinusU that there may be more problems there), we are still dealing with the original issue.

I've gotten a few crashes related to the callback in beta6 as well.

@hyochan hyochan reopened this Jan 15, 2019
@hyochan
Copy link
Member

hyochan commented Jan 15, 2019

I've reopened this. Hope someone can give PR for this.

@edo1493
Copy link

edo1493 commented Jan 21, 2019

Do you guys know when this was introduced? I'd be more than happy to roll back.

@dajaffe
Copy link
Contributor

dajaffe commented Jan 23, 2019

I can confirm the crashes exist on beta6 as well as 2.3.23 on Android. Fairly easy to recreate per this [1] comment.

Edit: Now that I'm at home, I'm debugging into the Android native code for beta6. In case this is helpful, this is what's going on (at least in my app, calling getAvailablePurchases):

  1. getAvailableItemsByType for "inapp" gets called without the mBillingClient setup yet.
  2. Billing client gets setup, the onBillingSetupFinished in the listener is called, and the callback (Promise) is consumed (resolved). The listener still exists.
  3. getAvailableItemsByType for "subs" gets called with mBillingClient setup. the new callback (Promise) is consumed (resolved).
  4. Kill the Play Store service - onServiceDisconnected in BillingClientImpl is called, the listener from part 1 handles it. A new service starts up, and the listener from part 1 handles the onBillingSetupFinished, and tries to consume the callback (Promise) from step one is called again.

I'll experiment with killing off the listener once it's no longer needed.

[1]
android/play-billing-samples#45 (comment)

@hyochan
Copy link
Member

hyochan commented Jan 25, 2019

I've merged #379 and released to beta8. Please try that.

@marioot
Copy link

marioot commented Apr 23, 2019

Is this fixed in the latest version? Is it safe to upgrade to latest?

@fokoz
Copy link

fokoz commented Jun 12, 2019

@hyochan this issue still appears in 3.0.0-rc-2 any idea?

java.lang.RuntimeException
com.dooboolab.RNIap.RNIapModule$3.onBillingSetupFinished

java.lang.RuntimeException: 
  at com.facebook.react.bridge.CallbackImpl.invoke (CallbackImpl.java:28)
  at com.facebook.react.bridge.PromiseImpl.resolve (PromiseImpl.java:30)
  at com.dooboolab.RNIap.RNIapModule$3.onBillingSetupFinished (RNIapModule.java:129)
  at com.android.billingclient.api.BillingClientImpl$BillingServiceConnection$1.run (BillingClientImpl.java:1518)
  at android.os.Handler.handleCallback (Handler.java:739)
  at android.os.Handler.dispatchMessage (Handler.java:95)
  at android.os.Looper.loop (Looper.java:148)
  at android.app.ActivityThread.main (ActivityThread.java:7325)
  at java.lang.reflect.Method.invoke (Native Method)
  at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run (ZygoteInit.java:1230)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1120)

hyochan pushed a commit that referenced this issue Jun 12, 2019
@hyochan
Copy link
Member

hyochan commented Jun 12, 2019

@fokoz Could you try 3.0.0-rc-4? It will work perfectly (hopefully).

@fokoz
Copy link

fokoz commented Jun 13, 2019

@hyochan If I use try catch this runtime error won't occur, I think rc-4 should resolve this.
Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Related to android 🐛 bug Something isn't working
Projects
None yet