[4.2] Connection::run breaks transaction logic when connection lost inside transaction.

[pinepain]

I've being looking for a way to catch query-related errors properly to fix bug with nested transactions support (the project I'm working on now) and I found that if we start transaction, then lost connection inside running transaction, whole transaction logic will be broken due to Connection::reconnectIfMissingConnection and Connection::tryAgainIfCausedByLostConnection calls in Connection::run() method:

    protected function run($query, $bindings, Closure $callback)
    {
        $this->reconnectIfMissingConnection();

        $start = microtime(true);

        // Here we will run this query. If an exception occurs we'll determine if it was
        // caused by a connection that has been lost. If that is the cause, we'll try
        // to re-establish connection and re-run the query with a fresh connection.
        try
        {
            $result = $this->runQueryCallback($query, $bindings, $callback);
        }
        catch (QueryException $e)
        {
            $result = $this->tryAgainIfCausedByLostConnection(
                $e, $query, $bindings, $callback
            );
        }

This bug looks for me quite critical while it silently may lead to broken data in database. Especially, keeping in mind that usually transaction are used in mission-critical places where data integrity required.

I propose to skip Connection::reconnectIfMissingConnection and Connection::tryAgainIfCausedByLostConnection calls if Connection::$transactions > 0
or modify them to throw exceptions and reset Connection::$transactions to 0 when they called.

[aik099]

Does Laravel emulate transactions by grouping SQLs or really use InnoDB transactions?

[pinepain]

Actually, the real mechanism depends on what connection is used (yupp, InnoDB or other transaction-safe engines transactoins for MySqlConnection), but in general PDO::beginTransaction, PDO::commit and PDO::rollBack are used.

[aik099]

For example you have following SQLs:

BEGIN TRANSACTION
INSERT 1
INSERT 2 // connection lost after this query
INSERT 3
COMMIT TRANSACTION

then we lost connection on which BEGIN TRANSACTION was called and we'll have:

1. INSERT 3 executed without any transaction
2. COMMIT TRANSACTION may trigger SQL error because it won't match with previous BEGIN TRANSACTION call

But I agree, that if SQL error happens during transaction SQL execution, then whole transaction should be aborted with an error. Reconnecting can't re-run previously executed SQLs in that transaction.

[pinepain]

if @GrahamCampbell or/and @taylorotwell (or/and others) confirm which of proposed ways is better or prose they own I will provide PR with tests in a few hours. This bug is damn critical for me and I spent a night finding out why my database looks like after 2n World War.

P.S.: I stands for adding transaction level check > 0 to Connection::reconnectIfMissingConnection and Connection::tryAgainIfCausedByLostConnection methods while they may be used outside of Connection::run() in future.

[GrahamCampbell]

Further discussion can move to your pull - #5944.