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

Code before:
            }
        };
        let result = conn.batch_execute(&*start_transaction_sql);
        let state = Self::get_transaction_state(conn)?;
        state.change_transaction_depth(TransactionDepthChange::IncreaseDepth, result)
    }

    fn rollback_transaction(conn: &mut Conn) -> QueryResult<()> {
        let transaction_state = Self::get_transaction_state(conn)?;
        let rollback_sql = match transaction_state.transaction_depth() {
            None => return Err(Error::NotInTransaction),
            Some(transaction_depth) if transaction_depth.get() == 1 => Cow::from("ROLLBACK"),
            Some(transaction_depth) => Cow::from(format!(
                "ROLLBACK TO SAVEPOINT diesel_savepoint_{}",
                transaction_depth.get() - 1
            )),
        };

        if let Some(msg) = transaction_state.previous_serialization_error.take() {
            // we can safely ignore the result here
            // as the only other error than the "rollback error" variants
            // is failing to load the transaction state, but that's something
            // we have already done above
            let _ = process_commit_error(
                conn,
                crate::result::Error::DatabaseError(
                    crate::result::DatabaseErrorKind::SerializationFailure,
                    Box::new(String::new()),
                ),
                rollback_sql,
            );
            let transaction_state = Self::get_transaction_state(conn)?;
            if transaction_state
                .transaction_depth
                .map(|t| t.get())
                .unwrap_or_default()
                > 0
            {
                transaction_state.previous_serialization_error = Some(msg.clone());
            }
            return Err(crate::result::Error::DatabaseError(
                crate::result::DatabaseErrorKind::SerializationFailure,
                Box::new(msg),
            ));
        }
        let result = conn.batch_execute(&*rollback_sql);
        let state = Self::get_transaction_state(conn)?;
        state.change_transaction_depth(TransactionDepthChange::DecreaseDepth, result)
    }

    /// If the transaction fails to commit due to a `SerializationFailure` or a

Review comment (suggestion):
> This "fix" just tracks at connection level if we hit a SerializationError or not and special cases the rollback correspondingly. At least from my point of view that seems like a hacky solution.

Curious what you mean by hacky? Or at least how the same logic could be achieved alternately? Mostly asking because this makes sense to me (basically an early return of sorts), and especially in the context of transactions/isolation levels -- any violation of which I would assume the right "correction" would be to immediately rollback what could be rolled back and then just bail on the rest, returning to the caller an error

Code after:
            }
        };
        let result = conn.batch_execute(&*start_transaction_sql);
        let state = Self::get_transaction_state(conn)?;
        state.change_transaction_depth(TransactionDepthChange::IncreaseDepth, result)
    }

    fn rollback_transaction(conn: &mut Conn) -> QueryResult<()> {
        let transaction_state = Self::get_transaction_state(conn)?;
        let rollback_sql = match transaction_state.transaction_depth() {
            None => return Err(Error::NotInTransaction),
            Some(transaction_depth) if transaction_depth.get() == 1 => Cow::from("ROLLBACK"),
            Some(transaction_depth) => Cow::from(format!(
                "ROLLBACK TO SAVEPOINT diesel_savepoint_{}",
                transaction_depth.get() - 1
            )),
        };

        if let Some((kind, msg)) = transaction_state
            .previous_error_relevant_for_rollback
            .take()
        {
            // we can safely ignore the result of process_commit_error here
            // as the only other error than the "rollback error" variants
            // is failing to load the transaction state, but that's something
            // we have already done above
            let _ = process_commit_error(
                conn,
                crate::result::Error::DatabaseError(kind, Box::new(String::new())),
                rollback_sql,
            );
            let transaction_state = Self::get_transaction_state(conn)?;
            if transaction_state
                .transaction_depth
                .map(|t| t.get())
                .unwrap_or_default()
                > 0
            {
                transaction_state.previous_error_relevant_for_rollback = Some((kind, msg.clone()));
            }
            return Err(crate::result::Error::DatabaseError(kind, Box::new(msg)));
        }
        let result = conn.batch_execute(&*rollback_sql);
        let state = Self::get_transaction_state(conn)?;
        state.change_transaction_depth(TransactionDepthChange::DecreaseDepth, result)
    }

    /// If the transaction fails to commit due to a `SerializationFailure` or a
    /// `ReadOnlyTransaction` a rollback will be attempted. If the rollback succeeds,
    /// the original error will be returned, otherwise the error generated by the rollback
    /// will be returned. In the second case the connection should be considered broken