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.ts
because it's easier to handle errors in one place. This means that all existingprocess.exit
calls in other files, like insrc/api.ts
andsrc/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
handleCommandError
function. 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,ApplicationError
is created (next step).2. Unified error class
ApplicationError
ApplicationError
is an extension ofError
and 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
formatErrorForPrinting
probably need refining, rephrasing, and spellchecking as for now they are sort of placeholders. Feedback is much appreciated.