hieuvp / react-native-fingerprint-scanner

Provide Fingerprint, Touch ID, and Face ID Scanner for React Native (Compatible with both Android and iOS)
https://www.npmjs.com/package/react-native-fingerprint-scanner
870 stars 298 forks source link

Android bug on component re-renders #204

Closed valdio closed 2 years ago

valdio commented 2 years ago

NullPointerException on BiometricViewModel

App crashes on android when trying to release biometric and the fingerprint modal is already off-screen. The react component re-render already released biometric or reference to the biometricPrompt has been lost.

I suspect that the issue is triggered at this particular part of the code, FingerprintScanner.release(); when the component dismounts in android.

I have noticed this issue in two cases:

  1. after requesting biometric identification the user directly navigates on another screen, causing the FingerprintScanner.release(); to be called on component dismount. I am using react-navigation in this case.
  2. after appCenter code push is installed and the app reloads on the same component again causing FingerprintScanner.release(); to be called.

Issues:

I had 2 separate issues reported causing NullPointerExceptions on different stages of the release process of the biometric modal, as listed bellow:

NullPointerException
Attempt to invoke virtual method 'androidx.biometric.CancellationSignalProvider androidx.biometric.BiometricViewModel.getCancellationSignalProvider()' on a null object reference

NullPointerException
Attempt to invoke virtual method 'androidx.biometric.BiometricPrompt$CryptoObject androidx.biometric.BiometricViewModel.getCryptoObject()' on a null object reference

All these crashes originate on the same Android native component, BiometricViewModel, related to the prompt requesting the biometric ID.

Possibly this fix also applies to this open issue, but I have no way to test it.

Use case in production build:

This is a screenshot after applying the fix and logging the error.

Screen Shot 2022-01-11 at 15 29 54
valdio commented 2 years ago

I am linking below the sentry stack traces of these errors. So far I have been able to see them only in production builds. https://sentry.io/share/issue/7c41fff8b7254655a55c0533d1276a90/ https://sentry.io/share/issue/6fc8176ea1c04b3b847f61d9bfc2a1ec/ https://sentry.io/share/issue/d9261d7e2ac942dcbce8d3f5201e7ad0/

I also agree that silencing the errors is not the best solution, I just could not find a better approach to handle on React to these cases when the biometric modal has been removed/lost. I will keep looking into this a bit more but feel free to jump in and have a look if you have the time. Would really appreciate it. 🙂

Regarding the NoClassDefFound problem, you are right that my solution does not fix it because it's catching the wrong exception, my bad. Still, that issue is related to the biometric modal, but please ignore it from my description.

mikehardy commented 2 years ago

@valdio I have never done a release on this project and don't have the time to try to learn now, many apologies, but you may rely directly on the commit hash via a direct git reference in your package.json now that it is merged to master, and that will produce stable / reliable / trace-able builds - https://github.com/hieuvp/react-native-fingerprint-scanner/commit/45da58ad1d683a70d13dd49bc4b79cd73a0e0709

valdio commented 2 years ago

@mikehardy I can help you with the release, or I can handle it if you give me access to the npm.js package.

mikehardy commented 2 years ago

Sorry, I'm not comfortable re-delegating any authority I've got here at the moment until I've at least done it once myself. That is a little obstructive (I am sorry) but I feel like any re-delegation comes with the promise that if anything at all breaks, I can fix it, and right now I can't make that statement with any confidence since I haven't done it. A commit hash is hopefully sufficient indefinitely though until I have a chance to actually run a release etc (or someone that already has authority does).

valdio commented 2 years ago

I understand. No problem on my side, just wanted to help.

But using the commit hash is no problem.

Thank you again for merging this fix.