Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

frontend: Improve error handling in list pages #2771

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sniok
Copy link
Contributor

@sniok sniok commented Jan 22, 2025

This PR combines and replaces different fields we have for error handling, currently we have 'error' 'errorMessage' 'clusterErrors' props in various places.

error is not enough since we can have more than one error
errorMessage is a string and loses important context like which cluster the error is coming from and limits how we can render it
clusterErrors prop has a limitation of one error per cluster but we can have more (see Role page example)

New field is
errors an array of ApiError

ApiError is also updated to be a class instead of an interface and now contains all relevant information (if available) like cluster name, namespace of a resource and status code.

ApiError message is now also parsed from backend response (if available)

To display the error the ClusterGroupErrorMessage component was updated since it is already can display error.
I would also change the name to something more generic like ErrorAlerts or something but it's not a priority.


Testing done / Examples

All error messages by default are collapsed, in screenshots I'll expand them

When resource is not available, endpoint returns 404

image

image

When user doesn't have access 403

image

image

When user doesn't have access and List page loads multiple resource
Role page loads Roles and ClusterRoles

image

image

When two clusters are selected and one of them doesn't have access

image

When two clusters are selected and one of them doesn't have access for a List page with multiple resources

image

Cluster overview page with no access
image

@sniok sniok force-pushed the fix-error-handling branch 4 times, most recently from 9c562cd to a506dd5 Compare January 22, 2025 14:08
@sniok sniok added frontend Issues related to the frontend bug Something isn't working enhancement New feature or request labels Jan 22, 2025
@sniok sniok requested a review from a team January 22, 2025 15:39
@sniok sniok marked this pull request as ready for review January 22, 2025 15:40
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jan 22, 2025
Signed-off-by: Oleksandr Dubenko <[email protected]>
@sniok sniok force-pushed the fix-error-handling branch from a506dd5 to b21b802 Compare January 24, 2025 13:32
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jan 24, 2025
@sniok
Copy link
Contributor Author

sniok commented Jan 24, 2025

Updated the error message to use Alert component from mui, and made 403 error display as blue since it's probably expected that user with limited access won't be able to see some things

@sniok sniok force-pushed the fix-error-handling branch from b21b802 to c4eadaf Compare January 24, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request frontend Issues related to the frontend size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
Development

Successfully merging this pull request may close these issues.

1 participant