Support for rollbacktransactionwith details variant

[jonatansalemes]

Its possible add to diesel error variants RollbackTransactionWithDetails(Box<dyn StdError + Send + Sync>). I have some cases that we want rollback for some outside db transaction reasons and want to track details about reasons later like map_err ...

type enum CustomError {
     SomeVariant(String),
     SomeOtherVariant(String),
}

let mut conn = db.get_connection().await;
    conn.transaction(|mut conn| async move {
    
           let r1 = some_db_oper(&mut conn);
           let r2 = some_another_db_oper(&mut conn);
           
           //check for invalid state
           if somecondition {
                Err(RollbackTransactionWithDetails(
                         Box::new(CustomError::SomeVariant("some error 1"))
                ))?
           }
           
           //check for invalid state
           if someanothercondition {
                Err(RollbackTransactionWithDetails(
                         Box::new(CustomError::SomeOtherVariant("some error 2"))
                ))?
           }
           
})....

current support RollbackTransaction seem does not help much in this cases. I can write PR if you guys want

[weiznich]

It's not necessary to have this details in the diesel provided error type as you can easily supply your own error type. The documentation states the function signature of Connection::transaction as follows:

fn transaction<T, E, F>(&mut self, f: F) -> Result<T, E>where
    F: FnOnce(&mut Self) -> Result<T, E>,
    E: From<Error>,

which clearly shows that you can return any custom error type from the closure as long as it implements From<diesel::result::Error>.

Also the documentation clearly states that the transaction will be rolled back for any error returned from the closure, not only for Error::RollbackTransaction:

The connection is committed if the closure returns Ok(), it will be rolled back if it returns Err(). For both cases the original result value will be returned from this function.

(Again from the linked documentation)

To summarize that all: You can just return your error type directly as long as diesel::result::Error can be converted to your type. There is no need to have a wrapping variant for that in diesels error type.

[weiznich]

To repeat that again: If you want to have a custom error you can just use a custom error. There is no need to use RollbackError at all.
If you still believe that's not the case please reread again what I've above and what's written in the linked documentation. If that still doesn't help: Please provide a complete self contained example of code that you think is problematic.

[jonatansalemes]

pub enum DomainError {
    Variant1(String),
    Variant2(String,String)
}
impl Error for DomainError {}


let mut conn = db.get_connection().await;
    conn.transaction(|conn| async move {
    
           let r1 = some_db_oper(&mut conn);
           let r2 = some_another_db_oper(&mut conn);
           
           //check for invalid state
           if somecondition {
                Err(Variant1("arg1"))?
           }
           
           //check for invalid state
           if someanothercondition {
                Err(Variant2("arg1","arg2"))?
           }
           
})

impl From<diesel::result::Error> for DomainError {
    fn from(diesel: diesel::result::Error) -> Self {
        //which variant of my error based on  diesel error since a i have no details wich variant was throws??
    }
}

[weiznich]

As written before: It's the obligation of the error type you want to use there to provide a From<diesel::result::Error> implementation and therefore to have a variant that can represent this error. If you you don't want to have that variant at DomainError you can literally have the thing you described earlier by just defining another enum for that in your crate.

The main point here is: All that's up to how your application does handle errors. It's a general thing in rust that the user of a library is responsible for integrating the error types of that library and not the other way around. The thing you described earlier is just the opposite of that.

[jonatansalemes]

It's quite evident that you (and not I) haven't grasped the current gap problem in transforming one error into another without a suitable semantic variant like RollbackWithDetails that can carry error details. Instead, I will use SerializationError, even though it doesn't provide a better solution, as it supports carrying errors internally. However, observing the way you repeatedly emphasize the same arguments, it's apparent that we're not making progress.

When acting as the maintainer of any open-source project, it's crucial to listen to others rather than adopting an ironic mindset that assumes you're the only one with intelligence, while others are lazy, don't read documentation, or lack an understanding of obvious things

[weiznich]

OK then let me word this differently: Please explain how your proposal would solve your problem in a better way that what I just pointed out as solution?

When acting as the maintainer of any open-source project, it's crucial to listen to others rather than adopting an ironic mindset that assumes you're the only one with intelligence, while others are lazy, don't read documentation, or lack an understanding of obvious things

Well you can simply avoid that by making it explicit that you read and understood the documentation (and potential related discussions). Especially you need to be upfront why you cannot use existing solutions. At least I do not see that in any of your postings.