use `anyhow` instead of `std::io::Error`

[rkuhn]

trafficstars

This may be controversial, and it is completely fine to explain to me why this idea is nuts or that there is a better solution. The issue I faced was that I need to recognise a specific upgrade failure while connecting with noise to redial in case of an uncoordinated TCP simultaneous open event. The only solution I could find is to turn the IO Error into a string and pattern match on it, which is only possible because I control the stack of upgrades applied by ipfs-embed.

anyhow provides more utility than std::io::Error: while the latter can only be traversed along the cause chain and turned into strings, the former allows downcasting to find a specific error type in the chain.

---

As a side remark, ipfs-embed now carries quite some code to make error conditions visible and discernible to the end user. I needed this because we use ipfs-embed in factories, meaning very demanding and actively operated environments. If something is not working, someone will want to see the root cause or at least a smoking gun within minutes and fix it, because operational losses can easily amount to $100k/h.

[nazar-pc]

anyhow is generally considered to be used in applications, not libraries. thiserror would be a more appropriate option for a library.

[tomaka]

Unfortunately our hands are kind of tied because we abstract over streams through the AsyncRead and AsyncWrite traits, which are hardcoded to std::io::Error

It is probably possible to improve the current situation, but it's not as trivial as replacing an error type with another.

[rkuhn]

@tomaka Hmm, that’s unfortunate. Maybe AsyncRead/Write are not the right abstraction, then? The errors we need to signal are upgrade errors, not I/O errors.

@nazar-pc thiserror produces std::error::Error, which has the same problem as I outline above — namely that the error causes can only be inspected via to_string. The Rust std-library unfortunately has a significant weakness in this regard; the optimal solution (which is of course not possible) would be for std::error::Error to extend Any.

[nazar-pc]

With thiserror you can have an enum that has multiple variants, some of which are other error instances, that way you can traverse it in depth if needed, not entirely sure what use case demands though.

[rkuhn]

My use-case is the following:

                let error = dial_error.to_string();
                // ... logging etc.
                if matches!(dial_error, DialError::Transport(_));
                    && retries > 0
                    && error.contains("Other(A(B(Apply(Io(Kind(InvalidData))))))")
                {
                    // TCP simultaneous open leads to both sides being initiator in the Noise
                    // handshake, which yields this particular error

The issue here cannot be fixed by thiserror or any other way of how we generate our Error impls. The issue is that the transport error type is std::io::ErrorKind::Other which doesn’t allow any other way of introspection than using to_string.

Maybe the above was not what you meant: if you meant that we replace TransportError<std::io::Error> (in DialError) with TransportError<UpgradeOrIoError> (which may well use thiserror in its implementation), then we are on the same page. This is already the case for the ConnectionError included in SwarmEvent::ConnectionClosed.

[dvc94ch]

generally considered to be used in applications, not libraries

it's quite a poor consideration for multiple reasons:

• no one using thiserror captures a backtrace
• pattern of using a single error enum per crate means that it's not useful for exhaustive matching on error conditions
• often requires stringifying the error message into Other variants
• often very nested enums one for each dependency containing that dependencies error enum
• penalizes the happy path with pattern matching instead of using unwind tables
• most errors are fatal/irrecoverable, like invalid inputs, programmer errors or transient errors with the only valid handling strategies being retry/abort/ignore
• non fatal errors are often better expressed as enums like Poll, Option or other enum on the Ok path

[rkuhn]

So what would be the optimal solution in the scenario I’m asking about?

[dvc94ch]

you can't avoid the stringification as @tomaka mentioned because io::Result is forced by the trait. the real solution would be moving anyhow into std and start relying on it pervasively. since that's unlikely to happen I guess string matching is the best you can do.

[rkuhn]

FTR: with some hints from other people I was able to avoid string matching, see

https://github.com/ipfs-rust/ipfs-embed/blob/a959d5f93fdc0f2319fabea4868bd62d46625828/src/net/peers.rs#L878-L902