-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: implement react-error-boundary for component error isolation #5732
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,82 @@ | ||||||
| import React from "react" | ||||||
| import { ErrorBoundary as ReactErrorBoundary, FallbackProps } from "react-error-boundary" | ||||||
| import { VSCodeButton } from "@vscode/webview-ui-toolkit/react" | ||||||
| import { useTranslation } from "react-i18next" | ||||||
| import { errorReporter } from "../../utils/errorReporting" | ||||||
|
|
||||||
| interface ErrorFallbackProps extends FallbackProps { | ||||||
| componentName?: string | ||||||
| } | ||||||
|
|
||||||
| function ErrorFallback({ error, resetErrorBoundary, componentName }: ErrorFallbackProps) { | ||||||
| const { t } = useTranslation("common") | ||||||
|
|
||||||
| return ( | ||||||
| <div className="flex flex-col items-center justify-center p-6 bg-vscode-editor-background border border-vscode-widget-border rounded-md m-4"> | ||||||
| <div className="flex items-center mb-4"> | ||||||
| <span className="codicon codicon-error text-vscode-errorForeground text-2xl mr-3" /> | ||||||
| <h2 className="text-lg font-semibold text-vscode-editor-foreground"> | ||||||
| {t("errorBoundary.title", "Something went wrong")} | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid using inline fallback strings in translation calls. Remove the second argument in t() (e.g. in t('errorBoundary.title', 'Something went wrong')) and rely solely on translations defined in the JSON files per our guidelines.
Suggested change
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX. |
||||||
| </h2> | ||||||
| </div> | ||||||
|
|
||||||
| {componentName && ( | ||||||
| <p className="text-sm text-vscode-descriptionForeground mb-2"> | ||||||
| {t("errorBoundary.componentError", "Error in {{componentName}} component", { componentName })} | ||||||
| </p> | ||||||
| )} | ||||||
|
|
||||||
| <p className="text-sm text-vscode-descriptionForeground mb-4 text-center max-w-md"> | ||||||
| {t( | ||||||
| "errorBoundary.description", | ||||||
| "An error occurred in this part of the interface. You can try to recover by clicking the button below.", | ||||||
| )} | ||||||
| </p> | ||||||
|
|
||||||
| <details className="mb-4 w-full max-w-md"> | ||||||
| <summary className="cursor-pointer text-sm text-vscode-descriptionForeground hover:text-vscode-editor-foreground"> | ||||||
| {t("errorBoundary.showDetails", "Show error details")} | ||||||
| </summary> | ||||||
| <pre className="mt-2 p-3 bg-vscode-textCodeBlock-background border border-vscode-widget-border rounded text-xs text-vscode-editor-foreground overflow-auto max-h-32"> | ||||||
| {error.message} | ||||||
| {error.stack && ( | ||||||
| <> | ||||||
| {"\n\n"} | ||||||
| {error.stack} | ||||||
| </> | ||||||
| )} | ||||||
| </pre> | ||||||
| </details> | ||||||
|
|
||||||
| <VSCodeButton appearance="primary" onClick={resetErrorBoundary}> | ||||||
| {t("errorBoundary.retry", "Try again")} | ||||||
| </VSCodeButton> | ||||||
| </div> | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| interface ErrorBoundaryProps { | ||||||
| children: React.ReactNode | ||||||
| componentName?: string | ||||||
| onError?: (error: Error, errorInfo: React.ErrorInfo) => void | ||||||
| } | ||||||
|
|
||||||
| export function ErrorBoundary({ children, componentName, onError }: ErrorBoundaryProps) { | ||||||
| const handleError = (error: Error, errorInfo: React.ErrorInfo) => { | ||||||
| // Report error using our error reporting utility | ||||||
| errorReporter.reportError(error, errorInfo, componentName) | ||||||
|
|
||||||
| // Call custom error handler if provided (for potential Sentry integration) | ||||||
| onError?.(error, errorInfo) | ||||||
| } | ||||||
|
|
||||||
| return ( | ||||||
| <ReactErrorBoundary | ||||||
| FallbackComponent={(props) => <ErrorFallback {...props} componentName={componentName} />} | ||||||
| onError={handleError}> | ||||||
| {children} | ||||||
| </ReactErrorBoundary> | ||||||
| ) | ||||||
| } | ||||||
|
|
||||||
| export default ErrorBoundary | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A cleaner pattern would be to just use the HoC so we don't need to litter the existing markup with a bunch of wrappers, the wrapping happens on component's file export
Instead of
export default Componentwe'd doexport default withErrorBoundary(Component)instead. That could be customised so we don't have repeated logic, all of the magic happens insidewithErroBoundarywrapper.