OneSignal / react-native-onesignal

React Native Library for OneSignal Push Notifications Service
Other
1.56k stars 369 forks source link

Fatal Exception: java.lang.RuntimeException #1015

Closed dembeEdward closed 2 years ago

dembeEdward commented 4 years ago

Description:

fatal Exception: java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code. After introducing onesignal and releasing app to market this issue has started trending in crashlytics. Only on android devices so far.

Environment

  1. Using react-native-onesignal ^3.7.3 on react-native 0.62.1
  2. Using yarn

Steps to Reproduce Issue:

Anything else:

Fatal Exception: java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code. at com.facebook.react.bridge.CallbackImpl.invoke(CallbackImpl.java:26) at com.geektime.rnonesignalandroid.RNOneSignal$6.onComplete(RNOneSignal.java:385) at com.onesignal.OneSignalStateSynchronizer$1$1.run(OneSignalStateSynchronizer.java:231) at android.os.Handler.handleCallback(Handler.java:888) at android.os.Handler.dispatchMessage(Handler.java:100) at android.os.Looper.loop(Looper.java:213) at android.app.ActivityThread.main(ActivityThread.java:8178) at java.lang.reflect.Method.invoke(Method.java) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:513) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1101)

rgomezp commented 3 years ago

Howdy, Thanks for your patience. Are you still seeing this issue?

If you could provide reproduction steps that would be greatly helpful. Thanks

rgomezp commented 3 years ago

Closing due to no response.

I will reopen if someone provides reproduction steps/example code.

Cheers

fedenelli commented 3 years ago

Hi, I am seeing this exact same problem (same stack trace). This happens only on some Android devices (I have seen it on Samsung S9 and Samsung J6 so far).

My app made all this things at the same time:

This is done when the user is logged in or reopens the app. Could this be the source of the problem? Should I wait for the getPermissionSubscriptionState to resolve before setting the external user id and sending the tags?

Thank you

rgomezp commented 3 years ago

Howdy, Can you please include your code? Particularly when it comes to setting the external id. This is occurring due to the callback firing twice in the Android bridge. I am not particularly sure why it is firing twice but it certainly could have to do with how you are calling it.


Coming from here.

fedenelli commented 3 years ago

Of course, I have two files: One with the redux reducer that handles the login, and other with the device status reporting.

import Config from 'react-native-config';
import OneSignal from 'react-native-onesignal';
import {LOGIN, LOGOUT, USER, UPDATING} from './actions';
import AnalyzeDevice from '../../services/deviceinfo';

const authReducer = (state, action) => {
  switch (action.type) {
    case LOGIN:
      const user = action.data;
      if (user) {
        if (user.email !== Config.GUEST_EMAIL) {
          AnalyzeDevice(user._id); // this calls OneSignal.getPermissionSubscriptionState()
        }
        OneSignal.setExternalUserId(user._id);
        OneSignal.sendTags({
          id: user._id,
          logged: true,
        });
      }
      return {
        ...state,
        token: action.token,
        user: action.data,
        logged: true,
      };
    default:
      return state;
  }
};

export default authReducer;

This is the one that checks the subscription state:

import axios from 'axios';
import Config from 'react-native-config';
import DeviceInfo from 'react-native-device-info';
import OneSignal from 'react-native-onesignal';

const Notifications = () =>
  new Promise((resolve) =>
    OneSignal.getPermissionSubscriptionState((status) => resolve(status)),
  );

const AnalyzeDevice = async (userId) => {
  const isEmulator = await DeviceInfo.isEmulator();

  if (isEmulator) {
    return false;
  }

  const NotificationData = await Notifications();

  const result = {
    oneSignalSubscription: NotificationData.subscriptionEnabled,
    remoteNotifications: NotificationData.notificationsEnabled,
    userId,
  };

  try {
    const response = await axios.post(Config.DEVICE_INFO_URL, result);

    return response.data;
  } catch (err) {
    console.warn(err);
    throw err;
  }
};

export default AnalyzeDevice;

Hope this helps. Please let me know if you need any further information.

rgomezp commented 3 years ago

Howdy, Maybe place a log in your reducer LOGIN case to make sure it is only firing once. Also, consider adding a callback that logs the result of setting the external id...

// Setting External User Id with Callback Available in SDK Version 3.7.0+
OneSignal.setExternalUserId(externalUserId, (results) => {
  // The results will contain push and email success statuses
  console.log('Results of setting external user id');
  console.log(results);

  // Push can be expected in almost every situation with a success status, but
  // as a pre-caution its good to verify it exists
  if (results.push && results.push.success) {
    console.log('Results of setting external user id push status:');
    console.log(results.push.success);
  }

  // Verify the email is set or check that the results have an email success status
  if (results.email && results.email.success) {
    console.log('Results of setting external user id email status:');
    console.log(results.email.success);
  }
});
fedenelli commented 3 years ago

I can confirm that the LOGIN is being fired just once. I have refactored the reducer using redux-thunk, and now I fire AnalyzeDevice and OneSignal.sendTags on the callback of OneSignal.setExternalUserId. I will let you know if this works, I will be deploying the update on the next few days.

fedenelli commented 3 years ago

After 2 days on production, I still see this errors. After reviewing the whole process, I see that immediately after the login, the app triggers an update in the local database with another call to OneSignal.sendTags. So that method is called on the LOGIN reducer and then again on the user update, almost simultaneously. Do you think this could be the reason of the crashes?

rgomezp commented 3 years ago

Howdy @fedenelli , Yes. That is most likely the reason in my opinion. Try to remove one of them so that the calls aren't being made simultaneously. Cheers

rgomezp commented 3 years ago

Howdy @fedenelli , Can you please update this issue?

fedenelli commented 3 years ago

Sure @rgomezp , the change certainly reduced the exceptions but the problem keeps happening. I haven't found time to try new things yet, but next week I am gonna try an update removing the call to OneSignal.getPermissionSubscriptionState() and see what happens.

If you think there is another thing to try, please let me know so I can work on it and release it in the next update.

Thank you

rgomezp commented 3 years ago

Howdy @fedenelli , I haven't heard from you in some time. Can you please update this issue when you get a chance?

nicoraynaud commented 3 years ago

We are having the same issue currently, was hoping this bug would be fixed by now, any news ??

rgomezp commented 3 years ago

@nicoraynaud , Have you been able to reproduce this on the Beta release?

rgomezp commented 3 years ago

Closing due to no response.

jfishman1 commented 3 years ago

Received another report of this issue with this code:

// Once we get a user id in our code base, we call this
export async function initTracking(user_id) {
  // some other analytics code here with Amplitude, nothing that is related to OneSginal

  OneSignal.setExternalUserId(user_id);
  OneSignal.sendTag("email", user_id);
}

Call stack of crash:

java.lang.RuntimeException:
at com.facebook.react.bridge.CallbackImpl.invoke (CallbackImpl.java:28)
at com.geektime.rnonesignalandroid.RNOneSignal$6.onComplete (RNOneSignal.java:385)
at com.onesignal.OneSignalStateSynchronizer$1$1.run (OneSignalStateSynchronizer.java:230)
at android.os.Handler.handleCallback (Handler.java:883)
at android.os.Handler.dispatchMessage (Handler.java:100)
at android.os.Looper.loop (Looper.java:237)
at android.app.ActivityThread.main (ActivityThread.java:7948)
at java.lang.reflect.Method.invoke (Native Method)
at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:493)
at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1075)

I just found that calling setExternalUserId() 10 times triggers the callback passed to it only once, so there is definitely something on the JS or Java side that checks that this function has already been called and I supposed that there is a race condition that could get it to think it has not already been called even though it has been. I will just add a check on he JS side to make sure it's only called once in the process lifetime.

rgomezp commented 3 years ago

@fedenelli , Could you please update this issue with the status of your findings? Were you able to resolve?

@jfishman1 , for the code shown above a workaround could be to put the sendTag in the callback for setExternalUserId. At least it should reduce the errors while we fix the problem.

rgomezp commented 3 years ago

I haven't been able to reproduce unfortunately. Can someone please reach out to our support channel at OneSignal.com with an example app that reproduces the issue? Cheers

lasryaric commented 3 years ago

Hi, I am the one who posted the data from @jfishman1 message via the onesignal customer support.

For info, it happened to 16000 of our users base with 36400 occurrences total.

I was never able to reproduce it myself. Should we just serialize in a Javascript queue all our calls to onesignal function @rgomezp ?

lasryaric commented 3 years ago

up @rgomezp

rgomezp commented 3 years ago

Howdy @lasryaric , Thanks for your patience.

You can give that a shot although it's certainly not the ideal solution. This needs to be fixed if it is indeed an issue on the SDK side.

For you and anyone else, it looks like errors in the setExternalUserId native method in the Java bridge doesn't handle the failure case and log to the native logcat. However, the latest beta release includes changes to the setExternalUserId method that would log any issues were this call to fail.

I recommend folks migrate to the beta which includes many changes that either a) might fix this issue altogether or b) at least provide better logging to diagnose and resolve this problem.

If anyone manages to reproduce this, please please reach out to us via our support channel and provide us with a private link or download to reproduce it in an example app.

Cheers

rgomezp commented 3 years ago

Quick update on this issue.

It looks like the two main factors of this problem are:

  1. the use of both the email and external id channels via the functions setEmail and setExternalUserId
  2. the use of a callback (and default function passed by the SDK for omitted callbacks) to the setExternalUserId function in the Java bridge

This will need to be resolved by OneSignal and is not resolved in the latest version 4.0.1.

We will get a fix out for this as soon as we possibly can.

Thanks to lasryaric for your cooperation.

rgomezp commented 3 years ago

Howdy, I just wanted to update this issue. The underlying cause of the problem is actually a native issue. This will need to get resolved permanently in the Android native SDK.

However, we have shipped an SDK that you can workaround this problem on in the 4.0.3 release.

To summarize: you will need to avoid passing a callback to the setExternalUserId function to workaround the problem until the native issue is resolved.

Thanks for your patience and for surfacing the issue.

Enjoy!

torufuruya commented 2 years ago

Hi, @rgomezp. I know this issue is already closed but could you kindly double-check if the issue on the native side has been fixed or not? I couldn't find anywhere mention about the issue.

nan-li commented 2 years ago

Hi @torufuruya,

Can you share more information about your error? Such as logs, reproduction steps, your code that is calling these OneSignal functions.

torufuruya commented 2 years ago

@nan-li This is all I have. I have never reproduced it. I got the log from Sentry.

java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code.
    at com.facebook.react.bridge.CallbackImpl.invoke(CallbackImpl.java:26)
    at com.geektime.rnonesignalandroid.RNOneSignal$8.onSuccess(RNOneSignal.java:476)
    at com.onesignal.OneSignalStateSynchronizer$1$1.run(OneSignalStateSynchronizer.java:277)
    at android.os.Handler.handleCallback(Handler.java:873)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loop(Looper.java:214)
    at android.app.ActivityThread.main(ActivityThread.java:7050)
    at java.lang.reflect.Method.invoke(Method.java)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:965)

Notes:

nan-li commented 2 years ago

@torufuruya Thank you, this callback exception still seems to be an issue.

manhquyen commented 2 years ago

I have same issue on firebase analysics please help

Fatal Exception: java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code.
       at com.facebook.react.bridge.CallbackImpl.invoke(CallbackImpl.java:26)
       at com.geektime.rnonesignalandroid.RNOneSignal$6.onComplete(RNOneSignal.java:385)
       at com.onesignal.OneSignalStateSynchronizer$1$1.run(OneSignalStateSynchronizer.java:230)
       at android.os.Handler.handleCallback(Handler.java:938)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loop(Looper.java:246)
       at android.app.ActivityThread.main(ActivityThread.java:8506)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:602)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1130)
torufuruya commented 2 years ago

After my PR was merged, it stop crashing at removeExternalUserId, but a similar crash arises from another place of code. This time, it seems to be setEmail. We probably need to patch there as well, or all places where define default callback for Android 😵‍💫 cc: @nan-li

java.lang.RuntimeException: Illegal callback invocation from native module. This callback type only permits a single invocation from native code.
    at com.facebook.react.bridge.CallbackImpl.invoke(CallbackImpl.java:26)
    at com.geektime.rnonesignalandroid.RNOneSignal$2.onSuccess(RNOneSignal.java:303)
    at com.onesignal.OneSignal.fireEmailUpdateSuccess(OneSignal.java:3079)
    at com.onesignal.UserStatePushSynchronizer.onSuccessfulSync(UserStatePushSynchronizer.java:229)
    at com.onesignal.UserStateSynchronizer$3.onSuccess(UserStateSynchronizer.java:376)
    at com.onesignal.OneSignalRestClient$5.run(OneSignalRestClient.java:269)
    at java.lang.Thread.run(Thread.java:923)
dan-f commented 2 years ago

My team is affected by this issue as well within the context of setExternalUserId. We're on version 4.3.3.

It appears that a similar issue https://github.com/OneSignal/react-native-onesignal/issues/484 was closed out without resolution.

@rgomezp - hope you're the right person to tag here. It's basically an unacceptable option for us to stop passing callbacks as you mentioned above. We need to keep track of player IDs for later use, as your various REST APIs rely on them. The subscription methods aren't a great solution for us because if there was an error, say with OneSignal.setSMSNumber, my understanding is that we wouldn't get notified of the error via the subscription handler, so we'd have no idea if and when that value would get set -- but correct me if I'm wrong.

We need a way to know definitively, for each push/sms/email channel, whether we've got the player ID, or whether we've bottomed out with a failure. Could we please have a clearer path to resolution here?

rgomezp commented 2 years ago

@dan-f , Thanks for the info.

We are working on a permanent fix for this issue in #1341.

Thanks for your patience

dan-f commented 2 years ago

Hi @rgomezp. Your permanent fix is continuing to crash out our users on 4.3.7. For example right on this line with the exception:

RuntimeException

Illegal callback invocation from native module. This callback type only permits a single invocation from native code.

I really want us to not need to continually monkey-patch this with each new release of the SDK. Now that I'm looking at it, I don't see why this block of code in the fix would have solved the bug...:

         @Override
         public void onSuccess(JSONObject results) {
            Log.i("OneSignal", "Completed setting external user id: " + externalId + "with results: " + results.toString());

            Callback callbackCopy = callback;
            if (callbackCopy != null) {
               callbackCopy.invoke(RNUtils.jsonToWritableMap(results));
               callbackCopy = null;
            }
         }

...since if callback is non-null, callbackCopy will never be null preceding the if; assigning callbackCopy to null does not change callback.

Do you have a timeline for a permanent fix here? I need to determine whether I'm going to have to monkey patch for our users by the time we run our next release.

jkasten2 commented 2 years ago

@dan-f Thanks for your feedback. We re-reviewed the code here and see that the handling here won't prevent double calls to the callback. The callbackCopy being set to null inside the if isn't the kind of logic that will prevent the double callback. We will provide an update as we get close to a fix.

rgomezp commented 2 years ago

Thanks for your patience. This will be fixed in the next release.