Incorrect error message on login screen #625
Conversation
Signed-off-by: Rakshithasai123 <[email protected]>
WalkthroughError handling for authentication server failures was updated: a specific error condition now maps to a dedicated error code, triggering a localized "server unreachable" message. Localization entries for this message were added across six languages, and UI error display logic now intercepts and shows this message for specific server-related error codes. Changes
Sequence DiagramsequenceDiagram
participant User
participant LoginPage
participant AuthApi
User->>LoginPage: Attempts authentication
LoginPage->>AuthApi: Calls authentication
AuthApi->>AuthApi: Invalid request detected
AuthApi-->>LoginPage: Returns error with<br/>REG_AUTH_SERVER_DOWN code
LoginPage->>LoginPage: _showErrorInSnackbar checks<br/>error code/message
Note over LoginPage: Matches REG_AUTH_SERVER_DOWN<br/>or KER-SYN-AUTH-001<br/>or Invalid Request?
LoginPage->>LoginPage: Fetch server_unreachable<br/>localized message
LoginPage-->>User: Display snackbar:<br/>"Authentication server<br/>unreachable. Please try<br/>again later."
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
android/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.java(1 hunks)assets/l10n/app_ar.arb(1 hunks)assets/l10n/app_en.arb(1 hunks)assets/l10n/app_fr.arb(1 hunks)assets/l10n/app_hi.arb(1 hunks)assets/l10n/app_kn.arb(1 hunks)assets/l10n/app_ta.arb(1 hunks)lib/ui/login_page.dart(1 hunks)
🔇 Additional comments (7)
assets/l10n/app_kn.arb (1)
335-336: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with Kannada translation and the trailing comma adjustment for valid JSON syntax are correct.assets/l10n/app_ar.arb (1)
335-336: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with Arabic translation and the trailing comma adjustment are correct.assets/l10n/app_en.arb (1)
335-336: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with English translation and the trailing comma adjustment are correct.assets/l10n/app_fr.arb (1)
335-336: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with French translation and the trailing comma adjustment are correct.assets/l10n/app_ta.arb (1)
344-345: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with Tamil translation and the trailing comma adjustment are correct.assets/l10n/app_hi.arb (1)
335-336: LGTM: Localization entry added correctly.The addition of the
server_unreachablekey with Hindi translation and the trailing comma adjustment are correct.android/app/src/main/java/io/mosip/registration_client/api_services/AuthenticationApi.java (1)
133-139: Verify the authentication server's actual error messages when it's unreachable.The concern raised is valid. The mapping of "Invalid Request" to
REG_AUTH_SERVER_DOWNappears semantically inconsistent:
- "Invalid Request" typically indicates a malformed or client-side validation error (consistent with its use in @notblank validation annotations throughout the codebase)
REG_AUTH_SERVER_DOWNsemantically suggests server unavailability- A separate error code
REG_INVALID_REQUESTexists elsewhere (line 165), indicating two distinct error types are expectedFrom the codebase, I cannot confirm whether the authentication server actually returns "Invalid Request" when unreachable or if this represents a different error condition. You should verify the server's actual error responses to ensure the mapping is correct, or consider using the existing
REG_INVALID_REQUESTcode if this represents invalid request format rather than server unavailability.
| if (errorMsg == "REG_AUTH_SERVER_DOWN" || errorMsg == "KER-SYN-AUTH-001" || errorMsg == "Invalid Request") { | ||
| _showInSnackBar(appLocalizations.server_unreachable); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Potential redundancy and fragile string comparison in error handling.
This code checks for three conditions: REG_AUTH_SERVER_DOWN, KER-SYN-AUTH-001, and "Invalid Request". Based on the changes in AuthenticationApi.java, the server message "Invalid Request" is now mapped to REG_AUTH_SERVER_DOWN before reaching the UI. This raises two concerns:
-
Redundancy: Checking for both the error code (
REG_AUTH_SERVER_DOWN) and the raw message ("Invalid Request") suggests either defensive programming or confusion about the error flow. If they're both needed, please clarify the different scenarios where each condition is true. -
Fragile string comparison: Directly comparing
errorMsg == "Invalid Request"is fragile—if the server message changes or gets localized, this will silently fail. Error codes should be used consistently for error handling.
Recommendation: Consider adding REG_AUTH_SERVER_DOWN and KER-SYN-AUTH-001 to the errors mapping in the ARB files (line 308) for consistency, rather than handling them as special cases here. This would align with how other error codes are handled.
// Instead of special-case handling, consider:
// 1. Add to errors mapping in ARB files:
// "errors": "{messages, select, ..., REG_AUTH_SERVER_DOWN{Authentication server unreachable. Please try again later}, KER-SYN-AUTH-001{Authentication server unreachable. Please try again later}, ...}"
//
// 2. Remove the special case and let the existing logic handle it:
// String snackbarText = appLocalizations.errors(errorMsg);
// if (snackbarText == "Some error occurred!") {
// snackbarText = errorMsg;
// }
// _showInSnackBar(snackbarText);🤖 Prompt for AI Agents
In lib/ui/login_page.dart around lines 357 to 360, the code special-cases
errorMsg values ("REG_AUTH_SERVER_DOWN", "KER-SYN-AUTH-001", and the raw
"Invalid Request") which is redundant and fragile; update the ARB error mapping
(around line 308) to include REG_AUTH_SERVER_DOWN and KER-SYN-AUTH-001 with the
desired user-facing text, remove the special-case if-block here, and let
appLocalizations.errors(errorMsg) drive the snackbar text; if you must keep both
checks, add a brief comment explaining the distinct server/client scenarios and
replace the raw-string comparison with a check against a centralized constant or
enum to avoid fragile literal matching.
1 participant
RCF-373
Summary by CodeRabbit
New Features
Bug Fixes