Skip to content
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

Error type updates #182

Closed
sagebind opened this issue May 3, 2020 · 0 comments · Fixed by #258
Closed

Error type updates #182

sagebind opened this issue May 3, 2020 · 0 comments · Fixed by #258
Labels
breaking Requires breaking backwards compatibility enhancement Make a feature better
Milestone

Comments

@sagebind
Copy link
Owner

sagebind commented May 3, 2020

We should update our Error type so that adding new kinds of errors is not a breaking change. One possibility is to leverage the new #[non_exhaustive] attribute so that users of the enum must always handle the case of unknown variants.

Another approach that I've seen taken lately is to use an error struct instead with an inner enum (or trait object), and provide methods on the struct to identify the kind of error. Not only is this backward compatible when adding new error types, it can also be backward compatible if we want even if we remove variants, as long as we change existing methods to return false.

Isahc is an interesting library case because normally, I'd push for very strict error enums defining all possible error cases, but with an HTTP client there's so many different kinds of errors that could occur, but generally only a few questions are actually interesting to consumers:

  • Is it transient?
  • Is it I/O related?
  • Is it a server compliance problem?
  • Is it a misuse of the client?

Think of the use-case of a GUI-based service using Isahc to consume an API. If fetching a resource fails, what do we tell the user? If we failed to reach the destination, we should tell the user that, as opposed to the server refusing the connection, since one is the server's fault and another is a network problem.

Whether isahc::Error stays an enum or changes to a struct, I think it is a good idea to add more general methods that help answer these more general questions about the error.

Also, std::error::Error::description is deprecated (for good reason) so we should stop using that when we make this change as well. (Right now our description() methods return useful error messages, so I'd consider it a "breaking change" for us to stop returning useful messages with that method.)

@sagebind sagebind added the enhancement Make a feature better label May 3, 2020
@sagebind sagebind added this to the 0.10 milestone May 3, 2020
@sagebind sagebind added the breaking Requires breaking backwards compatibility label Jun 1, 2020
@sagebind sagebind modified the milestones: 0.10, 1.0 Jul 29, 2020
@sagebind sagebind mentioned this issue Jul 29, 2020
8 tasks
sagebind added a commit that referenced this issue Nov 24, 2020
Update `isahc::Error` to support taking new variants in a backwards-compatible way, as well as hold more error sources when an error is caused by another.

Note that this is the first 1.0 breaking change to land on `master`. If 0.9 needs a patch fix in the future, we'll have to make a 0.9.x branch for it.

Fixes #182.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Requires breaking backwards compatibility enhancement Make a feature better
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant