Diesel: Remove Trait bound From<Error> from Connection::transaction

[gliderkite]

I am working on a crate that makes use of an Error type defined in a dependency library, which I use as return type of a method that needs to be run as part of a transaction, such as for example:

use my_lib::Error;

fn foo() -> Result<(), Error>;

conn.transaction(|| { foo() });

This results in the following error when compiling:

error[E0277]: the trait bound my_lib::Error: std::convert::From<diesel::result::Error> is not satisfied

AFAIK it's it not possible for me to create the From<Error> implementation required by this method, because the my_lib::Error type is defined in another crate. And I have to create my own TransactionError dummy type just to map the my_lib::Error (and map it back at the end of the transaction).

Is there a strong reason to have this Trait bound in place?
Would it be possible to remove it and make the API more flexible?

[weiznich]

Please make a concrete suggestions how a more flexible API would look like if we remove that bound. Also keep in mind that every use case that is supported today should be supported in the future, that includes using a custom error type as return type of the transaction callback.

I'm closing this issue now, because it is currently not actionable for the diesel team. If you have any concrete suggestions we can talk about reopening this issue.

[gliderkite]

I wasn't aware of this policy in Diesel (this was the first issue I opened); in fact I see many open tickets that are not actionable today but still open, for example https://github.com/diesel-rs/diesel/issues/2084, that by chance redirected me to this comment, that could be applied here as well https://github.com/diesel-rs/diesel/issues/399#issuecomment-603491154. I'll reopen this issue in case I'll find the time to think to an alternative, and leave to you the job to redirect other people opening similar issues to this closed one.

[weiznich]

I probably should explain why I closed this issue a bit more in detail.
The corresponding code for the transaction function is here , to show what we are talking about. We have the following constraints there:

• We need to return some error type from that function because a transaction may fail
• A potential user should be able to provide domain specific callbacks there, this implies they may return their own error type
• We need to have some way to handle an internal error (like connection was lost, or something like that) wile doing the transaction handling.

This leave us in my point of view with exactly two choices:

1. The current way, saying that there must be a way to convert a diesel error to the user provided error type. As an requirement this needs the From<diesel::result::Error> bound there.
2. Wrapping the user provided error type somehow in a diesel provided error type. This would also require some bound on the user provided error type there to be useful, probably std::error::Error or something like that.

That means in the end both solutions would require a trait bound on the user provided error type. So in the general case non of those solutions will solve your problem.

in fact I see many open tickets that are not actionable today but still open, for example #2084,

I consider this issue as actionable, see #2257.

I'll reopen this issue in case I'll find the time to think to an alternative, and leave to you the job to redirect other people opening similar issues to this closed one.

To make it clear again: Closing a issue does not mean the discussion here is done, just that I do not see what could be done immediately to solve the issue. Basically all of the open issues should contain something a potential contributor could pick up and work on. (I know that is not necessarily the case for some older issue)

As a potential workaround for your concrete issue: You could just do your own error type that wraps diesel::result::Error and the third party error type into a common type. So something like

enum MyError<E> {
    DieselError(diesel::result::Error),
    LibraryError(E)
}

impl<E> From<diesel::result::Error> for MyError<E> {
    fn from(e: diesel::result::Error) -> Self {
        MyError::DieselError(e)
    }
}

• wrapping the callback to do a additional error transformation should just work for you.