🐛fix: prevent duplicate toast notifications#2403
🐛fix: prevent duplicate toast notifications#2403prakharsingh-74 wants to merge 4 commits intokubestellar:devfrom
Conversation
|
Welcome to KubeStellar! 🚀 Thank you for submitting this Pull Request. Before your PR can be merged, please ensure: ✅ DCO Sign-off - All commits must be signed off with ✅ PR Title - Must start with an emoji: ✨ (feature), 🐛 (bug fix), 📖 (docs), 🌱 (infra/tests), Getting Started with KubeStellar: Contributor Resources:
🌟 Help KubeStellar Grow - We Need Adopters! Our roadmap is driven entirely by adopter feedback. Whether you're using KubeStellar yourself or know someone who could benefit from multi-cluster Kubernetes: 📋 Take our Multi-Cluster Survey - Share your use cases and help shape our direction! A maintainer will review your PR soon. Feel free to ask questions in the comments or on Slack! |
|
Hi @prakharsingh-74. Thanks for your PR. I'm waiting for a kubestellar member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Signed-off-by: prakharsingh-74 <prakharsingh7014@gmail.com>
cc4eb18 to
af1e3da
Compare
|
@MAVRICK-1 can you please review this PR |
|
@oksaumya can you please review this |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent duplicate error toast notifications by adding stable id values to the global React Query QueryCache and MutationCache error handlers.
Changes:
- Add
idoptions totoast.error(...)calls inQueryCache.onErrorto dedupe repeated query error toasts. - Add
idoptions totoast.error(...)calls inMutationCache.onErrorto dedupe repeated mutation error toasts.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| toast.error('Server error. Please try again later.', { id: 'mutation-error-500' }); | ||
| } else { | ||
| toast.error('An error occurred while updating data'); | ||
| toast.error('An error occurred while updating data', { id: `mutation-error-${status || 'unknown'}` }); | ||
| } |
There was a problem hiding this comment.
MutationCache has the same network-error duplication risk as QueryCache: when axios.isAxiosError(error) but there is no error.response, this will show the generic "updating data" toast while the global network error toast is displayed via NetworkErrorToastManager. Consider an early return for !error.response (or otherwise aligning with the global network-error handling) to avoid double toasts on connectivity failures.
| toast.error('Authentication required. Please log in again.', { id: 'query-error-401' }); | ||
| } | ||
| } else if (status === 403) { | ||
| toast.error("Access denied. You don't have permission for this action."); | ||
| toast.error("Access denied. You don't have permission for this action.", { id: 'query-error-403' }); | ||
| } else if (status && status >= 500) { | ||
| toast.error('Server error. Please try again later.'); | ||
| toast.error('Server error. Please try again later.', { id: 'query-error-500' }); | ||
| } else { | ||
| toast.error('An error occurred while fetching data'); | ||
| toast.error('An error occurred while fetching data', { id: `query-error-${status || 'unknown'}` }); |
There was a problem hiding this comment.
These toast IDs are scoped to query-error-*, but the Axios response interceptor also shows error toasts with IDs like api-error-${status} (see frontend/src/lib/api.ts). If a request is made via the api client (common in hooks), both the interceptor and this QueryCache handler will still fire and produce duplicate toasts because the IDs don’t match. Consider either (a) suppressing QueryCache toasts for Axios errors handled by the interceptor, or (b) using the same ID scheme (and ideally the same message source) so the toast is deduped across layers.
| toast.error('Server error. Please try again later.', { id: 'query-error-500' }); | ||
| } else { | ||
| toast.error('An error occurred while fetching data'); | ||
| toast.error('An error occurred while fetching data', { id: `query-error-${status || 'unknown'}` }); | ||
| } |
There was a problem hiding this comment.
For Axios errors with no response (e.g., network outage), status is undefined and this falls into the generic branch, showing a toast with id query-error-unknown. At the same time, the Axios interceptor sets setGlobalNetworkError(true) and NetworkErrorToastManager shows its own network-error toast, leading to duplicates. Consider explicitly detecting axios.isAxiosError(error) && !error.response here and returning early (or routing to the same global network-error UX) instead of showing an additional generic toast.
| toast.error('Authentication required. Please log in again.', { id: 'mutation-error-401' }); | ||
| } | ||
| } else if (status === 403) { | ||
| toast.error("Access denied. You don't have permission for this action."); | ||
| toast.error("Access denied. You don't have permission for this action.", { id: 'mutation-error-403' }); | ||
| } else if (status && status >= 500) { | ||
| toast.error('Server error. Please try again later.'); | ||
| toast.error('Server error. Please try again later.', { id: 'mutation-error-500' }); | ||
| } else { | ||
| toast.error('An error occurred while updating data'); | ||
| toast.error('An error occurred while updating data', { id: `mutation-error-${status || 'unknown'}` }); | ||
| } |
There was a problem hiding this comment.
Same dedupe issue as QueryCache: these IDs are mutation-error-*, but requests using the shared Axios api client will also trigger the interceptor toast (api-error-${status}), so duplicates can still occur across layers because the IDs differ. Consider reusing the interceptor’s ID scheme (or suppressing one of the two toasting layers) so a single toast is shown per error.
Description
Duplicate toast notifications were appearing when users encountered errors, particularly "Access denied" messages. The same error would trigger 2-3 identical toasts simultaneously, degrading the user experience.
Fixes #2185
Changes Made
Root Cause
Multiple layers of error handling were independently triggering toast notifications:
When an error occurred (e.g., 403 Forbidden), all three handlers would fire, creating duplicate toasts for the same error.
Checklist
Please ensure the following before submitting your PR:
Screenshots or Logs (if applicable)
Before

After
