Simplify and document CLI error handling #5700
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
rynowak
commented
Jun 12, 2023
rynowak
commented
Jun 12, 2023
rynowak
commented
Jun 12, 2023
rynowak
commented
Jun 12, 2023
docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md
Outdated
Show resolved
Hide resolved
Test Results2 736 tests +67 2 729 ✔️ +71 1m 59s ⏱️ -3s Results for commit a3870bf. ± Comparison against base commit 76114cf. This pull request removes 2 and adds 69 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
For the detailed report, please go to
|
willdavsmith
approved these changes
Jun 15, 2023
docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md
Outdated
Show resolved
Hide resolved
youngbupark
reviewed
Jun 15, 2023
docs/contributing/contributing-code/contributing-code-cli/README.md
Outdated
Show resolved
Hide resolved
docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md
Outdated
Show resolved
Hide resolved
docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md
Outdated
Show resolved
Hide resolved
ytimocin
reviewed
Jun 15, 2023
docs/contributing/contributing-code/contributing-code-cli/README.md
Outdated
Show resolved
Hide resolved
docs/contributing/contributing-code/contributing-code-cli/README.md
Outdated
Show resolved
Hide resolved
docs/contributing/contributing-code/contributing-code-tests/running-functional-tests.md
Outdated
Show resolved
Hide resolved
pkg/cli/connections/factory.go
Outdated
| @@ -61,7 +61,7 @@ func (i *impl) CreateDeploymentClient(ctx context.Context, workspace workspaces. | |||
|
|
|||
| err = sdk.TestConnection(ctx, connection) | |||
| if errors.Is(err, &sdk.ErrRadiusNotInstalled{}) { | |||
| return nil, &cli.FriendlyError{Message: err.Error()} | |||
| return nil, clierrors.MessageWithCause(err, "could not connect to Radius") | |||
There was a problem hiding this comment.
Suggested change
| return nil, clierrors.MessageWithCause(err, "could not connect to Radius") | |
| return nil, clierrors.MessageWithCause(err, "Could not connect to Radius.") |
This change turns the `FriendlyError` type into an interface and adds convenience functionality for creating user-facing errors. This allows us more flexibility for packages to define their own types of errors that are handled gracefully by the CLI - for example the console interrupt in the prompt package no longer needs special handling. Additionally I documented our existing practices for user-facing errors in the development documentation and did a consistency pass of ALL of the existing error messages I could find.
rynowak
force-pushed
the
rynowak/clierrors
branch
from
77895a7
to
a3870bf
Compare
Radius functional test overview
Click here to see the list of tools in the current test run
Test Status⌛ Building Radius and pushing container images for functional tests... |
|
For the detailed report, please go to
|
nithyatsu
pushed a commit
that referenced
this pull request
Jun 21, 2023
# Description This change turns the `FriendlyError` type into an interface and adds convenience functionality for creating user-facing errors. This allows us more flexibility for packages to define their own types of errors that are handled gracefully by the CLI - for example the console interrupt in the prompt package no longer needs special handling. Additionally I documented our existing practices for user-facing errors in the development documentation and did a consistency pass of ALL of the existing error messages I could find. ## Auto-generated summary <!-- GitHub Copilot for docs will auto-generate a summary of the PR --> <!-- copilot:all --> ### <samp>🤖 Generated by Copilot at 77895a7</samp> ### Summary 📝🐛🚀 <!-- 1. 📝 - This emoji represents the changes that add or improve documentation for the CLI, such as the new section for error handling guidelines, the instructions for testing and debugging, and the warning for functional tests. 2. 🐛 - This emoji represents the changes that fix or enhance the error handling and messaging in the CLI, such as the use of the clierrors package, the simplification of the error checking logic, and the removal of redundant code. 3. 🚀 - This emoji represents the changes that improve the user experience and performance of the CLI, such as the more consistent and informative error messages, the credential validation, and the aws package. --> This pull request introduces a new `clierrors` package for creating and handling user-friendly error messages in the `rad` CLI. It refactors the existing code in the `cmd` and `cli` packages to use the new package and simplifies the error handling logic. It also updates the documentation for contributing code and running tests for the CLI. > _`clierrors` package_ > _enhances CLI error messages_ > _friendly and clear_ ### Walkthrough * Introduce the `clierrors` package to define and create user-friendly error types and messages for the CLI ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-63404f70ef0aebce10d585c4f666381cb466470798b0eb5f7a1573318e542819R1-R31), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-c59fbc84834d0c8744895c1743d50ad3b52585c5b966c6e44fb6953c7ec7fd42R1-R35), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2f6b9e0c158058ace98b7beafd108285fd9648eff08b80d822afbf9ea24cb645R1-R56)) * Replace the use of the `cli.FriendlyError` type with the `clierrors.Message` or `clierrors.MessageWithCause` functions, which simplify the error creation and formatting logic ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-3abc8210264132b6c7c39842e1f0e9cfc67d8bcffc0dc82370261d4a83005a71L65-R66), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-eb1fb0acb333d81cd98eb18a726c7a9c218a7fc40b41fe09c698b3a9e1f36085L76-R96), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-bf4e917446c73565ee6967f28bdd8e619c0d4fa40b215f195b840c021dd36ce9L439-R440), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-298b3426311d1616260c81704588becbb63bafaeca245d9b8480b53b684a6708L102-R102), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-e81cb9ae5d8b783a0b674004a2df20c8faba1adf9585f4d028615c205e40e40cL123-R123), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2b3a46be3fab0d68fc258c962cf6ce1621b93ca7667fb4239a2ba6e0d1ec3308L124-R124), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL121-R123), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL134-R133), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL152-R151), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-65dd9b06f95ecad5a7ed9bd20d23ab719dc556133f5ca13c33f0ed99dd06cb09L45-R44), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-8515ea5f77ea63b91103fb2b1b17d23fdc76cd50ecbbe1552ce1274497ac4290L119-R127)) * Replace the use of the `errors.Is` function with the direct comparison of the error type or the `clierrors.IsFriendlyError` function, which improve the error checking logic ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-d36b7b4d04f01a6cf60c50dd9c01da8fb8c190231ea4c59cbbfbe070178b510bL146-R153), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-0d19f5b0c83b43f0f60b644a588f32e118296e1c8232e239d3dcc164c53d8f0bL291-R290), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-cea48b3105483f1a27bcc32c7c0ad99f4f5e68040aef470abe48e024d96ba646L168-R168), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-d3a3459434e925881f5cd87077b6f7db62ff30d190094cb46a71d632a71fe32dL228-R228)) * Remove unused imports of the `errors`, `fmt`, and `cli` packages from various files ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-d36b7b4d04f01a6cf60c50dd9c01da8fb8c190231ea4c59cbbfbe070178b510bL22), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-bf4e917446c73565ee6967f28bdd8e619c0d4fa40b215f195b840c021dd36ce9R25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-87907dd86f66c5e0992fef2ea8d8bc09cf79e4cf4e6d9108cb137a536da03c10L21), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-0d19f5b0c83b43f0f60b644a588f32e118296e1c8232e239d3dcc164c53d8f0bL25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-cea48b3105483f1a27bcc32c7c0ad99f4f5e68040aef470abe48e024d96ba646L24-R25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-d3a3459434e925881f5cd87077b6f7db62ff30d190094cb46a71d632a71fe32dL24-R26), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL27-R28), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-65dd9b06f95ecad5a7ed9bd20d23ab719dc556133f5ca13c33f0ed99dd06cb09L20-R24), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-8515ea5f77ea63b91103fb2b1b17d23fdc76cd50ecbbe1552ce1274497ac4290L21-R24)) * Add the `clierrors` package to various files that use the friendly error types or functions ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-3abc8210264132b6c7c39842e1f0e9cfc67d8bcffc0dc82370261d4a83005a71R28), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-eb1fb0acb333d81cd98eb18a726c7a9c218a7fc40b41fe09c698b3a9e1f36085R27), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-d36b7b4d04f01a6cf60c50dd9c01da8fb8c190231ea4c59cbbfbe070178b510bR31), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-bf4e917446c73565ee6967f28bdd8e619c0d4fa40b215f195b840c021dd36ce9R25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-298b3426311d1616260c81704588becbb63bafaeca245d9b8480b53b684a6708L21-R25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-e81cb9ae5d8b783a0b674004a2df20c8faba1adf9585f4d028615c205e40e40cL21-R24), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-cea48b3105483f1a27bcc32c7c0ad99f4f5e68040aef470abe48e024d96ba646L24-R25), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2b3a46be3fab0d68fc258c962cf6ce1621b93ca7667fb4239a2ba6e0d1ec3308L21-R24), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL27-R28), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-65dd9b06f95ecad5a7ed9bd20d23ab719dc556133f5ca13c33f0ed99dd06cb09L20-R24), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-8515ea5f77ea63b91103fb2b1b17d23fdc76cd50ecbbe1552ce1274497ac4290L21-R24)) * Remove unnecessary use of the `cli.FriendlyError` type for errors that already implement the `clierrors.FriendlyError` interface ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-87907dd86f66c5e0992fef2ea8d8bc09cf79e4cf4e6d9108cb137a536da03c10L128-L130)) * Improve some error messages by adding or removing information ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-65dd9b06f95ecad5a7ed9bd20d23ab719dc556133f5ca13c33f0ed99dd06cb09L45-R44), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL134-R133), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-52020886a399cc0113d50174c1beafe22429f71d4a740d10b8be819659eee8fcL152-R151)) * Add and update documentation for contributing code to the CLI, writing code for the CLI, and running functional tests ([link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2dc9993002cdaecc3a50b94159e9a38375dac771eb42557306649c0a4883002bR7-R10), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2dc9993002cdaecc3a50b94159e9a38375dac771eb42557306649c0a4883002bL69-R75), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-2dc9993002cdaecc3a50b94159e9a38375dac771eb42557306649c0a4883002bL78-R107), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-e286d51f1368685af6439c0bd568dc2cbee5c7c3806112b211a8844754c0b1c6R49-R50), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-e286d51f1368685af6439c0bd568dc2cbee5c7c3806112b211a8844754c0b1c6R86-R99), [link](https://github.com/project-radius/radius/pull/5700/files?diff=unified&w=0#diff-19c6617fdff9355dc160bf265e7efdc44ac178e0a5257bd6bd96a22dd56f1883R52-R89))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This change turns the
FriendlyErrortype into an interface and adds convenience functionality for creating user-facing errors. This allows us more flexibility for packages to define their own types of errors that are handled gracefully by the CLI - for example the console interrupt in the prompt package no longer needs special handling.Additionally I documented our existing practices for user-facing errors in the development documentation and did a consistency pass of ALL of the existing error messages I could find.
Auto-generated summary
🤖 Generated by Copilot at 77895a7
Summary
📝🐛🚀
This pull request introduces a new
clierrorspackage for creating and handling user-friendly error messages in theradCLI. It refactors the existing code in thecmdandclipackages to use the new package and simplifies the error handling logic. It also updates the documentation for contributing code and running tests for the CLI.Walkthrough
clierrorspackage to define and create user-friendly error types and messages for the CLI (link, link, link)cli.FriendlyErrortype with theclierrors.Messageorclierrors.MessageWithCausefunctions, which simplify the error creation and formatting logic (link, link, link, link, link, link, link, link, link, link, link)errors.Isfunction with the direct comparison of the error type or theclierrors.IsFriendlyErrorfunction, which improve the error checking logic (link, link, link, link)errors,fmt, andclipackages from various files (link, link, link, link, link, link, link, link, link)clierrorspackage to various files that use the friendly error types or functions (link, link, link, link, link, link, link, link, link, link, link)cli.FriendlyErrortype for errors that already implement theclierrors.FriendlyErrorinterface (link)