Skip to content

fix(admin-ui): remove unwanted toast when the FIDO service is down #2885

Merged
duttarnab merged 1 commit into
mainfrom
admin-ui-issue-2884
Jun 22, 2026
Merged

fix(admin-ui): remove unwanted toast when the FIDO service is down #2885
duttarnab merged 1 commit into
mainfrom
admin-ui-issue-2884

Conversation

@faisalsiddique4400

@faisalsiddique4400 faisalsiddique4400 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

fix(admin-ui): remove unwanted toast when the FIDO service is down (#2884)

Summary

This PR removes an unnecessary error toast that appears in the Admin UI when the FIDO service is unavailable.

The FIDO health endpoint may legitimately return 503 Service Unavailable when the FIDO service is down. Displaying an error toast for this expected service-health condition creates confusion for administrators and provides little actionable value.

The update suppresses the toast for this specific scenario while preserving normal error handling behavior for actual user-facing failures.


Fix Summary

  • Updated FIDO health-check error handling
  • Suppressed unnecessary error toast notifications for expected 503 Service Unavailable responses from the FIDO health endpoint
  • Prevented confusing alerts when the FIDO service is intentionally stopped or unavailable
  • Preserved existing health-check behavior and status detection
  • Maintained standard error handling for genuine user-facing failures
  • Improved overall user experience by reducing noisy notifications

Verification

npm run check:all

passes successfully.

  • Verified no error toast is displayed when the FIDO health endpoint returns 503 Service Unavailable
  • Verified FIDO service status continues to be detected correctly
  • Verified other API failures continue to surface appropriate error notifications
  • Verified no regressions in FIDO-related Admin UI functionality

🔗 Ticket

Closes: #2884

Summary by CodeRabbit

  • Refactor
    • Simplified FIDO2 health status checking implementation for improved efficiency and maintainability.

…2884)

Signed-off-by: faisalsiddique4400 <faisalsiddique10886@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The useFido2HealthStatus hook is simplified by removing the useEffect side effect that dispatched an error toast when the query failed. Unused imports (useEffect, useTranslation, useAppDispatch, updateToast, getQueryErrorMessage) are removed, and the hook now returns useQuery(...) directly instead of assigning it to an intermediate variable.

useFido2HealthStatus hook cleanup

Layer / File(s) Summary
Remove toast side effect and simplify hook return
admin-ui/plugins/admin/components/Health/hooks/useFido2HealthStatus.ts
Removes the useEffect that dispatched an error toast on query.isError, drops the intermediate query variable, returns useQuery(...) inline, and trims all associated imports.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • GluuFederation/flex#2790: Applies the same pattern of removing useEffect-based updateToast dispatch on query failure, in useDashboardLockStats.

Suggested labels

comp-admin-ui, kind-bug

Suggested reviewers

  • duttarnab
  • moabu

Poem

🐇 No more toast when FIDO sleeps,
The hook now clean, no side-effect leaps.
useEffect gone, the imports trimmed,
Return inline — the rabbit grinned.
Silence is golden when services dim! 🍞✂️

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The PR description indicates the fix suppresses toast for 503 responses, but the code summary shows all error-handling utilities were removed, making it unclear if the 503-specific logic was properly implemented. Verify that the error-handling changes specifically suppress the 503 response toast while preserving other error notifications as described in issue #2884.
Out of Scope Changes check ❓ Inconclusive The code change removes multiple error-handling imports and the direct useQuery result pattern, but it is unclear from the summary whether these changes fully align with the stated objective of selectively suppressing 503 errors. Confirm that removing all error-handling utilities does not inadvertently disable error handling for non-503 failures or create other unintended side effects.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: removing an unwanted toast notification when the FIDO service is unavailable, which is the primary objective of this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch admin-ui-issue-2884

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto mo-auto added comp-admin-ui Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality labels Jun 22, 2026
@sonarqubecloud

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
admin-ui/plugins/admin/components/Health/hooks/useFido2HealthStatus.ts (1)

44-52: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Non-503 failures no longer trigger user-facing error notification.

By removing the error side effect entirely, this hook now suppresses toasts for all FIDO health query failures, not only 503 Service Unavailable. That breaks the stated behavior contract to keep notifications for actionable failures.

Suggested fix (preserve suppression only for 503)
+import { useEffect } from 'react'
+import { useTranslation } from 'react-i18next'
+import { useAppDispatch, useAppSelector } from '`@/redux/hooks`'
+import { updateToast } from '`@/redux/features/toastSlice`'
+import { getQueryErrorMessage } from '`@/utils/getQueryErrorMessage`'
@@
 export const useFido2HealthStatus = (options?: { enabled?: boolean }) => {
+  const { t } = useTranslation()
+  const dispatch = useAppDispatch()
   const hasSession = useAppSelector((state) => state.authReducer?.hasSession)
   const isEnabled = (options?.enabled ?? true) && hasSession === true
 
-  return useQuery({
+  const query = useQuery({
     queryKey: FIDO2_HEALTH_QUERY_KEY,
     queryFn: ({ signal }) => fetchFido2Health(signal),
     enabled: isEnabled,
     staleTime: HEALTH_CACHE_CONFIG.STALE_TIME,
     gcTime: HEALTH_CACHE_CONFIG.GC_TIME,
     retry: false,
     select: transformFido2Health,
   })
+
+  useEffect(() => {
+    if (!query.isError) return
+    const status = (query.error as { response?: { status?: number } } | null)?.response?.status
+    if (status === 503) return
+    dispatch(
+      updateToast({
+        show: true,
+        title: t('health.error.fetchingfido2status'),
+        body: getQueryErrorMessage(query.error, t),
+        type: 'error',
+      }),
+    )
+  }, [query.isError, query.error, dispatch, t])
+
+  return query
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@admin-ui/plugins/admin/components/Health/hooks/useFido2HealthStatus.ts`
around lines 44 - 52, The useFido2HealthStatus hook's useQuery configuration is
missing error handling that previously distinguished between 503 Service
Unavailable errors (which should be silently suppressed) and other actionable
failures (which should show user notifications). Add an onError callback to the
useQuery hook in useFido2HealthStatus that checks if the error is a 503 status
code. If it is a 503, suppress the notification; otherwise, trigger an error
toast notification to alert the user of the failure. This ensures the hook
maintains the contract of suppressing only service unavailable errors while
surfacing other meaningful failures to the user.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@admin-ui/plugins/admin/components/Health/hooks/useFido2HealthStatus.ts`:
- Around line 44-52: The useFido2HealthStatus hook's useQuery configuration is
missing error handling that previously distinguished between 503 Service
Unavailable errors (which should be silently suppressed) and other actionable
failures (which should show user notifications). Add an onError callback to the
useQuery hook in useFido2HealthStatus that checks if the error is a 503 status
code. If it is a 503, suppress the notification; otherwise, trigger an error
toast notification to alert the user of the failure. This ensures the hook
maintains the contract of suppressing only service unavailable errors while
surfacing other meaningful failures to the user.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 77830aa0-69a0-415d-ac04-9b8af8982336

📥 Commits

Reviewing files that changed from the base of the PR and between 36df92f and c3537d5.

📒 Files selected for processing (1)
  • admin-ui/plugins/admin/components/Health/hooks/useFido2HealthStatus.ts

@duttarnab duttarnab merged commit bc82d77 into main Jun 22, 2026
10 checks passed
@duttarnab duttarnab deleted the admin-ui-issue-2884 branch June 22, 2026 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-admin-ui Component affected by issue or PR kind-bug Issue or PR is a bug in existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(admin-ui): Remove the unwanted toast coming when the FIDO service is down

3 participants