Skip to content

Add fancy errors#143

Open
FireIsGood wants to merge 2 commits intonelsonjchen:masterfrom
FireIsGood:fancy-errors
Open

Add fancy errors#143
FireIsGood wants to merge 2 commits intonelsonjchen:masterfrom
FireIsGood:fancy-errors

Conversation

@FireIsGood
Copy link
Copy Markdown

Fixes #142, maybe #124?

Adds colored errors, mostly to clean up how the offline error is displayed. These do not apply to Clap errors.

Since this adds color_eyre and errors are always returned, the new error parsing may be able to replace the entire error module. I have decided not to do that to make fewer changes for this pull request.

This adds color_eyre and thiserror.

Before:

image

After:

image

Adds colored errors via color_eyre as a unified way to show the errors.
This also adds the thiserror crate to easily implement the default
std::display trait.
@nelsonjchen
Copy link
Copy Markdown
Owner

Hmm, I never claimed to make this tool a library, and reluctantly made some of this available to be used as such. I wonder if this breaks anything for those users.

@FireIsGood
Copy link
Copy Markdown
Author

Technically yes.

This changes the return types of some of the public functions from Result<Response, SpeedTestError> to color_eyre::Result<Response>, which may break match statements.

The return types are mostly changed so you can get a better stack trace and suggestions in a reasonable place, however you can easily just revert the return types since the thiserror change makes your error::SpeedTestError variants have the Error trait. This means that they will automatically convert into the color_eyre type in the main function, but doesn't allow the .suggestion() for turning on the internet.

You could consider this a breaking change and then update the major version or change the return types back.

@nelsonjchen
Copy link
Copy Markdown
Owner

I'll need to think on this for a bit. I'll be honest, I wrote this a long time ago and I haven't kept up with the zeitgeist for error crate stuff. Even this project doesn't seem to build correctly at the moment in some PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve offline error messages

2 participants