Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit b048d76

Browse filesBrowse files
fabpotfabpot
committed
bug #59348 [Lock] Fix predis command error checking (dciprian-petrisor)
This PR was merged into the 7.2 branch. Discussion ---------- [Lock] Fix predis command error checking | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Issues | Fix #59087 | License | MIT We seem to have had an incompatibility with how Predis clients are initialized (`'exceptions' => false`) and the implementation of RedisStore, which only surfaced now due to the recent EVAL -> EVALSHA changes. According to https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/Cache/Traits/RedisTrait.php#L65 and https://github.com/symfony/symfony/blob/7.2/src/Symfony/Component/Cache/Traits/RedisTrait.php#L419 , the Predis client is always initialized with exceptions disabled; it returns Error objects instead. This PR fixes the tests to replicate this behaviour and the implementation. An small additional change was made regarding error checking, to ensure the initial `evalSha` does not fail with anything else besides the expected `NOSCRIPT` error. Commits ------- 4816402 Fix predis command error checking
2 parents e36382c + 4816402 commit b048d76
Copy full SHA for b048d76

File tree

3 files changed

+29
-25
lines changed
Filter options

3 files changed

+29
-25
lines changed

‎src/Symfony/Component/Lock/Store/RedisStore.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Store/RedisStore.php
+17-22Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
namespace Symfony\Component\Lock\Store;
1313

14-
use Predis\Response\ServerException;
14+
use Predis\Response\Error;
1515
use Relay\Relay;
1616
use Symfony\Component\Lock\Exception\InvalidTtlException;
1717
use Symfony\Component\Lock\Exception\LockConflictedException;
@@ -250,10 +250,10 @@ private function evaluate(string $script, string $resource, array $args): mixed
250250
}
251251

252252
$result = $this->redis->evalSha($scriptSha, array_merge([$resource], $args), 1);
253+
}
253254

254-
if (null !== $err = $this->redis->getLastError()) {
255-
throw new LockStorageException($err);
256-
}
255+
if (null !== $err = $this->redis->getLastError()) {
256+
throw new LockStorageException($err);
257257
}
258258

259259
return $result;
@@ -273,37 +273,32 @@ private function evaluate(string $script, string $resource, array $args): mixed
273273
}
274274

275275
$result = $client->evalSha($scriptSha, array_merge([$resource], $args), 1);
276+
}
276277

277-
if (null !== $err = $client->getLastError()) {
278-
throw new LockStorageException($err);
279-
}
278+
if (null !== $err = $client->getLastError()) {
279+
throw new LockStorageException($err);
280280
}
281281

282282
return $result;
283283
}
284284

285285
\assert($this->redis instanceof \Predis\ClientInterface);
286286

287-
try {
288-
return $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
289-
} catch (ServerException $e) {
290-
// Fallthrough only if we need to load the script
291-
if (self::NO_SCRIPT_ERROR_MESSAGE !== $e->getMessage()) {
292-
throw new LockStorageException($e->getMessage(), $e->getCode(), $e);
287+
$result = $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
288+
if ($result instanceof Error && self::NO_SCRIPT_ERROR_MESSAGE === $result->getMessage()) {
289+
$result = $this->redis->script('LOAD', $script);
290+
if ($result instanceof Error) {
291+
throw new LockStorageException($result->getMessage());
293292
}
294-
}
295293

296-
try {
297-
$this->redis->script('LOAD', $script);
298-
} catch (ServerException $e) {
299-
throw new LockStorageException($e->getMessage(), $e->getCode(), $e);
294+
$result = $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
300295
}
301296

302-
try {
303-
return $this->redis->evalSha($scriptSha, 1, $resource, ...$args);
304-
} catch (ServerException $e) {
305-
throw new LockStorageException($e->getMessage(), $e->getCode(), $e);
297+
if ($result instanceof Error) {
298+
throw new LockStorageException($result->getMessage());
306299
}
300+
301+
return $result;
307302
}
308303

309304
private function getUniqueToken(Key $key): string

‎src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Tests/Store/CombinedStoreTest.php
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ protected function getClockDelay(): int
3939

4040
public function getStore(): PersistingStoreInterface
4141
{
42-
$redis = new \Predis\Client(array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => 6379]));
42+
$redis = new \Predis\Client(
43+
array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => 6379]),
44+
['exceptions' => false],
45+
);
4346

4447
try {
4548
$redis->connect();

‎src/Symfony/Component/Lock/Tests/Store/PredisStoreTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Tests/Store/PredisStoreTest.php
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ class PredisStoreTest extends AbstractRedisStoreTestCase
2020
{
2121
public static function setUpBeforeClass(): void
2222
{
23-
$redis = new \Predis\Client(array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]));
23+
$redis = new \Predis\Client(
24+
array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]),
25+
['exceptions' => false],
26+
);
2427
try {
2528
$redis->connect();
2629
} catch (\Exception $e) {
@@ -30,7 +33,10 @@ public static function setUpBeforeClass(): void
3033

3134
protected function getRedisConnection(): \Predis\Client
3235
{
33-
$redis = new \Predis\Client(array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]));
36+
$redis = new \Predis\Client(
37+
array_combine(['host', 'port'], explode(':', getenv('REDIS_HOST')) + [1 => null]),
38+
['exceptions' => false],
39+
);
3440
$redis->connect();
3541

3642
return $redis;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.