Skip to content

Instantly share code, notes, and snippets.

@withoutboats
Last active June 28, 2018 06:19
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save withoutboats/75fc2f30092054c9f28417c12c38434c to your computer and use it in GitHub Desktop.
Save withoutboats/75fc2f30092054c9f28417c12c38434c to your computer and use it in GitHub Desktop.

We don't need a need a new error trait if we just fix std::error::Error

Our great anguish around error handling is the incorrectness of the Error trait. Here's an easy way to solve the problem: fix the error trait. Literally: just change the existing trait so that it is correct.

Step 1: Remove description (DONE)

Description is already being deprecated, with a default method provided. Done and done.

Step 2: Add backtrace (TRIVIAL)

backtrace has a default implementation and can be added whenever. It might cause ambiguity issues, but it is considered a non-breaking change.

Step 3: Fix cause (LESS TRIVIAL)

Here of course is where we get stopped up: cause is wrong in a way that makes it much less useful, because you can't downcast it. What do we do??

I see two options:

  1. The nuclear option: Release a breaking change. We all agree that cause is wrong, that its definition was a mistake. If a crater run shows no widespread devestation, I think it is wrong to punish our users with a transition cost to uphold some self-imposed law about breakage that isn't benefiting them in this particular case. Its morally equivalent to when we have previously broken unsound APIs in std.

  2. Deprecate and rename: Rather than deprecating the whole trait, we could just deprecate cause and replace it with a new method (source? underlying? origin?). Though a non-breaking solution, I think this is strictly worse than the prior option: cause is the correct name for this API, and this will cause much more breakage in practice because everyone who has defined cause will need to instead modify their code to define the new method.

Even the second option would be less breakage than introducing a new trait and deprecating the whole Error trait.

Step 4: Add Send + Sync + 'static (UNNECESSARY)

The last change would be to impose the triple S bound on Error. This is very troubling: it is definitely a huge breaking change to do this.

However, it is also unnecessary! In early iterations of failure, these bounds were imposed on the conversion from F: Fail + SS's to Error, not on Fail itself. This was Alex's preference, changed based on Aaron's preference, I (boats) had no strong opinion. We could just impose these bounds on the cause API or its replacement and then in both error-chain and failure's extensions.

FIN

Fixing std::error::Error is actually not nearly as out of the question as we have all assumed. This is the ideal solution to the problem of replacing the trait: just fix it.

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