Repository: diesel-rs/diesel
File: diesel/src/connection/transaction_manager.rs (Rust)

Code before:
        use crate::connection::{
            Connection, ConnectionGatWorkaround, SimpleConnection, TransactionManager,
        };
        use crate::expression::QueryMetadata;
        use crate::query_builder::{AsQuery, QueryFragment, QueryId};
        use crate::result::{Error, QueryResult};
        use crate::test_helpers::TestConnection;

        pub(crate) struct MockConnection {
            pub(crate) next_result: Option<QueryResult<usize>>,
            pub(crate) next_batch_execute_result: Option<QueryResult<()>>,
            pub(crate) broken: bool,
            transaction_state: AnsiTransactionManager,
        }

        impl SimpleConnection for MockConnection {
            fn batch_execute(&mut self, _query: &str) -> QueryResult<()> {
                self.next_batch_execute_result
                    .take()
                    .expect("No next result")
            }
        }

        impl<'conn, 'query>
            ConnectionGatWorkaround<'conn, 'query, <TestConnection as Connection>::Backend>
            for MockConnection
        {
            type Cursor = <TestConnection as ConnectionGatWorkaround<
                'conn,
                'query,
                <TestConnection as Connection>::Backend,
            >>::Cursor;

            type Row = <TestConnection as ConnectionGatWorkaround<
                'conn,
                'query,
                <TestConnection as Connection>::Backend,
            >>::Row;
        }

        impl CommitErrorProcessor for MockConnection {
            fn process_commit_error(&self, error: Error) -> CommitErrorOutcome {
                if self.broken {
                    CommitErrorOutcome::ThrowAndMarkManagerAsBroken(error)
                } else {
                    CommitErrorOutcome::Throw(error)
                }
            }
        }

        impl Connection for MockConnection {

Review comment (bug):
More for my own understanding than anything, since we decided to introduce a generic error relevant for rollback I wonder two things
1. Should `is_serialization_error` be named something else like `is_error_relevant_for_rollback`?
2. Should the matching on line 315-321 just forward the error kind of the `Some(transaction_state. previous_error_relevant_for_rollback)` (I think it's a tuple so maybe can just return the `.0`th element of that `Some()`)

Asking because it seems to me like we should just forward along anything inside the `if` check on line 314 regardless of whether it matches a `crate::result::DatabaseErrorKind::SerializationFailure` or not

Code after:
    where
        F: FnOnce(&mut Conn) -> Result<R, E>,
        E: From<Error>,
    {
        Self::begin_transaction(conn)?;
        match f(&mut *conn) {
            Ok(value) => {
                Self::commit_transaction(conn)?;
                Ok(value)
            }
            Err(e) => {
                let transaction_state = Self::get_transaction_state(conn)?;
                let is_serialization_error = transaction_state
                    .previous_error_relevant_for_rollback
                    .is_some();

                Self::rollback_transaction(conn).map_err(|e| {
                    if is_serialization_error {
                        if let Error::DatabaseError(
                            crate::result::DatabaseErrorKind::SerializationFailure,
                            _,
                        ) = e
                        {
                            return e;
                        }
                    }

                    Error::RollbackError(Box::new(e))
                })?;
                Err(e)
            }
        }
    }
}

fn process_commit_error<Conn>(
    conn: &mut Conn,
    error: Error,
    rollback_sql: Cow<'_, str>,
) -> QueryResult<()>
where
    Conn: Connection<TransactionManager = AnsiTransactionManager> + CommitErrorProcessor,
{
    let commit_error_outcome = conn.process_commit_error(error);
    let state = AnsiTransactionManager::get_transaction_state(conn)?;
    match commit_error_outcome {
        CommitErrorOutcome::RollbackAndThrow(error) => {
            // We should try to rollback the transaction here
            let rollback_result = conn.batch_execute(&*rollback_sql);
            let state = AnsiTransactionManager::get_transaction_state(conn)?;
            let rollback_result = state