From d21fab0d0b189e634db44a2221adb0df778a7f05 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 25 Jan 2016 20:16:47 +0100 Subject: [PATCH] [Cache] Handle and log errors properly --- .../Cache/Adapter/AbstractAdapter.php | 102 ++++++++++++++---- .../Component/Cache/Adapter/ArrayAdapter.php | 12 ++- src/Symfony/Component/Cache/CacheItem.php | 21 ++++ src/Symfony/Component/Cache/composer.json | 6 +- 4 files changed, 115 insertions(+), 26 deletions(-) diff --git a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php index 9570d100b00bf..0cf42f7addb71 100644 --- a/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/AbstractAdapter.php @@ -13,14 +13,18 @@ use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Symfony\Component\Cache\CacheItem; use Symfony\Component\Cache\Exception\InvalidArgumentException; /** * @author Nicolas Grekas */ -abstract class AbstractAdapter implements CacheItemPoolInterface +abstract class AbstractAdapter implements CacheItemPoolInterface, LoggerAwareInterface { + use LoggerAwareTrait; + private $namespace; private $deferred = array(); private $createCacheItem; @@ -119,8 +123,12 @@ public function getItem($key) $isHit = false; $value = null; - foreach ($this->doFetch(array($id)) as $value) { - $isHit = true; + try { + foreach ($this->doFetch(array($id)) as $value) { + $isHit = true; + } + } catch (\Exception $e) { + CacheItem::log($this->logger, 'Failed to fetch key "{key}"', array('key' => $key, 'exception' => $e)); } return $f($key, $value, $isHit); @@ -139,7 +147,12 @@ public function getItems(array $keys = array()) foreach ($keys as $key) { $ids[$key] = $this->getId($key); } - $items = $this->doFetch($ids); + try { + $items = $this->doFetch($ids); + } catch (\Exception $e) { + CacheItem::log($this->logger, 'Failed to fetch requested items', array('keys' => $keys, 'exception' => $e)); + $items = array(); + } $ids = array_flip($ids); return $this->generateItems($items, $ids); @@ -154,15 +167,28 @@ public function hasItem($key) if (isset($this->deferred[$key])) { $item = (array) $this->deferred[$key]; - $ok = $this->doSave(array($item[CacheItem::CAST_PREFIX.'key'] => $item[CacheItem::CAST_PREFIX.'value']), $item[CacheItem::CAST_PREFIX.'lifetime']); - unset($this->deferred[$key]); - - if (true === $ok || array() === $ok) { - return true; + try { + $e = null; + $value = $item[CacheItem::CAST_PREFIX.'value']; + $ok = $this->doSave(array($key => $value), $item[CacheItem::CAST_PREFIX.'lifetime']); + unset($this->deferred[$key]); + + if (true === $ok || array() === $ok) { + return true; + } + } catch (\Exception $e) { } + $type = is_object($value) ? get_class($value) : gettype($value); + CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e)); } - return $this->doHave($id); + try { + return $this->doHave($id); + } catch (\Exception $e) { + CacheItem::log($this->logger, 'Failed to check if key "{key}" is cached', array('key' => $key, 'exception' => $e)); + + return false; + } } /** @@ -172,7 +198,13 @@ public function clear() { $this->deferred = array(); - return $this->doClear($this->namespace); + try { + return $this->doClear($this->namespace); + } catch (\Exception $e) { + CacheItem::log($this->logger, 'Failed to clear the cache', array('exception' => $e)); + + return false; + } } /** @@ -195,7 +227,29 @@ public function deleteItems(array $keys) unset($this->deferred[$key]); } - return $this->doDelete($ids); + try { + if ($this->doDelete($ids)) { + return true; + } + } catch (\Exception $e) { + } + + $ok = true; + + // When bulk-save failed, retry each item individually + foreach ($ids as $key => $id) { + try { + $e = null; + if ($this->doDelete(array($id))) { + continue; + } + } catch (\Exception $e) { + } + CacheItem::log($this->logger, 'Failed to delete key "{key}"', array('key' => $key, 'exception' => $e)); + $ok = false; + } + + return $ok; } /** @@ -225,9 +279,9 @@ public function saveDeferred(CacheItemInterface $item) try { $item = clone $item; } catch (\Exception $e) { - @trigger_error($e->__toString()); - - return false; + $value = $item->get(); + $type = is_object($value) ? get_class($value) : gettype($value); + CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e)); } $this->deferred[$item->getKey()] = $item; @@ -243,8 +297,12 @@ public function commit() $ko = array(); foreach ($f($this->deferred, $this->namespace) as $lifetime => $values) { - if (true === $ok = $this->doSave($values, $lifetime)) { - continue; + try { + if (true === $ok = $this->doSave($values, $lifetime)) { + continue; + } + } catch (\Exception $e) { + $ok = false; } if (false === $ok) { $ok = array_keys($values); @@ -260,11 +318,15 @@ public function commit() // When bulk-save failed, retry each item individually foreach ($ko as $lifetime => $values) { foreach ($values as $v) { - if (!in_array($this->doSave($v, $lifetime), array(true, array()), true)) { + try { + $e = $this->doSave($v, $lifetime); + } catch (\Exception $e) { + } + if (true !== $e && array() !== $e) { $ok = false; - foreach ($v as $key => $value) { - @trigger_error(sprintf('Failed to cache key "%s" of type "%s"', is_object($value) ? get_class($value) : gettype($value))); + $type = is_object($value) ? get_class($value) : gettype($value); + CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e instanceof \Exception ? $e : null)); } } } diff --git a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php index e74c0eefad1c1..64d0a4696d521 100644 --- a/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php +++ b/src/Symfony/Component/Cache/Adapter/ArrayAdapter.php @@ -13,14 +13,18 @@ use Psr\Cache\CacheItemInterface; use Psr\Cache\CacheItemPoolInterface; +use Psr\Log\LoggerAwareInterface; +use Psr\Log\LoggerAwareTrait; use Symfony\Component\Cache\CacheItem; use Symfony\Component\Cache\Exception\InvalidArgumentException; /** * @author Nicolas Grekas */ -class ArrayAdapter implements CacheItemPoolInterface +class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface { + use LoggerAwareTrait; + private $values = array(); private $expiries = array(); private $createCacheItem; @@ -125,11 +129,9 @@ public function save(CacheItemInterface $item) if (is_object($value)) { try { $value = clone $value; - } catch (\Error $e) { } catch (\Exception $e) { - } - if (isset($e)) { - @trigger_error($e->__toString()); + $type = is_object($value) ? get_class($value) : gettype($value); + CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e)); return false; } diff --git a/src/Symfony/Component/Cache/CacheItem.php b/src/Symfony/Component/Cache/CacheItem.php index 0237670846897..e46cdef33cf35 100644 --- a/src/Symfony/Component/Cache/CacheItem.php +++ b/src/Symfony/Component/Cache/CacheItem.php @@ -12,6 +12,7 @@ namespace Symfony\Component\Cache; use Psr\Cache\CacheItemInterface; +use Psr\Log\LoggerInterface; use Symfony\Component\Cache\Exception\InvalidArgumentException; /** @@ -105,4 +106,24 @@ public function expiresAfter($time) return $this; } + + /** + * Internal logging helper. + * + * @internal + */ + public function log(LoggerInterface $logger = null, $message, $context = array()) + { + if ($logger) { + $logger->warning($message, $context); + } else { + $replace = array(); + foreach ($context as $k => $v) { + if (is_scalar($v)) { + $replace['{'.$k.'}'] = $v; + } + } + @trigger_error(strtr($message, $replace), E_USER_WARNING); + } + } } diff --git a/src/Symfony/Component/Cache/composer.json b/src/Symfony/Component/Cache/composer.json index ad315c9fc364c..ee8dc226bfdf6 100644 --- a/src/Symfony/Component/Cache/composer.json +++ b/src/Symfony/Component/Cache/composer.json @@ -20,12 +20,16 @@ }, "require": { "php": ">=5.5.9", - "psr/cache": "~1.0" + "psr/cache": "~1.0", + "psr/log": "~1.0" }, "require-dev": { "cache/integration-tests": "dev-master", "doctrine/cache": "~1.6" }, + "suggest": { + "symfony/polyfill-apcu": "For using ApcuAdapter on HHVM" + }, "autoload": { "psr-4": { "Symfony\\Component\\Cache\\": "" }, "exclude-from-classmap": [