From b20892fb1e0ed9084928d0e4b143efaa69d4dfff Mon Sep 17 00:00:00 2001
From: HypeMC <hypemc@gmail.com>
Date: Thu, 30 Jan 2025 23:42:50 +0100
Subject: [PATCH] [Lock] Fix Predis error handling

---
 .../Component/Lock/Store/RedisStore.php       | 42 ++++++++++++++-----
 .../Store/PredisStoreWithExceptionsTest.php   | 36 ++++++++++++++++
 ...p => PredisStoreWithoutExceptionsTest.php} |  2 +-
 3 files changed, 68 insertions(+), 12 deletions(-)
 create mode 100644 src/Symfony/Component/Lock/Tests/Store/PredisStoreWithExceptionsTest.php
 rename src/Symfony/Component/Lock/Tests/Store/{PredisStoreTest.php => PredisStoreWithoutExceptionsTest.php} (93%)

diff --git a/src/Symfony/Component/Lock/Store/RedisStore.php b/src/Symfony/Component/Lock/Store/RedisStore.php
index 5d828449131a4..f2d8a5e9766fb 100644
--- a/src/Symfony/Component/Lock/Store/RedisStore.php
+++ b/src/Symfony/Component/Lock/Store/RedisStore.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\Lock\Store;
 
 use Predis\Response\Error;
+use Predis\Response\ServerException;
 use Relay\Relay;
 use Symfony\Component\Lock\Exception\InvalidTtlException;
 use Symfony\Component\Lock\Exception\LockConflictedException;
@@ -284,21 +285,18 @@ private function evaluate(string $script, string $resource, array $args): mixed
 
         \assert($this->redis instanceof \Predis\ClientInterface);
 
-        $result = $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
-        if ($result instanceof Error && str_starts_with($result->getMessage(), self::NO_SCRIPT_ERROR_MESSAGE_PREFIX)) {
-            $result = $this->redis->script('LOAD', $script);
-            if ($result instanceof Error) {
-                throw new LockStorageException($result->getMessage());
+        try {
+            return $this->handlePredisError(fn () => $this->redis->evalSha($scriptSha, 1, $resource, ...$args));
+        } catch (LockStorageException $e) {
+            // Fallthrough only if we need to load the script
+            if (!str_starts_with($e->getMessage(), self::NO_SCRIPT_ERROR_MESSAGE_PREFIX)) {
+                throw $e;
             }
-
-            $result = $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
         }
 
-        if ($result instanceof Error) {
-            throw new LockStorageException($result->getMessage());
-        }
+        $this->handlePredisError(fn () => $this->redis->script('LOAD', $script));
 
-        return $result;
+        return $this->handlePredisError(fn () => $this->redis->evalSha($scriptSha, 1, $resource, ...$args));
     }
 
     private function getUniqueToken(Key $key): string
@@ -347,4 +345,26 @@ private function getNowCode(): string
             now = math.floor(now * 1000)
         ';
     }
+
+    /**
+     * @template T
+     *
+     * @param callable(): T $callback
+     *
+     * @return T
+     */
+    private function handlePredisError(callable $callback): mixed
+    {
+        try {
+            $result = $callback();
+        } catch (ServerException $e) {
+            throw new LockStorageException($e->getMessage(), $e->getCode(), $e);
+        }
+
+        if ($result instanceof Error) {
+            throw new LockStorageException($result->getMessage());
+        }
+
+        return $result;
+    }
 }
diff --git a/src/Symfony/Component/Lock/Tests/Store/PredisStoreWithExceptionsTest.php b/src/Symfony/Component/Lock/Tests/Store/PredisStoreWithExceptionsTest.php
new file mode 100644
index 0000000000000..6b24711b89a8e
--- /dev/null
+++ b/src/Symfony/Component/Lock/Tests/Store/PredisStoreWithExceptionsTest.php
@@ -0,0 +1,36 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Lock\Tests\Store;
+
+/**
+ * @group integration
+ */
+class PredisStoreWithExceptionsTest extends AbstractRedisStoreTestCase
+{
+    public static function setUpBeforeClass(): void
+    {
+        $redis = new \Predis\Client(array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]));
+        try {
+            $redis->connect();
+        } catch (\Exception $e) {
+            self::markTestSkipped($e->getMessage());
+        }
+    }
+
+    protected function getRedisConnection(): \Predis\Client
+    {
+        $redis = new \Predis\Client(array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]));
+        $redis->connect();
+
+        return $redis;
+    }
+}
diff --git a/src/Symfony/Component/Lock/Tests/Store/PredisStoreTest.php b/src/Symfony/Component/Lock/Tests/Store/PredisStoreWithoutExceptionsTest.php
similarity index 93%
rename from src/Symfony/Component/Lock/Tests/Store/PredisStoreTest.php
rename to src/Symfony/Component/Lock/Tests/Store/PredisStoreWithoutExceptionsTest.php
index 74a72b5a4003a..bb135a4676406 100644
--- a/src/Symfony/Component/Lock/Tests/Store/PredisStoreTest.php
+++ b/src/Symfony/Component/Lock/Tests/Store/PredisStoreWithoutExceptionsTest.php
@@ -16,7 +16,7 @@
  *
  * @group integration
  */
-class PredisStoreTest extends AbstractRedisStoreTestCase
+class PredisStoreWithoutExceptionsTest extends AbstractRedisStoreTestCase
 {
     public static function setUpBeforeClass(): void
     {