Problem understanding adding clone trait to custom error type

help
1

I have an iced based program that graphs data onto a canvas. I wanted to be able to assemble the data needed to draw the graph in a future async task and be able to use the ? for error handling. So I wrote the future to return a Result with a data structure and a custom error (it is my understanding that all functions in the future must kick out the same error type for ? to work). The custom error needs to have the clone trait so I added it to the #[derive ... line and all but the ChronoError in the custom error enumeration kicked out the same error "the trait std::clone::Clone is not implemented. So I removed "Clone" from the #[derive ... line and treated Clone like it was similar to an error because I did not know what else to do. And for some reason it worked but is this really the way to do it or was I just very lucky. The error code is below. Any assistance will be appreciated.

#[derive(Debug)]
pub enum CustomError  {
    SqlError(rusqlite::Error), 
    CsvError(csv::Error),
    IOError(std::io::Error),
    // if I add clone to #[derive line the above 3 items each kick out the same error
    // the error is: the trait std::clone::Clone in not implemented
    ChronoError(chrono::ParseError),
    Clone(),    
}

impl fmt::Display for CustomError {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            CustomError::SqlError(error_msg) => write!(f, "SQL error {}", error_msg),
            CustomError::CsvError(error_msg) => write!(f, "CSV error {}", error_msg),
            CustomError::ChronoError(error_msg) => write!(f, "Chrono error {}", error_msg),
            CustomError::IOError(error_msg) => write!(f, "IO Error {}", error_msg),
            CustomError::Clone() => write!(f, "clone message"),
        }
    }
}
impl Clone for CustomError {
    fn clone(&self) -> Self {
        CustomError::Clone()
    }
}
impl std::error::Error for CustomError {}
impl From<rusqlite::Error> for CustomError {
    fn from(error: rusqlite::Error) -> Self {
        CustomError::SqlError(error)
    }
}
impl From<csv::Error> for CustomError {
    fn from(error: csv::Error) -> Self {
        CustomError::CsvError(error)
    }
}
impl From<chrono::ParseError> for CustomError {
    fn from(error: chrono::ParseError) -> Self {
        CustomError::ChronoError(error)
    }
}
impl From<std::io::Error> for CustomError {
    fn from(error: std::io::Error) -> Self {
        CustomError::IOError(error)
    }
}
2

the #[derive(Clone)] on an enum requires each variant is Clone. the derived Clone implementation is in the shape of:

#[derive(Clone)]
enum Foo {
    A(A),
    B { b: B },
    C,
}

// the derive macro generates code like this
impl Clone for Foo {
    fn clone(&self) -> Self {
        match self {
            Foo::A(a) => Foo::A(a.clone()),
            Foo::B { b } => Foo::B { b.clone() },
            Foo::C => Foo::C,
        }
    }
}

in your case, none of the rusqlite::Error, csv::Error, or std::io::Error type is Clone, so the derive macro doesn't work. the reason for the design? I don't know. maybe it's to hide details, maybe it's for compatibility, or maybe it's simply considerred unnecessary.

only the chrono::ParseError type implements Clone.

however, I would like to know why you need the Clone trait in the first place for an error type. can you explain your use case in detail? it's actually not common to clone errors.

typically when an error occurs, you either propagate it to the caller, or you handle the error yourself and try to recover. in these scenarios, you either move the error or consume it, but you don't need to clone it.

3

In a case where you do need the error information in multiple places (say, if you are storing it for purposes like "this data is missing because an error was encountered"), you can use Arc<CustomError> (or Arc inside of CustomError) to make it possible to share the information despite not being able to clone the third-party errors.

But outside of that situation, you should probably consider whether you actually need Clone.

4

Thank you for the input. Below is some additional code which might shed light on what I am doing wrong. I hope this helps. If more data is needed, please let me know and I will supply it. It should be noted that I have used the custom error before but to knowledge never in a future.

#[derive(Debug, Clone)]
pub enum Message {
    DisplayGraph(Result<GraphingData, CustomError>),
    Welcome,
    AssignStartDates(Vec<String>),
    Exit,
    GetGraphPoints(usize),
    UpdateGraphState(Vec<Point>, Vec<String>, Vec<String>, Vec<IndexDateClose>, StartValuesAndDeltas),
}

#[derive(Debug, Clone)]
pub struct GraphingData {
    pub points: Vec<Point>,
    pub x_labels: Vec<String>,
    pub y_labels: Vec<String>,
    pub min_max: Vec<IndexDateClose>,
    pub starts_deltas: StartValuesAndDeltas,
}

        Message::GetGraphPoints(date_index) => {

            state.graph.start_date_index = date_index;
            let index = state.start_dates[state.graph.start_date_index].clone();

            let get_data_points_task = Task::perform(
                
                async move {
                    let closing_data = closing_values(&index).await?;
                    let index_values = index_min_max_values(&closing_data).await?;
                    let index_min = index_values.0;
                    let index_max = index_values.1;
                    let min_close = index_values.2;
                    let max_close = index_values.3;

                    let graph_points = convert_data_to_points(&closing_data, min_close, max_close);
                    let x_labels = x_axis_labels(&closing_data);                   
                    let y_labels  = y_axis_labels(min_close, max_close);
                    let min_max_values = lookup_min_max(&closing_data, index_min, index_max);
                    let starts_and_deltas = starts_deltas(&closing_data, min_close, max_close);

                    let (points, x_labels, y_labels, min_max, starts_deltas) =
                        futures::try_join!(graph_points, x_labels, y_labels, min_max_values, starts_and_deltas)?;

                    Ok( GraphingData {
                            points: points,
                            x_labels: x_labels,
                            y_labels: y_labels,
                            min_max: min_max,
                            starts_deltas: starts_deltas,
                    })

                },|result|
                Message::DisplayGraph(result)
            );

            return get_data_points_task
        },

        Message::DisplayGraph(result) => {
           // listing in message enum DisplayGraph(Result<GraphingData, CustomError>),

            match result {
                Ok(data) => {
                    state.graph.graph_points = data.points;
                    state.graph.x_axis_labels = data.x_labels;
                    state.graph.y_axis_labels = data.y_labels;
                    state.graph.min_and_max_values = data.min_max;
                    state.graph.starts_and_deltas = data.starts_deltas;
                },
                Err(error) => {
                    println!("error = {}", error);
                }
            } // end match

            state.screen = Screen::DisplayGraph
        },
5

The only clone() I see in that code is cloning an index, which I assume is an integer type; the code thus seems rather irrelevant to this discussion. To repeat what the other users have said: Why do you need or believe you need your error type to be Clone?

6

To answer this part: No, there is no planet on which implementing Clone for an error by returning a "Clone" variant is the correct thing to do. The purpose of Clone is to produce a duplicate of a value, but by converting everything to CustomError::Clone, you're throwing away all information about the actual error. To be perfectly frank, the fact that you thought this was in any way acceptable indicates that you do not understand what you are doing.

7

In an attempt to work through the problem I removed all clone references. With that done, I found a struct that represents a rectangle in my canvas program that needs to have clone implemented. So I added it back in for that struct. The only error remaining is in a view. There is a button widget that on press sends a Message and the ide indicates that the trait std::clone::Clone in not implemented for enumerations::Message required by a bound in iced::widget::container. If I add clone to the Message enum that is where I think clone flows over into the custom error. I do not know how to get around this issue. Below is the button widget code and the Message enum again. I hope this helps. It is interesting to note that in my canvas update function I have a line of code (first line below) and the ide does not flag it as a problem ie it does not seem to need clone.


return Some(Action::publish(Message::GetGraphPoints(0)))

        widget::container(
            widget::button(
                widget::text("Display Graph")
                    .size(25.0)
            )
            .on_press(Message::GetGraphPoints(0))
// the trait std::clone::Clone is not implemented for enumerations::Message
// required by a bound in `iced::widget::container`
// required by a bound introduced by this call
        )
        .width(Length::Fill)
        .align_x(iced::alignment::Horizontal::Center)
        .align_y(iced::alignment::Vertical::Center)
        .padding(iced::padding::top(100)),


#[derive(Debug)]
pub enum Message {
    DisplayGraph(Result<GraphingData, CustomError>),
    Welcome,
    AssignStartDates(Vec<String>),
    Exit,
    GetGraphPoints(usize),
    UpdateGraphState(Vec<Point>, Vec<String>, Vec<String>, Vec<IndexDateClose>, StartValuesAndDeltas),
}
8

the real reason is you put the error type in the Message type, but the Message type has to be Clone: each time an event occurs, such as a button is clicked, iced needs to create a new message to send by cloning the message you set when you created the widget.

my sidebar opinion

I think this is an unintentional limitation of the iced api design. async tasks can generate new messages arbitrarily, so they don't need Clone, while the widgets mainly clone messages. but there's only one update() function so they must share the message type.

more interestingly, iced widgets actually doesn't strictly require the message type to be clonable, you can generate the message using a callback, e.g. in theory, Button::on_press() can be implemented on top of Button::on_press_with(). but the current api (and implementation) does limit to clonable message types.

if I read your post correctly, you inlcuded the Result in the Message enum because you want to use the try operator (the question mark) in the async task. you might think the async task must return the same Message type, but this is not required: you already used the Task::perform() function to create the task, which allows you to map the async return type to the Message type, but you didn't really do any meaning transformation/conversion there.

now, think about it: what's your error handling strategy in case the async task failed, say, the sqlite database was invalid? does the main application needs the precise error information, or does it only need some text to display? or maybe it just need to know the operation failed and a generic warning can be shown to the user? or it simply does nothing if no data is available?

if you really want the full error value, I would say it is acceptable to "fake" the Clone for your CustomError, since this message is only used in async tasks, it is never sent by a ui widget, so it is not actually cloned. your way to make a dummy variant CustomError::Clone can do the trick, but I would suggest the fake Clone to simply panic. alternatively, you can wrap it in an Arc, but again, it is meaningless since you never clone it.

if you can get away with some reduced error information, such as the textual description, or just a failed indicator, you can easily do so by mapping the error of the Future into a clonable type. for example:

#[derive(Debug, Clone)]
pub enum Message {
    // use `String`, which is clonable
    DisplayGraph(Result<GraphingData, String>),
    // or, use `Option` if a simple indicator is enough, or no-data equals no-op
    //DisplayGraph(Option<GraphingData>),
    Welcome,
    //...
}

let get_data_points_task = Task::perform(async move {
    // preparing data
    //...
    Ok(GraphingData {...})
}, |result: Result<GraphingData, CustomError>| {
    // first place to do error handling
    // convert the error into a text message
    let result = result.map(|e| format!("{}", e));
    // or, convert `Result` to `Option`, discard the error
    //let result = result.ok();
    Message::DisplayGraph(result)
})
9

Off the top of my head, a Clone impl is only required on your overall Message enum when the time-travel feature is enabled.

At the widget level, at least some widgets always require Clone, e.g. using button::on_press, because the button could be pressed multiple times per iteration of the event loop... Or something like that.

If you want to include something non-Clone in your message enum, which commonly happens if you are doing something that returns io::Error, then it's typical to wrap it in an Arc. You can do e.g. [fallible io function].map_err(Arc::new), which is pretty tidy.

10

This is what I needed. I changed the message enum as indicated. I converted CustomError to String in the future as shown below and deal with the error in Message::DisplayGraph in a match statement. Thank you for your perseverance. It is most appreciated.

        },|result: Result<GraphingData, CustomError>| 
        Message::DisplayGraph(result.map_err(|e| e.to_string()))
    ); // end of future
    return get_data_points_task