-
Notifications
You must be signed in to change notification settings - Fork 11
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
fix(PDiskPage): fix error boundary on failed restart #2069
Conversation
64b758b
to
505bcdc
Compare
// TODO: extend with other error types | ||
type ResponseErrorData = TIssueMessage; | ||
|
||
export interface IResponseError<T = ResponseErrorData> { |
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.
data
is unknown
, but this type prevented ts errors in our code, where we used this error.
Made data
unknown
, added proper type guards and some tests for them
505bcdc
to
df83060
Compare
df83060
to
8ea5ba4
Compare
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.
Pull Request Overview
This PR fixes the error boundary handling on failed restarts by refactoring how response errors are detected and parsed. Key changes include:
- Adding new utility functions (isResponseError, isResponseErrorWithIssues, isIssue, isIssuesArray) to standardize error checking.
- Updating error message handling in errors and UI components by leveraging the new response error detection.
- Adjusting type definitions for response errors.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/utils/response.ts | Introduces new functions for error identification and parsing related to API errors |
src/utils/errors/index.ts | Updates error message extraction using the new error checking utilities |
src/utils/test/response.test.ts | Adds tests to validate the new error detection and parsing functions |
src/types/api/error.ts | Changes the default generic type in IResponseError |
src/containers/Tenant/Query/utils/isQueryCancelledError.ts | Removes the local duplicate isResponseError function |
src/components/CriticalActionDialog/CriticalActionDialog.tsx | Updates error parsing logic for critical action dialogs using the new utilities |
Comments suppressed due to low confidence (2)
src/utils/response.ts:74
- [nitpick] Consider using an explicit comparison, e.g. 'arr.length > 0', for clarity instead of relying on the truthy value of 'arr.length'.
return Boolean(Array.isArray(arr) && arr.length && arr.every(isIssue));
src/types/api/error.ts:1
- The default generic type for IResponseError has been changed from a specific type to 'unknown'; please confirm this is intentional and update any related documentation if needed.
export interface IResponseError<T = unknown> {
Closes #2068
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: ✅
Current: 83.25 MB | Main: 83.25 MB
Diff: +3.40 KB (0.00%)
✅ Bundle size unchanged.
ℹ️ CI Information