Skip to content

Commit c5a7886

Browse files
fabpotfabpot
committed
feature #17530 [Cache] Handle and log errors properly (nicolas-grekas)
This PR was merged into the 3.1-dev branch. Discussion ---------- [Cache] Handle and log errors properly | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Best viewed ignoring white-spaces: https://github.com/symfony/symfony/pull/17530/files?w=1 Commits ------- d21fab0 [Cache] Handle and log errors properly
2 parents 48d91a8 + d21fab0 commit c5a7886

File tree

4 files changed

+115
-26
lines changed

4 files changed

+115
-26
lines changed

‎src/Symfony/Component/Cache/Adapter/AbstractAdapter.php‎

Lines changed: 82 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@
1313

1414
use Psr\Cache\CacheItemInterface;
1515
use Psr\Cache\CacheItemPoolInterface;
16+
use Psr\Log\LoggerAwareInterface;
17+
use Psr\Log\LoggerAwareTrait;
1618
use Symfony\Component\Cache\CacheItem;
1719
use Symfony\Component\Cache\Exception\InvalidArgumentException;
1820

1921
/**
2022
* @author Nicolas Grekas <p@tchwork.com>
2123
*/
22-
abstract class AbstractAdapter implements CacheItemPoolInterface
24+
abstract class AbstractAdapter implements CacheItemPoolInterface, LoggerAwareInterface
2325
{
26+
use LoggerAwareTrait;
27+
2428
private $namespace;
2529
private $deferred = array();
2630
private $createCacheItem;
@@ -119,8 +123,12 @@ public function getItem($key)
119123
$isHit = false;
120124
$value = null;
121125

122-
foreach ($this->doFetch(array($id)) as $value) {
123-
$isHit = true;
126+
try {
127+
foreach ($this->doFetch(array($id)) as $value) {
128+
$isHit = true;
129+
}
130+
} catch (\Exception $e) {
131+
CacheItem::log($this->logger, 'Failed to fetch key "{key}"', array('key' => $key, 'exception' => $e));
124132
}
125133

126134
return $f($key, $value, $isHit);
@@ -139,7 +147,12 @@ public function getItems(array $keys = array())
139147
foreach ($keys as $key) {
140148
$ids[$key] = $this->getId($key);
141149
}
142-
$items = $this->doFetch($ids);
150+
try {
151+
$items = $this->doFetch($ids);
152+
} catch (\Exception $e) {
153+
CacheItem::log($this->logger, 'Failed to fetch requested items', array('keys' => $keys, 'exception' => $e));
154+
$items = array();
155+
}
143156
$ids = array_flip($ids);
144157

145158
return $this->generateItems($items, $ids);
@@ -154,15 +167,28 @@ public function hasItem($key)
154167

155168
if (isset($this->deferred[$key])) {
156169
$item = (array) $this->deferred[$key];
157-
$ok = $this->doSave(array($item[CacheItem::CAST_PREFIX.'key'] => $item[CacheItem::CAST_PREFIX.'value']), $item[CacheItem::CAST_PREFIX.'lifetime']);
158-
unset($this->deferred[$key]);
159-
160-
if (true === $ok || array() === $ok) {
161-
return true;
170+
try {
171+
$e = null;
172+
$value = $item[CacheItem::CAST_PREFIX.'value'];
173+
$ok = $this->doSave(array($key => $value), $item[CacheItem::CAST_PREFIX.'lifetime']);
174+
unset($this->deferred[$key]);
175+
176+
if (true === $ok || array() === $ok) {
177+
return true;
178+
}
179+
} catch (\Exception $e) {
162180
}
181+
$type = is_object($value) ? get_class($value) : gettype($value);
182+
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
163183
}
164184

165-
return $this->doHave($id);
185+
try {
186+
return $this->doHave($id);
187+
} catch (\Exception $e) {
188+
CacheItem::log($this->logger, 'Failed to check if key "{key}" is cached', array('key' => $key, 'exception' => $e));
189+
190+
return false;
191+
}
166192
}
167193

168194
/**
@@ -172,7 +198,13 @@ public function clear()
172198
{
173199
$this->deferred = array();
174200

175-
return $this->doClear($this->namespace);
201+
try {
202+
return $this->doClear($this->namespace);
203+
} catch (\Exception $e) {
204+
CacheItem::log($this->logger, 'Failed to clear the cache', array('exception' => $e));
205+
206+
return false;
207+
}
176208
}
177209

178210
/**
@@ -195,7 +227,29 @@ public function deleteItems(array $keys)
195227
unset($this->deferred[$key]);
196228
}
197229

198-
return $this->doDelete($ids);
230+
try {
231+
if ($this->doDelete($ids)) {
232+
return true;
233+
}
234+
} catch (\Exception $e) {
235+
}
236+
237+
$ok = true;
238+
239+
// When bulk-save failed, retry each item individually
240+
foreach ($ids as $key => $id) {
241+
try {
242+
$e = null;
243+
if ($this->doDelete(array($id))) {
244+
continue;
245+
}
246+
} catch (\Exception $e) {
247+
}
248+
CacheItem::log($this->logger, 'Failed to delete key "{key}"', array('key' => $key, 'exception' => $e));
249+
$ok = false;
250+
}
251+
252+
return $ok;
199253
}
200254

201255
/**
@@ -225,9 +279,9 @@ public function saveDeferred(CacheItemInterface $item)
225279
try {
226280
$item = clone $item;
227281
} catch (\Exception $e) {
228-
@trigger_error($e->__toString());
229-
230-
return false;
282+
$value = $item->get();
283+
$type = is_object($value) ? get_class($value) : gettype($value);
284+
CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
231285
}
232286
$this->deferred[$item->getKey()] = $item;
233287

@@ -243,8 +297,12 @@ public function commit()
243297
$ko = array();
244298

245299
foreach ($f($this->deferred, $this->namespace) as $lifetime => $values) {
246-
if (true === $ok = $this->doSave($values, $lifetime)) {
247-
continue;
300+
try {
301+
if (true === $ok = $this->doSave($values, $lifetime)) {
302+
continue;
303+
}
304+
} catch (\Exception $e) {
305+
$ok = false;
248306
}
249307
if (false === $ok) {
250308
$ok = array_keys($values);
@@ -260,11 +318,15 @@ public function commit()
260318
// When bulk-save failed, retry each item individually
261319
foreach ($ko as $lifetime => $values) {
262320
foreach ($values as $v) {
263-
if (!in_array($this->doSave($v, $lifetime), array(true, array()), true)) {
321+
try {
322+
$e = $this->doSave($v, $lifetime);
323+
} catch (\Exception $e) {
324+
}
325+
if (true !== $e && array() !== $e) {
264326
$ok = false;
265-
266327
foreach ($v as $key => $value) {
267-
@trigger_error(sprintf('Failed to cache key "%s" of type "%s"', is_object($value) ? get_class($value) : gettype($value)));
328+
$type = is_object($value) ? get_class($value) : gettype($value);
329+
CacheItem::log($this->logger, 'Failed to save key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e instanceof \Exception ? $e : null));
268330
}
269331
}
270332
}

‎src/Symfony/Component/Cache/Adapter/ArrayAdapter.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,18 @@
1313

1414
use Psr\Cache\CacheItemInterface;
1515
use Psr\Cache\CacheItemPoolInterface;
16+
use Psr\Log\LoggerAwareInterface;
17+
use Psr\Log\LoggerAwareTrait;
1618
use Symfony\Component\Cache\CacheItem;
1719
use Symfony\Component\Cache\Exception\InvalidArgumentException;
1820

1921
/**
2022
* @author Nicolas Grekas <p@tchwork.com>
2123
*/
22-
class ArrayAdapter implements CacheItemPoolInterface
24+
class ArrayAdapter implements CacheItemPoolInterface, LoggerAwareInterface
2325
{
26+
use LoggerAwareTrait;
27+
2428
private $values = array();
2529
private $expiries = array();
2630
private $createCacheItem;
@@ -125,11 +129,9 @@ public function save(CacheItemInterface $item)
125129
if (is_object($value)) {
126130
try {
127131
$value = clone $value;
128-
} catch (\Error $e) {
129132
} catch (\Exception $e) {
130-
}
131-
if (isset($e)) {
132-
@trigger_error($e->__toString());
133+
$type = is_object($value) ? get_class($value) : gettype($value);
134+
CacheItem::log($this->logger, 'Failed to clone key "{key}" ({type})', array('key' => $key, 'type' => $type, 'exception' => $e));
133135

134136
return false;
135137
}

‎src/Symfony/Component/Cache/CacheItem.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Cache;
1313

1414
use Psr\Cache\CacheItemInterface;
15+
use Psr\Log\LoggerInterface;
1516
use Symfony\Component\Cache\Exception\InvalidArgumentException;
1617

1718
/**
@@ -105,4 +106,24 @@ public function expiresAfter($time)
105106

106107
return $this;
107108
}
109+
110+
/**
111+
* Internal logging helper.
112+
*
113+
* @internal
114+
*/
115+
public function log(LoggerInterface $logger = null, $message, $context = array())
116+
{
117+
if ($logger) {
118+
$logger->warning($message, $context);
119+
} else {
120+
$replace = array();
121+
foreach ($context as $k => $v) {
122+
if (is_scalar($v)) {
123+
$replace['{'.$k.'}'] = $v;
124+
}
125+
}
126+
@trigger_error(strtr($message, $replace), E_USER_WARNING);
127+
}
128+
}
108129
}

‎src/Symfony/Component/Cache/composer.json‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,16 @@
2020
},
2121
"require": {
2222
"php": ">=5.5.9",
23-
"psr/cache": "~1.0"
23+
"psr/cache": "~1.0",
24+
"psr/log": "~1.0"
2425
},
2526
"require-dev": {
2627
"cache/integration-tests": "dev-master",
2728
"doctrine/cache": "~1.6"
2829
},
30+
"suggest": {
31+
"symfony/polyfill-apcu": "For using ApcuAdapter on HHVM"
32+
},
2933
"autoload": {
3034
"psr-4": { "Symfony\\Component\\Cache\\": "" },
3135
"exclude-from-classmap": [

0 commit comments

Comments
 (0)