Reconnect the database after any failed worker cycle (#170)

A failed cycle can leave the connection in a state it never recovers from on
its own. When a transaction is aborted and the rollback that follows also
fails, the connection throws away its own transaction bookkeeping while the
driver keeps considering a transaction open. Every later attempt to begin one
is then rejected by the driver with "There is already an active transaction",
which is not a lost connection, so the worker held on to the connection and
failed every single cycle until it was restarted by hand.

Author: LauJosefsen

src/RequestInsurance/RequestInsuranceWorker.php

@@ -139,29 +139,29 @@ public function run(bool $runOnlyOnce = false): void
      */
     protected function handleCycleFailure(Throwable $throwable): void
     {
-        if ( ! $this->wasCausedByLostConnection($throwable)) {
+        if ($this->wasCausedByLostConnection($throwable)) {
+            $this->consecutiveLostConnections++;
+
+            $message = sprintf(
+                'RequestInsurance Worker (#%s) lost its database connection during %s and is reconnecting (%d in a row)',
+                $this->runningHash,
+                $this->currentPhase,
+                $this->consecutiveLostConnections
+            );
+
+            if ($this->consecutiveLostConnections > self::QUIET_LOST_CONNECTION_RECOVERIES) {
+                Log::error($message, ['exception' => $throwable]);
+            } else {
+                Log::debug($message);
+            }
+        } else {
             $this->consecutiveLostConnections = 0;
 
             Log::error($throwable);
-
-            return;
-        }
-
-        $this->consecutiveLostConnections++;
-
-        $message = sprintf(
-            'RequestInsurance Worker (#%s) lost its database connection during %s and is reconnecting (%d in a row)',
-            $this->runningHash,
-            $this->currentPhase,
-            $this->consecutiveLostConnections
-        );
-
-        if ($this->consecutiveLostConnections > self::QUIET_LOST_CONNECTION_RECOVERIES) {
-            Log::error($message, ['exception' => $throwable]);
-        } else {
-            Log::debug($message);
         }
 
+        // The connection is given up on no matter what failed the cycle, since a failure can leave it in a
+        // state it never recovers from on its own, such as a transaction its driver still considers open.
         rescue(fn () => $this->reconnectToDatabase(), null, false);
     }
 

tests/Unit/RequestInsuranceWorkerTest.php

@@ -4,6 +4,7 @@
 
 use Exception;
 use Throwable;
+use PDOException;
 use Tests\TestCase;
 use GuzzleHttp\Psr7\Request;
 use Illuminate\Support\Facades\Log;
@@ -501,7 +502,20 @@ public function test_it_reports_cycle_failures_that_are_not_lost_connections():
 
         $worker->exposeReportCycleFailure($throwable);
 
-        $this->assertSame(0, $worker->reconnects);
+        $this->assertSame(1, $worker->reconnects);
+    }
+
+    public function test_it_gives_up_on_a_connection_holding_a_stranded_transaction(): void
+    {
+        $worker = $this->getWorkerProbe();
+
+        Log::shouldReceive('error')->times(5);
+
+        foreach (range(1, 5) as $ignored) {
+            $worker->exposeReportCycleFailure(new PDOException('There is already an active transaction'));
+        }
+
+        $this->assertSame(5, $worker->reconnects);
     }
 
     /**