Skip to content

RetryError doesn't report error sources when used in the most obvious way

RetryError expects to be provided with things that are Display. It then Displays as the inners' Displays.

But usually the innert things are errors. When they are errors, per EHWG policy, the Display impl is broken: it doesn't display the source.

Consequences

This can result in bad log output, eg:

2023-07-14T16:00:58Z  WARN tor_dirmgr::bootstrap: error while downloading: error: Problem downloading directory object: Error while getting a circuit: Tried to find or build a circuit 3 times, but all attempts failed
Attempt 1: Problem opening a channel to [scrubbed]
Attempt 2: Problem opening a channel to [scrubbed]
Attempt 3: Problem opening a channel to [scrubbed]

instead of

Attempt 1: Problem opening a channel to [scrubbed]: No plugin available for the transport obfs4

etc.

(In my current error handling test setup, this bug composes badly with #880 (closed) and the lack of !1229 (merged) )

Option A

Declare that it's RetryError's job to report sources.

This means RetryError will have to accept only an E implementing std::error::Error. That would be a breaking change.

It might mean moving some of the report machinery from tor-error, but really we only want the colon printing contraption which is simple enough that we could just copy it - and, arguably, RetryError has a different job to do since it must be able to print multiple errors.

Option B

Declare that it's E's job to report sources.

This means that putting an implementor of std::error::Error into a RetryError is a bug.

In practice, most call sites would need to be updated to use RetryError<tor_error::Report<E>> instead of RetryError<E>. (This is already done in tor-hsclient.)

Ideally we would find some way to detect these bugs. The only way I can think of is to invent a marker trait DisplayImplIsNotBroken and deprecate RetryError::in_attempt_to in favour of a new constructor.

Option C

Embark on a campaign to drain this swamp in Rust upstream. It would involve:

  1. Define an entirely new way for traits to be different between editions, with some kind of bidirectional trait gateway between Error2024 and Error2021.
  2. Use this to define Error2024 which doesn't have Display as a supertrait but instead has a fmt method on the Error2024 trait.
  3. Wait until mid-2025 and try to start using the new stuff.
  4. Discover all the "useless error output" bugs (not just the ones due to tor_error::Report, but everywhere an Error2021's Display is used without inspecting sources), and fix them.