Improve error management #396
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
This commit refactors how errors are handled and communicated to the user. First, it makes the necessary changes so that all unrecoverable errors eventually bubble up `src/index.ts`. It's easier to handle errors in one place. Second, the commit adds the `handleCommandError` and `ApplicationError` utilities. `handleCommandError` is what is used in `src/index.ts` to capture, interpret and display errors.
In case of errors, displaying a huge block of text is not useful. So, this commit moves the various `cfont.say` statements after all required API calls, so that fonts will be displayed only if the app is sure to have all the data it needs.
Previously, the spinner would not be displayed at all if there is no API key available, even if the application is in the data fetching phase. This commit moves the spinner creation at the top of `callApi` so that it can be set to display error if the API key is missing.
Co-authored-by: Alessandro Cifani <alessandro.cifani@gmail.com>
The application can work without caching, so there is no need to have an `ErrorCode.CACHE_WRITE` case.
Closed
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.
This PR is a proposal for improving error management and how such errors are displayed to the user. It addresses #17.
There are two major steps involved, plus some small UX changes for consistency.
1. Streamline how errors are surfaced and managed
Necessary changes are made so that all unrecoverable errors eventually bubble up
src/index.tsbecause it's easier to handle errors in one place. This means that all existingprocess.exitcalls in other files, like insrc/api.tsandsrc/commands/team.ts, are removed.In other words, all commands can throw errors and they are captured in
src/index.ts.All errors are handled with the newly added
handleCommandErrorfunction. This function will interpret the errors to pick an appropriate error message to display and an error code to use withprocess.exit. To make this easier,ApplicationErroris created (next step).2. Unified error class
ApplicationErrorApplicationErroris an extension ofErrorand is designed to be used for unrecoverable errors. It is used to convey the nature of the error and have extra contextual info (for certain types of errors).Small UX changes
Don't use cfonts until all data is available
Seeing a giant piece of text is noisy when in fact the application encounters an error, it is not useful. It might even initially give the illusion that "it's working". So, cfonts is now used only if all the API data has been fetched correctly.
Before
After
Display an error spinner even when no API key is available.
Previously, the spinner would not be displayed at all if there is no API key available, even if though the application is in the data fetching phase.
Things that need improving before merging
Most of the error messages of
formatErrorForPrintingprobably need refining, rephrasing, and spellchecking as for now they are sort of placeholders. Feedback is much appreciated.