Skip to content

fix: implement Global Error Boundary to prevent application crashes#246

Open
Priyasharma620064 wants to merge 1 commit into
volcano-sh:mainfrom
Priyasharma620064:fix/error-boundary
Open

fix: implement Global Error Boundary to prevent application crashes#246
Priyasharma620064 wants to merge 1 commit into
volcano-sh:mainfrom
Priyasharma620064:fix/error-boundary

Conversation

@Priyasharma620064
Copy link
Copy Markdown

Description

while exploring the codebase, I noticed that the dashboard didn't have a 'safety net' for runtime errors. Right now, if a component crashes (like from a bad API response or a data parsing issue), the whole screen just goes white.

I've added a Global Error Boundary to catch these unexpected crashes. Now, instead of a blank screen, users will see a professional 'Something went wrong' page with a quick reload button. This makes the dashboard much more resilient and reliable for production users.

Changes made:

  • Added a GlobalErrorBoundary component using Material UI.
  • Wrapped the main App component to protect the entire application tree.
  • Added a 'Reload' utility to help users recover quickly without a manual browser refresh.

Signed-off-by: Priya Sharma <priyasharma1001a@gmail.com>
@volcano-sh-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign monokaix for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a GlobalErrorBoundary component to provide a graceful fallback UI when uncaught errors occur in the frontend. The boundary is integrated into the main App component, wrapping the entire application structure. Review feedback suggests moving the GlobalErrorBoundary inside the ThemeProvider to ensure the fallback UI respects the application's custom theme and recommends externalizing hardcoded strings to support future internationalization.

Comment thread frontend/src/App.jsx
Comment on lines +25 to +43
<GlobalErrorBoundary>
<ThemeProvider theme={theme}>
<Router>
<Routes>
<Route path="/" element={<Layout />}>
<Route
index
element={<Navigate to="/dashboard" replace />}
/>
<Route path="dashboard" element={<Dashboard />} />
<Route path="jobs" element={<Jobs />} />
<Route path="queues" element={<Queues />} />
<Route path="pods" element={<Pods />} />
<Route path="podgroups" element={<PodGroups />} />
</Route>
</Routes>
</Router>
</ThemeProvider>
</GlobalErrorBoundary>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The GlobalErrorBoundary is currently wrapping the ThemeProvider. When an error occurs and the fallback UI is rendered, it will be outside the theme context, causing Material UI components to use their default styles instead of your custom theme. Moving the error boundary inside the ThemeProvider ensures that the error page maintains visual consistency with the rest of the application.

Suggested change
<GlobalErrorBoundary>
<ThemeProvider theme={theme}>
<Router>
<Routes>
<Route path="/" element={<Layout />}>
<Route
index
element={<Navigate to="/dashboard" replace />}
/>
<Route path="dashboard" element={<Dashboard />} />
<Route path="jobs" element={<Jobs />} />
<Route path="queues" element={<Queues />} />
<Route path="pods" element={<Pods />} />
<Route path="podgroups" element={<PodGroups />} />
</Route>
</Routes>
</Router>
</ThemeProvider>
</GlobalErrorBoundary>
<ThemeProvider theme={theme}>
<GlobalErrorBoundary>
<Router>
<Routes>
<Route path="/" element={<Layout />}>
<Route
index
element={<Navigate to="/dashboard" replace />}
/>
<Route path="dashboard" element={<Dashboard />} />
<Route path="jobs" element={<Jobs />} />
<Route path="queues" element={<Queues />} />
<Route path="pods" element={<Pods />} />
<Route path="podgroups" element={<PodGroups />} />
</Route>
</Routes>
</Router>
</GlobalErrorBoundary>
</ThemeProvider>

Comment on lines +51 to +63
Something went wrong
</Typography>
<Typography variant="body1" color="text.secondary" sx={{ mb: 4 }}>
The dashboard encountered an unexpected error. This might be due to a temporary network issue or invalid data from the cluster.
</Typography>

<Button
variant="contained"
startIcon={<RefreshIcon />}
onClick={this.handleReset}
sx={{ textTransform: "none", px: 4, py: 1 }}
>
Reload Dashboard
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The user-facing strings "Something went wrong", the error description, and "Reload Dashboard" are hardcoded. To ensure the application is ready for internationalization (i18n), these strings should be moved to a localization file or a constants file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants