Fix: Multiple Error Message Popups on Login Failure (HWC UI) #125#265
Fix: Multiple Error Message Popups on Login Failure (HWC UI) #125#265megha1807 wants to merge 1 commit intoPSMRI:mainfrom
Conversation
📝 WalkthroughWalkthroughUpdated error handling in the login component to extract and display user-friendly error messages from the authentication service response, with fallback chains for missing error details, rather than alerting raw error objects. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (1)
src/app/user-login/login/login.component.ts (1)
276-283: Good fix for displaying meaningful error messages.The fallback chain correctly extracts a user-friendly message instead of alerting the raw error object. The approach handles the common cases:
- API error response body (
err.error.errorMessage)- Standard Error message (
err.message)- Static fallback for unexpected error shapes
For consistency, consider aligning with the existing
getErrorMessage()pattern inhttp-interceptor.service.ts(lines 152-184), which also handles edge cases like string errors anderror.erroras a string. You could either:
- Extract a shared utility function, or
- Add similar defensive checks here:
♻️ Optional: More defensive error extraction
(err) => { this.resetCaptcha(); - const errorMessage = - err?.error?.errorMessage || - err?.message || - 'Login failed. Please check your credentials and try again.'; + let errorMessage = 'Login failed. Please check your credentials and try again.'; + if (typeof err === 'string' && err.trim()) { + errorMessage = err; + } else if (err && typeof err === 'object') { + errorMessage = + err.error?.errorMessage || + err.message || + (typeof err.error === 'string' ? err.error : errorMessage); + } this.confirmationService.alert(errorMessage, 'error'); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/user-login/login/login.component.ts` around lines 276 - 283, The current error extraction in the login handler uses err?.error?.errorMessage || err?.message || fallback; update it to the same defensive logic as getErrorMessage() from http-interceptor.service.ts by either calling that shared utility or replicating its checks: handle cases where err.error is a string, where err itself is a string, and where nested error shapes exist before falling back to 'Login failed...'; apply this change inside the login component's error callback (the handler that calls this.resetCaptcha() and this.confirmationService.alert()) so confirmationService.alert receives a normalized string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/app/user-login/login/login.component.ts`:
- Around line 276-283: The current error extraction in the login handler uses
err?.error?.errorMessage || err?.message || fallback; update it to the same
defensive logic as getErrorMessage() from http-interceptor.service.ts by either
calling that shared utility or replicating its checks: handle cases where
err.error is a string, where err itself is a string, and where nested error
shapes exist before falling back to 'Login failed...'; apply this change inside
the login component's error callback (the handler that calls this.resetCaptcha()
and this.confirmationService.alert()) so confirmationService.alert receives a
normalized string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: baed2c07-6a8f-4cc5-b71a-049a64c69925
📒 Files selected for processing (1)
src/app/user-login/login/login.component.ts
|
Hello @snehar-nd, Changes made:
Please review when you get a chance. Thank you! |
1 participant
Problem
Fixes #125
When login fails, two error popups were displayed:
elseblock on session conflict cancel (with incorrect message)errcallback with a raw error object (meaningless message)Changes Made
confirmationService.alert()from the session conflict cancel blockerrobject in error callback with a meaningful fallback error messageExpected Behaviour
Summary by CodeRabbit