Feature request: impl From<Error> for io::Error

[becky112358]

Firstly: Thank you for this crate. :)

It would be helpful if From<Error> for io::Error was implemented, so that applications using this crate can use the ? operator on both crate Results and std::io::Results.

I think the implementation would be performed in errors.rs and would look as follows (perhaps with even more match cases filled in):

impl From<Error> for io::Error {
    fn from(err: Error) -> io::Error {
        match err {
            Error::Io(e) => e,
            Error::Timeout => io::Error::new(io::ErrorKind::TimedOut, err),
            _ => io::Error::new(io::ErrorKind::Other, err),
        }
    }
}

I tested this locally with my own code, and it does indeed allow me to use the ? operator on Results from this crate.

[fpagliughi]

Doesn't the existing implementation catch it? Or do you want something more fine-grained?

https://github.com/eclipse/paho.mqtt.rust/blob/9df0e5072996773b97bf1fc8156c5f9046776439/src/errors.rs#L50-L52

[becky112358]

I don't think the existing implementation catches it, but I'm still learning and can believe I'm wrong!

I created this repository to demonstrate my feature request:
https://github.com/becky112358/rust_result_experimentation

Essentially the repository code is as follows:

use std::io::Result;
use std::process::Command;

fn main() -> Result<()> {
    Command::new("ping").arg("192.168.1.1").output()?;

    chrono::Duration::seconds(-5).to_std();

    hm305p::get_voltage_mv()?;

    paho_mqtt::Client::new(String::from("192.168.1.1"));

    serialport::available_ports()?;

    Ok(())
}

If I try to put the ? operator at the end of the paho_mqtt line, I get the following build error:

^ the trait `From<paho_mqtt::Error>` is not implemented for `std::io::Error`

I acknowledge that I could use anyhow crate - but it would be an even more amazing solution to have From<paho_mqtt::Error> implemented for std::io::Error. :)

[fpagliughi]

Oh, I'm sorry. I misread your first post.

I was thinking of it the other way around.... With the existing code you can create an mqtt_paho::Error from a std::io::Error, but not going in the other direction. So potentially, if you made main() return a paho_mqtt::Result<()> it might work, though you might need to coerce some of the errors like from hm305p to behave properly.

But your suggestion sounds reasonable. (Technically MQTT is an I/O library and its errors can be considered I/O errors, right?) I hadn't even thought of it. And the implementation you suggest looks like the proper way to do it.

I'll test it out.

But have you tried the anyhow crate? It's a good way to deal with heterogenous errors in a Rust application, like the one you show. Even if I get the suggested fix in, anyhow may still be useful to easily add a context to the errors. Something like:

use anyhow::{Result, Context};
use std::process::Command;

fn main() -> Result<()> {
    hm305p::get_voltage_mv()
        .context("Couldn't retrieve voltage")?;

    serialport::available_ports()?;

    let _cli = paho_mqtt::Client::new("192.168.1.1")
        .context("Error connecting to MQTT broker")?;

    Command::new("ping").arg("192.168.1.1").output()
        .context("Failed to run the 'ping' command")?;

    let _ = chrono::Duration::seconds(-5).to_std();

    Ok(())
}

On my machine, I get:

$ ./target/debug/result 
Error: Couldn't retrieve voltage

Caused by:
    Cannot find power supply

[fpagliughi]

Oh, and of course you could just catch a generic error if you didn't want to add the anyhow crate:

use std::{
    error::Error,
    process::Command,
};

fn main() -> Result<(), Box<dyn Error + 'static>> {

    Command::new("ping").arg("192.168.1.1").output()?;

    chrono::Duration::seconds(-5).to_std();

    hm305p::get_voltage_mv()?;

    paho_mqtt::Client::new(String::from("192.168.1.1"));

    serialport::available_ports()?;

    Ok(())
}

[fpagliughi]

Added to the develop branch. I'll go out with the next release.

[becky112358]

Yaaay! Thank you!! 😃💚