[10.x] After commit callback throwing an exception causes broken tran…

…sactions afterwards (#50423)

* Fix after commit callback throwing an exception causing the transaction level to be incorrect

* Extract level being committed

src/Illuminate/Database/Concerns/ManagesTransactions.php

@@ -41,22 +41,15 @@ public function transaction(Closure $callback, $attempts = 1)
                 continue;
             }
 
+            $levelBeingCommitted = $this->transactions;
+
             try {
                 if ($this->transactions == 1) {
                     $this->fireConnectionEvent('committing');
                     $this->getPdo()->commit();
                 }
 
-                [$levelBeingCommitted, $this->transactions] = [
-                    $this->transactions,
-                    max(0, $this->transactions - 1),
-                ];
-
-                $this->transactionsManager?->commit(
-                    $this->getName(),
-                    $levelBeingCommitted,
-                    $this->transactions
-                );
+                $this->transactions = max(0, $this->transactions - 1);
             } catch (Throwable $e) {
                 $this->handleCommitTransactionException(
                     $e, $currentAttempt, $attempts
@@ -65,6 +58,12 @@ public function transaction(Closure $callback, $attempts = 1)
                 continue;
             }
 
+            $this->transactionsManager?->commit(
+                $this->getName(),
+                $levelBeingCommitted,
+                $this->transactions
+            );
+
             $this->fireConnectionEvent('committed');
 
             return $callbackResult;

tests/Integration/Database/EloquentTransactionWithAfterCommitTests.php

@@ -101,6 +101,45 @@ public function testObserverIsCalledEvenWhenDeeplyNestingTransactions()
         $this->assertTrue($user1->exists);
         $this->assertEquals(1, $observer::$calledTimes, 'Failed to assert the observer was called once.');
     }
+
+    public function testTransactionCallbackExceptions()
+    {
+        [$firstObject, $secondObject] = [
+            new EloquentTransactionWithAfterCommitTestsTestObjectForTransactions(),
+            new EloquentTransactionWithAfterCommitTestsTestObjectForTransactions(),
+        ];
+
+        $rootTransactionLevel = DB::transactionLevel();
+
+        // After commit callbacks may fail with an exception. When they do, the rest of the callbacks are not
+        // executed. It's important that the transaction would already be committed by that point, so the
+        // transaction level should be modified before executing any callbacks. Also, exceptions in the
+        // callbacks should not affect the connection's transaction level.
+        $this->assertThrows(function () use ($rootTransactionLevel, $secondObject, $firstObject) {
+            DB::transaction(function () use ($rootTransactionLevel, $firstObject, $secondObject) {
+                DB::transaction(function () use ($rootTransactionLevel, $firstObject) {
+                    $this->assertSame($rootTransactionLevel + 2, DB::transactionLevel());
+
+                    DB::afterCommit(function () use ($rootTransactionLevel, $firstObject) {
+                        $this->assertSame($rootTransactionLevel, DB::transactionLevel());
+
+                        $firstObject->handle();
+                    });
+                });
+
+                $this->assertSame($rootTransactionLevel + 1, DB::transactionLevel());
+
+                DB::afterCommit(fn () => throw new \RuntimeException());
+                DB::afterCommit(fn () => $secondObject->handle());
+            });
+        }, \RuntimeException::class);
+
+        $this->assertSame($rootTransactionLevel, DB::transactionLevel());
+
+        $this->assertTrue($firstObject->ran);
+        $this->assertFalse($secondObject->ran);
+        $this->assertEquals(1, $firstObject->runs);
+    }
 }
 
 class EloquentTransactionWithAfterCommitTestsUserObserver
@@ -150,3 +189,16 @@ public function handle(): void
         });
     }
 }
+
+class EloquentTransactionWithAfterCommitTestsTestObjectForTransactions
+{
+    public $ran = false;
+
+    public $runs = 0;
+
+    public function handle()
+    {
+        $this->ran = true;
+        $this->runs++;
+    }
+}