Skip to content

Commit

Permalink
Merge pull request #69 from boesing/bugfix/redis-exception-handling
Browse files Browse the repository at this point in the history 
Handle `RedisException` in `RedisResourceManager`
  • Loading branch information
@Ocramius
Ocramius committed Dec 17, 2022
2 parents d5a7c96 + b0e4eea commit 0135f35
Show file tree
Hide file tree
Showing 12 changed files with 277 additions and 136 deletions.
91 changes: 15 additions & 76 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.29.0@7ec5ffbd5f68ae03782d7fd33fff0c45a69f95b3">
<files psalm-version="4.30.0@d0bc6e25d89f649e4f36a534f330f8bb4643dd69">
<file src="src/Redis.php">
<InvalidArgument occurrences="4">
<code>$ttl</code>
Expand Down Expand Up @@ -39,14 +39,6 @@
<MixedArgumentTypeCoercion occurrences="1">
<code>$normalizedKeys</code>
</MixedArgumentTypeCoercion>
<MixedAssignment occurrences="6">
<code>$namespacedKeyValuePairs[$this-&gt;namespacePrefix . $normalizedKey]</code>
<code>$normalizedKey</code>
<code>$serializer</code>
<code>$serializer</code>
<code>$value</code>
<code>$value</code>
</MixedAssignment>
<MixedInferredReturnType occurrences="2">
<code>RedisOptions</code>
<code>int|float</code>
Expand Down Expand Up @@ -84,10 +76,6 @@
<PropertyNotSetInConstructor occurrences="1">
<code>Redis</code>
</PropertyNotSetInConstructor>
<RedundantCastGivenDocblockType occurrences="2">
<code>(string) $namespace</code>
<code>(string) $prefix</code>
</RedundantCastGivenDocblockType>
<ReservedWord occurrences="1">
<code>$info</code>
</ReservedWord>
Expand All @@ -108,11 +96,6 @@
<MoreSpecificReturnType occurrences="1">
<code>RedisOptions</code>
</MoreSpecificReturnType>
<RedundantCastGivenDocblockType occurrences="3">
<code>(string) $namespace</code>
<code>(string) $namespaceSeparator</code>
<code>(string) $resourceId</code>
</RedundantCastGivenDocblockType>
</file>
<file src="src/RedisResourceManager.php">
<ArgumentTypeCoercion occurrences="1">
Expand All @@ -134,19 +117,28 @@
<code>normalizePersistentId</code>
<code>normalizeServer</code>
</MissingReturnType>
<MixedArgument occurrences="9">
<MixedArgument occurrences="19">
<code>$k</code>
<code>$key</code>
<code>$redis</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource['database']</code>
<code>$resource['lib_options']</code>
<code>$resource['persistent_id']</code>
<code>$resource['persistent_id']</code>
<code>$resource['server']</code>
<code>$server</code>
<code>$server['host']</code>
<code>$server['host']</code>
<code>$server['host']</code>
<code>$server['host']</code>
<code>$server['port']</code>
<code>$server['port']</code>
<code>$server['timeout']</code>
<code>$server['timeout']</code>
<code>$server['timeout']</code>
</MixedArgument>
<MixedArrayAccess occurrences="37">
<code>$info['redis_version']</code>
<MixedArrayAccess occurrences="36">
<code>$resource['database']</code>
<code>$resource['initialized']</code>
<code>$resource['initialized']</code>
Expand Down Expand Up @@ -199,39 +191,6 @@
<MixedArrayOffset occurrences="1">
<code>$libOptions[$constValue]</code>
</MixedArrayOffset>
<MixedAssignment occurrences="31">
<code>$constValue</code>
<code>$info</code>
<code>$k</code>
<code>$key</code>
<code>$key</code>
<code>$libOptions[$constValue]</code>
<code>$redis</code>
<code>$redis</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource</code>
<code>$resource['version']</code>
<code>$resource['version']</code>
<code>$result[$key]</code>
<code>$server</code>
<code>$success</code>
<code>$success</code>
<code>$success</code>
<code>$success</code>
<code>$v</code>
<code>$value</code>
<code>$value</code>
</MixedAssignment>
<MixedInferredReturnType occurrences="8">
<code>RedisResource</code>
<code>array</code>
Expand All @@ -242,17 +201,9 @@
<code>string</code>
<code>string|null</code>
</MixedInferredReturnType>
<MixedMethodCall occurrences="10">
<code>auth</code>
<code>connect</code>
<code>connect</code>
<code>connect</code>
<MixedMethodCall occurrences="2">
<code>getOption</code>
<code>getOption</code>
<code>info</code>
<code>pconnect</code>
<code>select</code>
<code>setOption</code>
</MixedMethodCall>
<MixedReturnStatement occurrences="8">
<code>$resource['database']</code>
Expand All @@ -270,18 +221,6 @@
<RedundantCast occurrences="1">
<code>(int) $server['port']</code>
</RedundantCast>
<RedundantCastGivenDocblockType occurrences="4">
<code>(int) $database</code>
<code>(int) $key</code>
<code>(string) $id</code>
<code>(string) $persistentId</code>
</RedundantCastGivenDocblockType>
<ReservedWord occurrences="1">
<code>$info</code>
</ReservedWord>
<UndefinedMethod occurrences="1">
<code>$info</code>
</UndefinedMethod>
</file>
<file src="test/integration/Laminas/RedisTest.php">
<MixedArgument occurrences="2">
Expand Down
22 changes: 22 additions & 0 deletions psalm.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
errorBaseline="psalm-baseline.xml"
findUnusedPsalmSuppress="true"
>
<projectFiles>
<directory name="src"/>
Expand All @@ -26,6 +27,27 @@
<referencedMethod name="Laminas\Cache\Storage\Adapter\RedisClusterOptions::setResourceManager"/>
</errorLevel>
</InternalMethod>
<!--
Since these are usually suppressed anyway AND psalm is already thinking about removing this issue at all,
suppressing these might be a good idea. See https://github.com/vimeo/psalm/pull/8776 for more information.
-->
<MixedAssignment>
<errorLevel type="suppress">
<directory name="*"/>
</errorLevel>
</MixedAssignment>
<!--
Mark these redundant casts as info as some docblocks are not accurate about the return values.
The redis extension does not provide integer exception codes and thus, passing null as a code starts
being deprecated starting with PHP 8.1 and might trigger errors in PHP 9.0 and thus, lets allow
casts for docblock types. Psalm has also `RedundantCast` which is shown when a native type-hint is being
cast. i.e. such as property type or return type.
-->
<RedundantCastGivenDocblockType>
<errorLevel type="info">
<directory name="*"/>
</errorLevel>
</RedundantCastGivenDocblockType>
</issueHandlers>
<plugins>
<pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
Expand Down
13 changes: 13 additions & 0 deletions src/Exception/RedisRuntimeException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
namespace Laminas\Cache\Storage\Adapter\Exception;

use Laminas\Cache\Exception\RuntimeException as LaminasCacheRuntimeException;
use Redis;
use RedisCluster;
use RedisClusterException;
use RedisException;
use Throwable;

final class RedisRuntimeException extends LaminasCacheRuntimeException
Expand All @@ -26,4 +28,15 @@ public static function fromFailedConnection(Throwable $exception): self
$exception
);
}

public static function fromRedisException(RedisException $exception, Redis $redis): self
{
try {
$message = $redis->getLastError() ?? $exception->getMessage();
} catch (RedisException $exceptionThrownByGetLastErrorMethod) {
$message = $exception->getMessage();
}

return new self($message, (int) $exception->getCode(), $exception);
}
}
6 changes: 0 additions & 6 deletions src/RedisCluster.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ protected function internalGetItem(&$normalizedKey, &$success = null, &$casToken
return null;
}

/** @psalm-suppress MixedAssignment */
$value = $casToken = $values[$normalizedKey];
$success = true;
return $value;
Expand All @@ -202,15 +201,13 @@ protected function internalGetItems(array &$normalizedKeys): array
}

$result = [];
/** @psalm-suppress MixedAssignment */
foreach ($resultsByIndex as $keyIndex => $value) {
$normalizedKey = $normalizedKeys[$keyIndex];
$namespacedKey = $namespacedKeys[$keyIndex];
if ($value === false && ! $this->isFalseReturnValuePersisted($redis, $namespacedKey)) {
continue;
}

/** @psalm-suppress MixedAssignment */
$result[$normalizedKey] = $value;
}

Expand Down Expand Up @@ -299,15 +296,13 @@ protected function internalSetItems(array &$normalizedKeyValuePairs): array
$ttl = (int) $this->getOptions()->getTtl();

$namespacedKeyValuePairs = [];
/** @psalm-suppress MixedAssignment */
foreach ($normalizedKeyValuePairs as $normalizedKey => $value) {
$namespacedKeyValuePairs[$this->createNamespacedKey((string) $normalizedKey)] = $value;
}

$successByKey = [];

try {
/** @psalm-suppress MixedAssignment */
foreach ($namespacedKeyValuePairs as $key => $value) {
if ($ttl) {
/**
Expand Down Expand Up @@ -443,7 +438,6 @@ private function clusterException(
*/
private function isFalseReturnValuePersisted(RedisClusterFromExtension $redis, string $key): bool
{
/** @psalm-suppress MixedAssignment */
$serializer = $this->getLibOption(RedisClusterFromExtension::OPT_SERIALIZER);
if ($serializer === RedisClusterFromExtension::SERIALIZER_NONE) {
return false;
Expand Down
12 changes: 0 additions & 12 deletions src/RedisClusterResourceManager.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,10 @@ private function applyLibraryOptions(
RedisClusterFromExtension $resource,
array $options
): RedisClusterFromExtension {
/** @psalm-suppress MixedAssignment */
foreach ($options as $option => $value) {
/**
* @see https://github.com/phpredis/phpredis#setoption
*
* @psalm-suppress InvalidArgument
* @psalm-suppress MixedArgument
*/
$resource->setOption($option, $value);
Expand All @@ -157,11 +155,6 @@ private function mergeLibraryOptionsFromCluster(array $options, RedisClusterFrom
continue;
}

/**
* @see https://github.com/phpredis/phpredis#getoption
*
* @psalm-suppress InvalidArgument
*/
$options[$option] = $resource->getOption($option);
}

Expand All @@ -178,11 +171,6 @@ public function getLibOption(int $option)
return $this->libraryOptions[$option];
}

/**
* @see https://github.com/phpredis/phpredis#getoption
*
* @psalm-suppress InvalidArgument
*/
return $this->libraryOptions[$option] = $this->getResource()->getOption($option);
}

Expand Down
Loading

0 comments on commit 0135f35

Please sign in to comment.