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

Added error handling to schedule tab #1014

Open
wants to merge 2 commits into
base: 1.x
Choose a base branch
from

Conversation

Corepex
Copy link
Contributor

@Corepex Corepex commented Feb 18, 2025

Changes in this pull request

Resolves #971

Additional info

This pull request includes changes to improve error handling in the VersionIdCell and ScheduleTabContainer components by adding error tracking and handling mechanisms. The most important changes include adding imports for the error handling utilities and modifying the components to track and handle errors appropriately.

Error handling improvements:

@Corepex Corepex requested a review from markus-moser February 18, 2025 14:47
@Corepex Corepex self-assigned this Feb 18, 2025
Comment on lines -77 to +87
if (isLoading || data === undefined) {
return <Content loading />
}

if (isError) {
trackError(new ApiError(error))
return <div>Error</div>
}

if (isLoading || data === undefined) {
return <Content loading />
}

Copy link
Contributor Author

@Corepex Corepex Feb 18, 2025

Choose a reason for hiding this comment

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

@markus-moser, as you see here, I moved the isError condition above the isLoading condition since rtk doesn't reset the isLoading when an error is thrown.

This leads to the behavior that now "error" appears in the tab instead of an infinite loading spinner ... Should we define an "error view" similar to the empty content or loading view?

WDYT

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I would go for an error view where we show the error message instead - maybe a simple (and then remove the default handling with the alert).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule on an asset does not work
2 participants