-
Notifications
You must be signed in to change notification settings - Fork 16
Handle errors at Flink AI resource level #3079
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
base: main
Are you sure you want to change the base?
Conversation
This reverts commit c51fb21. revert: to TS commit
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 demonstrates two different error handling approaches for Flink AI resource loading: a TypeScript-style try/catch approach (commit 1) and a Go-style error handling approach with Promise.allSettled (commit 2). The implementation improves error resilience by allowing partial resource loading when some resource types fail, while providing clear error feedback to users.
Key changes:
- Replaced sequential await calls with concurrent Promise.allSettled for parallel resource loading
- Added graceful error handling that logs failures and shows user notifications while allowing successful resources to load
- Added comprehensive test coverage for error scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/viewProviders/multiViewDelegates/flinkAiDelegate.ts | Implemented Promise.allSettled-based error handling for Flink AI resource loading with user notifications |
| src/viewProviders/multiViewDelegates/flinkAiDelegate.test.ts | Added test coverage for error handling scenarios when resource loading fails |
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Summary of Changes
Fixes #3060
This PR handles errors at the Flink AI resources level, using
Promise.allSettledto track the results of the resource loader and handle them accordingly. There's also a small helper method added to converts a promise rejection reason to anErrorinstance. 2 unit tests are added as well.Clicktesting
or something like this, if the nature of
Promise.allSettledis not bubbling up what you want to verify (for me, it failed the other resources when I rejected one):Then observe that the error is thrown and you see an appropriate notification.
Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes