marcshilling / react-native-idle-timer

A cross-platform bridge that allows you to enable and disable the screen idle timer in your React Native app
MIT License
212 stars 41 forks source link

Android crash after start #32

Closed paolosantarsiero closed 6 months ago

paolosantarsiero commented 1 year ago

I build an APK release Android and just open it crash with this log:

com.marcshilling.idletimer.IdleTimerManager.activate(IdleTimerManager.java:41)
at com.marcshilling.idletimer.IdleTimerManager.setIdleTimerDisabled(IdleTimerManager.java:33)

After multiple test opening it works

marcshilling commented 1 year ago

How are you calling this module from your app's code?

paolosantarsiero commented 1 year ago
import IdleTimerManager from 'react-native-idle-timer';
...
// Prevent sleep screen
  useEffect(() => {
    IdleTimerManager.setIdleTimerDisabled(true, 'trucky');

    return () => IdleTimerManager.setIdleTimerDisabled(false, 'trucky');
  }, []);

i can't remove string tag because in typescript the second parameter is required

marcshilling commented 1 year ago

I can't reproduce...it's working fine for me both debug and release builds. I'm going to fix that TypeScript issue with the 2nd param being required though, thanks.

paolosantarsiero commented 1 year ago

Can you try close and reopen app multiple times to reproduce the crash?

Hmoulvad commented 1 year ago

I've updated the package as well, and I am seeing this issue too in the newer versions:

com.marcshilling.idletimer.IdleTimerManager.deactivate IdleTimerManager.java:50
com.marcshilling.idletimer.IdleTimerManager.setIdleTimerDisabled IdleTimerManager.java:35
java.lang.reflect.Method.invoke Method.java
com.facebook.react.bridge.JavaMethodWrapper.invoke JavaMethodWrapper.java:372
com.facebook.react.bridge.JavaModuleWrapper.invoke JavaModuleWrapper.java:188
com.facebook.jni.NativeRunnable.run NativeRunnable.java
android.os.Handler.handleCallback Handler.java:938
android.os.Handler.dispatchMessage Handler.java:99
com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage MessageQueueThreadHandler.java:27
android.os.Looper.loop Looper.java:233
com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run MessageQueueThreadImpl.java:228
java.lang.Thread.run Thread.java:923

Going back to an older version till this is fixed.

marcshilling commented 1 year ago

@wood1986 any idea about this? Seems like it might be related to your change https://github.com/marcshilling/react-native-idle-timer/pull/31

wood1986 commented 1 year ago

I will follow up this tonight

wood1986 commented 1 year ago

I cannot reproduce for both

  useEffect(() => {
    IdleTimerManager.setIdleTimerDisabled(true, 'a');
    return () => {
      IdleTimerManager.setIdleTimerDisabled(false, 'a');
    };
  }, []);

  useEffect(() => {
    IdleTimerManager.setIdleTimerDisabled(true);
    return () => {
      IdleTimerManager.setIdleTimerDisabled(false);
    };
  }, []);

And I do not see typescript error for the second parameter

wood1986 commented 1 year ago

Can someone create a repo for me to reproduce?

Hmoulvad commented 1 year ago

Hey @wood1986 I can tell you this is ONLY happening in the background. It seems that it will look for a tag, and then crash when it doesn't find it :-(

wood1986 commented 1 year ago

@Hmoulvad, I think the crash is because of the null activity.

Hmoulvad commented 1 year ago

@wood1986 would it make sense to check if the activity exists before trying to close it in the background, so we don't end up with a crash?

wood1986 commented 1 year ago

@Hmoulvad it is easy to add the null check but I do not think it makes sense. I try to go to background in new simple react native app and I still cannot reproduce.

At least you need to have the sample code and the repo steps to show me it crashes.

I am concerned I fixed your issue and others could or could not keep the screen on after adding the null check.

Hmoulvad commented 1 year ago

Alright. I will just keep using the older version for now, until I can give you something to replicate.

dppo commented 1 year ago

317015 java.lang.NullPointerException

Attempt to invoke virtual method 'void android.app.Activity.runOnUiThread(java.lang.Runnable)' on a null object reference

com.marcshilling.idletimer.IdleTimerManager.deactivate(IdleTimerManager.java:50)

activity may occur null

@ReactMethod public void setIdleTimerDisabled(final boolean disabled, final String tag) { final Activity activity = this.getCurrentActivity(); if (activity == null) return; if (disabled) { activate(activity, tag); } else { deactivate(activity, tag); } }

glenswift commented 1 year ago

@Hmoulvad what version did you end up using? I'd appreciate it if you can share your experience because I have the same error.

jerinjohnk commented 1 year ago

Recently upgraded our app to the latest version, and this error started popping up in Crashlytics.

"react-native": "0.71.6",
"react-native-idle-timer": "^2.2.1",

Usage

useEffect(() => {
    IdleTimerManager.setIdleTimerDisabled(true);
    return () => {
      IdleTimerManager.setIdleTimerDisabled(false);
    };
  }, []);

Planning to do a release today with a patch fix of adding activity null checks in node_modules/react-native-idle-timer/android/src/main/java/com/marcshilling/idletimer/IdleTimerManager.java

 if (activity == null || activity.isFinishing()) {
            return;
        }
Hmoulvad commented 1 year ago

@Hmoulvad what version did you end up using? I'd appreciate it if you can share your experience because I have the same error.

I havn't done anything besides using old version 2.17.0 and waiting for @marcshilling to be able to reproduce the issue and fix it :-)

wood1986 commented 1 year ago

This crash come from my latest change. Then you guys have to tell if the null check is added and it cannot enable or disable the timer, what the expected behaviour is? Or how do you let developer know the request is failed to perform? And how the developer should deal with that situation?

If you take a look https://github.com/expo/expo/blob/main/packages/expo-keep-awake/android/src/main/java/expo/modules/keepawake/ExpoKeepAwakeManager.kt, it will throw an exception if activity not found.

vasnakos commented 9 months ago

Anynews on that issue?

PS: I downgraded to 2.1.7 in order to avoid crashes

heath-clink commented 6 months ago

Commit bc344737 removes the test for activity != null for some reason. That's a mistake, because it's possible for getCurrentActivity() to return null.