Repository navigation
fix(auth, android): report an empty email or password as missing-email/missing-password instead of channel-error - #18876
Conversation
…l/missing-password instead of channel-error
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Android Firebase Auth plugin to catch IllegalArgumentException when email or password is empty, mapping them to specific missing email/password error codes. It also adds integration tests to verify this behavior. The review feedback suggests replacing Platform.isAndroid with defaultTargetPlatform != TargetPlatform.android in the tests to prevent potential runtime errors on Web and maintain consistency.
| }, | ||
| // Only the Android SDK rejects empty values itself; elsewhere the | ||
| // error comes from the backend and differs per method. | ||
| skip: kIsWeb || !Platform.isAndroid, |
There was a problem hiding this comment.
For consistency with other platform checks in this file (e.g., lines 1212 and 1275), and to avoid direct usage of Platform from dart:io which can throw runtime errors on Web if not guarded carefully, use defaultTargetPlatform != TargetPlatform.android instead of !Platform.isAndroid.
| skip: kIsWeb || !Platform.isAndroid, | |
| skip: kIsWeb || defaultTargetPlatform != TargetPlatform.android, |
| ), | ||
| ); | ||
| }, | ||
| skip: kIsWeb || !Platform.isAndroid, |
There was a problem hiding this comment.
For consistency with other platform checks in this file, use defaultTargetPlatform != TargetPlatform.android instead of !Platform.isAndroid.
| skip: kIsWeb || !Platform.isAndroid, | |
| skip: kIsWeb || defaultTargetPlatform != TargetPlatform.android, |
| ), | ||
| ); | ||
| }, | ||
| skip: kIsWeb || !Platform.isAndroid, |
There was a problem hiding this comment.
For consistency with other platform checks in this file, use defaultTargetPlatform != TargetPlatform.android instead of !Platform.isAndroid.
| skip: kIsWeb || !Platform.isAndroid, | |
| skip: kIsWeb || defaultTargetPlatform != TargetPlatform.android, |
|
Applied Gemini's suggestions in 4134a71: the three Android-only skips now use |
…id-empty-credentials # Conflicts: # packages/firebase_auth/firebase_auth/example/integration_test/firebase_auth_instance_e2e_test.dart
Description
On Android, calling
createUserWithEmailAndPassword,signInWithEmailAndPasswordorsendPasswordResetEmailwith an empty email or password fails with:Empty input is the caller's mistake, and this PR does not change that. What's wrong is how the mistake gets reported. Every other platform returns a
FirebaseAuthExceptionwith an auth error code. On Android the app gets an error that reads like a broken plugin installation, and the actual cause only shows up in logcat.Why it happens
Preconditions.checkNotEmptyand throwsIllegalArgumentException("Given String is empty or null")synchronously. It throws before it returns aTaskand before it sends any request. You can see this in thefirebase-auth25.0.0 bytecode forcreateUserWithEmailAndPassword,signInWithEmailAndPasswordand bothsendPasswordResetEmailoverloads.FlutterFirebaseAuthPluginonly handles failures insideaddOnCompleteListener, so this exception escapes the host method.FirebaseAuthHostApimethod is@async. For async methods, Pigeon doesn't wrap theapi.*call intry/catch(inGeneratedAndroidFirebaseAuth.g.kt, the onlycatch (exception: Throwable)is in the one synchronous handler).BasicMessageChannel.IncomingMessageHandler.onMessage. That method logsFailed to handle messageand callscallback.reply(null). Inmessages.pigeon.dart,_extractReplyValueOrThrowturns anullreply intoPlatformException(code: 'channel-error'), andconvertPlatformExceptionpasses that through asFirebaseAuthException(code: 'channel-error').#11931 shows the cost. The reporter tried
flutter clean,pub upgradeand downgradingfirebase_auth/firebase_corebefore a maintainer foundGiven String is empty or nullin the stack trace. The issue was closed as user error, which is fair for the input itself. The plugin still surfaced the platform's validation error as a channel failure.What this PR changes
The three host methods now catch the SDK's synchronous
IllegalArgumentExceptionand return it through the Pigeon callback, the same way the plugin reports every other SDK failure:channel-errormissing-email: "An email address must be provided."channel-errormissing-password: "A password must be provided."IllegalArgumentExceptionchannel-errorparserExceptionToFluttermappingThese codes and messages are the Android SDK's own
ERROR_MISSING_EMAILandERROR_MISSING_PASSWORDentries (An email address must be provided./A password must be provided.). The SDK simply never reaches them for empty input, because its precondition throws first. LikeERROR_INVALID_EMAIL, they reach Dart asmissing-emailandmissing-password.For comparison, the same input on other platforms:
FirebaseAuth/Sources/Swift/Auth/Auth.swift):createUser: empty email givesmissing-email, empty password givesweak-password.signIn: empty password giveswrong-password, and an empty email goes to the backend.MISSING_EMAILforsignUpand for the password-resetsendOobCodeINVALID_EMAILforsignInWithPasswordMISSING_PASSWORDforsignUpandsignInWithPasswordSo every other platform already returns a typed auth error here. Android was the outlier.
Scope
This PR is limited to the three email/password entry points where users actually run into this. The same synchronous
checkNotEmptyprecondition exists in other methods that the plugin calls the same way:FirebaseAuth:applyActionCode,checkActionCode,confirmPasswordReset,verifyPasswordResetCode,fetchSignInMethodsForEmail,sendSignInLinkToEmail,signInWithCustomToken,revokeAccessTokenFirebaseUser:updatePassword,updateEmail,unlinkI'm happy to extend the fix to them here or in a follow-up if you'd like.
Tests
I added Android-only cases to
example/integration_test/firebase_auth_instance_e2e_test.dartfor all three methods, covering both empty email and empty password. They're skipped on the other platforms, which return different codes for the same input (see above). The Android cases fail before any request is sent, so they don't depend on the emulator's responses.I ran
flutter test integration_test/e2e_test.darton an API 36 Android emulator against the Auth emulator (firebase-tools 15.34.1, Flutter 3.47.8, Firebase BoM 35.1.0):main: the three new cases fail with[firebase_auth/channel-error], and logcat showsBasicMessageChannel#…createUserWithEmailAndPassword Failed to handle message/java.lang.IllegalArgumentException: Given String is empty or null.createUserWithEmailAndPassword,sendPasswordResetEmail()andsignInWithEmailAndPassword()groups: 13 passed and 1 skipped (the existing Android skip), including the success paths.Related Issues
createUserWithEmailAndPasswordreportedchannel-error; the cause was empty input.checkNotEmptyprecondition fails there, but on a different path: it throws on the SDK's background thread duringlinkWithCredentialand crashes the app. This PR doesn't touch that path.Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?