Phone auth (auth/popup-closed-by-user)

[iM-GeeKy]

I'm following the MFA-like Account Creation and it's been super simple to follow and works great. However, in local development I'm running into an issue where canceling the reCAPTCHA results in an unhandled promise (successfully verifying seems to work as expecte). The code that seems to be at play here is:

  // Handle the verify phone button press
  async function verifyPhoneNumber(phoneNumber) {
    const confirmation = await auth().verifyPhoneNumber(phoneNumber);
    setConfirm(confirmation);
  }

and wrapping this in a try/catch doesn't seem to resolve the issue. Is there a way to catch this so it can be gracefully handled? I'd like to catch it in order to stop the progress bar from animating as seen in the video below.

Screen.Recording.2024-02-22.at.2.15.48.PM.mov

[mikehardy]

Interesting - I wonder if this whole block:

react-native-firebase/packages/auth/ios/RNFBAuth/RNFBAuthModule.m

Lines 984 to 1010 in 3c0dc54

	firebaseApp.name)[[FIRPhoneAuthProvider providerWithAuth:[FIRAuth authWithApp:firebaseApp]]
	verifyPhoneNumber:phoneNumber
	UIDelegate:nil
	completion:^(NSString *_Nullable verificationID, NSError *_Nullable error) {
	if (error) {
	NSDictionary *jsError = [self getJSError:error];
	NSDictionary *body = @{
	@"type" : @"onVerificationFailed",
	@"requestKey" : requestKey,
	@"state" : @{@"error" : jsError},
	};
	[RNFBSharedUtils sendJSEventForApp:firebaseApp
	name:PHONE_AUTH_STATE_CHANGED_EVENT
	body:body];
	} else {
	NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];
	[defaults setObject:verificationID forKey:@"authVerificationID"];
	NSDictionary *body = @{
	@"type" : @"onCodeSent",
	@"requestKey" : requestKey,
	@"state" : @{@"verificationId" : verificationID},
	};
	[RNFBSharedUtils sendJSEventForApp:firebaseApp
	name:PHONE_AUTH_STATE_CHANGED_EVENT
	body:body];
	}
	}];

Might need a @try/@catch wrapper, where the catch had a block like so:

react-native-firebase/packages/auth/ios/RNFBAuth/RNFBAuthModule.m

Line 193 in 3c0dc54

[self promiseRejectAuthException:reject error:error];

Can you try this locally since you have a reproduction case all set up and reproducing? reaching right into node_modules and just hacking at the objective-c to put the try/catch in and handle the catch with that reject should be pretty quick

If it works "patch-package" is the trick to make it persistent https://github.com/ds300/patch-package and a PR would be fantastic

[mikehardy]

I think the try catch needs to be outside the completion handler as I believe we're getting a native error that is outside the completion handler scope otherwise we'd hit that error handler inside it. So outside of the entire dlog even?

I believe you're correct about the scoping of the error symbol regardless, but that's even more out of scope if try catch is expanded to encompass the whole call to sdk

[iM-GeeKy]

Good catch, I updated the snippet. Would it be okay to pass the exception to error (are they compatible)?

[mikehardy]

I could be wrong (hope not, but of course could be...) but they seem to be compatible. Editing it directly in Xcode will tell the tale of course as I believe Xcode will complain if the shapes don't line up

[iM-GeeKy]

I tried to build with that and it looks like reject is also out of scope. I opened it in Xcode and it didn't seem to complain and i'm not sure building and checking is a terribly effective way to debug as it takes 15-20min to build and each failed build uses up a build against my build quota (I have 2 iOS builds left for the month).

❌  (node_modules/@react-native-firebase/auth/ios/RNFBAuth/RNFBAuthModule.m:1015:50)

  1013 |              }];
  1014 |              } @catch (NSException *exception) {
> 1015 |                 [self promiseRejectAuthException:reject error:exception]; 
       |                                                  ^ use of undeclared identifier 'reject'
  1016 |              }
  1017 | }
  1018 | 

[iM-GeeKy]

I'll also note Android appears to not give any result (such as "auth/popup-closed-by-user") when the user cancels out of the recaptcha or a confirmation callback upon successful verification. Furthermore I had to add

// Handle the verify phone button press
  async function verifyPhoneNumber(phoneNumber) {
    // Added the next 2 lines to get Android recaptch to show..
    auth().settings.appVerificationDisabledForTesting = false;
    auth().settings.forceRecaptchaFlowForTesting = true;
    const confirmation = await auth().verifyPhoneNumber(phoneNumber);
    setConfirm(confirmation);
  }

in order to get the recaptcha to show on the android emulator. Does this functionality not work on emulator or device, it just seems to timeout after verification? I found a comment in one of the tests that might hint at that fact (7c082be).

test.mov

[iM-GeeKy]

@mikehardy Do you have any further insight into this?

[mikehardy]

Not at the moment - sorry. Kind shocking you literally can't build locally, I'll admit I'm surprised by that - that is a very interesting development cycle / development experience.

I did look back at this last night actually but did not have new insight - the items you've brought up make it appear as if there are at least a couple things going on at once and they each need some specific investigation via exploratory code changes and e2e testing support, but I haven't had the time quite yet and if you literally can't do local builds that really slows things down

[iM-GeeKy]

Hey, thanks for the quick reply. I can build locally (https://docs.expo.dev/build-reference/local-builds/) without having to rebuild the native code on Expo servers via eas build, but more often than not, no native changes are required.

From what i've gathered it does seem like there are two problems.

1. Closing the recaptcha, doesn't appear to be handled properly on iOS or android (device or simulator).
2. Verifying the recatpcha always times out on android after successful verification (device or emulator).

[iM-GeeKy]

Do you have a timeframe when you may take a deeper dive into this? I'd be happy to help, since you have a better working knowledge of the codebase. I do plan on tackling this soon, because this bug is blocking me from integrating and testing phone verification logic in my application. Is this a good spot to continue posting questions, as I start jumping into the code?

[iM-GeeKy]

@mikehardy I've started to look into 1. Closing the recaptcha, doesn't appear to be handled properly on iOS or android (device or simulator) and it appears to be handling the error properly, but the function

  _verificationFailedHandler(state) {
      const snapshot = {
        verificationId: state.verificationId,
        code: null,
        error: null,
        state: 'error',
      };
  
      snapshot.error = new NativeFirebaseError({ userInfo: state.error }, this._jsStack, 'auth');
      this._emitToObservers(snapshot);
      this._emitToErrorCb(snapshot); // Changed to this._emitToSuccessCb(snapshot) and it at least passes the snapshot back to the client.
      this._removeAllListeners();
 
  }

never seems to pass the snapshot back to the client. However, if you use this._emitToSuccessCb(snapshot) instead of this._emitToErrorCb(snapshot) it at least passes the snapshot back to the client. It doesn't seem like the best solution, but might work as a patch temporarily.

[iM-GeeKy]

As for the second point 2. Verifying the recatpcha always times out on android after successful verification (device or emulator)."There is a bit of logic that seems to be causing the issue

_codeSentHandler(credential) {
    const snapshot = {
      verificationId: credential.verificationId,
      code: null,
      error: null,
      state: 'sent',
    };
    this._emitToObservers(snapshot);

    if (isIOS) {
      this._emitToSuccessCb(snapshot);
    }

    if (isAndroid) {
      // android can auto retrieve so we don't emit to successCb immediately,
      // if auto retrieve times out then that will emit to successCb
    }
  }

Notice the last conditional. This seems to be the culprit. Is there a way of emitting if auto retrieve is not being used? It wouldn't seem like the best user experience to have to wait until a timeout to get the response back. Not sure if that's possible to determine if it has been auto retrieved, but I would think if it's android and it is auto retrieved then we don't emit the success callback immediately.

Also, it appears iOS autoverifies the recaptcha so I'm not sure if that is the same is "auto retrieve". If so, perhaps that conditional can be ommited?

[iM-GeeKy]

Below is my patch. I'm not sure how permanent it is, but it solves the issues presented in the videos above.

diff --git a/node_modules/@react-native-firebase/auth/lib/PhoneAuthListener.js b/node_modules/@react-native-firebase/auth/lib/PhoneAuthListener.js
index 9d5378a..7381e84 100644
--- a/node_modules/@react-native-firebase/auth/lib/PhoneAuthListener.js
+++ b/node_modules/@react-native-firebase/auth/lib/PhoneAuthListener.js
@@ -135,15 +135,7 @@ export default class PhoneAuthListener {
     };
 
     this._emitToObservers(snapshot);
-
-    if (isIOS) {
-      this._emitToSuccessCb(snapshot);
-    }
-
-    if (isAndroid) {
-      // android can auto retrieve so we don't emit to successCb immediately,
-      // if auto retrieve times out then that will emit to successCb
-    }
+    this._emitToSuccessCb(snapshot);
   }
 
   _codeAutoRetrievalTimeoutHandler(credential) {
@@ -182,7 +174,7 @@ export default class PhoneAuthListener {
     snapshot.error = new NativeFirebaseError({ userInfo: state.error }, this._jsStack, 'auth');
 
     this._emitToObservers(snapshot);
-    this._emitToErrorCb(snapshot);
+    this._emitToSuccessCb(snapshot);
     this._removeAllListeners();
   }

[SoyDiego]

Hi @mikehardy @Salakar @Ehesp, any help here?
I would like to know how to proceed with this problem or the patch will temporarily suffice @iM-GeeKy.
Thanks :)