Name the store when it is the thing that failed - #198
Merged
Conversation
`symfony/cache` adapters never throw at the application: an unreachable store answers a read as
a miss and a write as `false`. So `guard()` catches nothing, no `cache_error` is recorded, and a
Redis restart reads as a run of ordinary cold reads - the one shape an operator cannot act on.
The write side at least said `saved: false`; the read side said nothing at all.
The adapters report those failures to a PSR-3 logger, so the pools are handed the cache log:
`pool_error{key, operation, error, exceptionClass}` carries the backend's own message beside the
miss it caused. Wired where the pools are built (Redis and Memcached), demonstrated against a
real adapter pointed at a closed port, and distinguished from `cache_error` in both guides -
that one is an exception this package caught, with the resource URI in hand.
Found by an application flow that pointed the DSN at a dead port and asked what the log said.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`#[Refresh]` and `#[Purge]` carry a URI. A resource whose entries share one invalidation handle - a corpus tag across every query string that produced an entry - cannot be expressed that way, and a write that does not happen inside a resource method has no interceptor at all. Both cases end at `invalidateTags()`, which is why the log roots a direct call in `manual_invalidate` instead of dropping its events. Written where a reader looks when the cache misbehaves, not in the attribute that already types its argument as a URI.
The closed-port probes connected eagerly under ext-redis (Predis is lazy by default), so the InvalidArgumentException escaped at configure time instead of surfacing as pool_error events at run time - every ubuntu CI job runs with ext-redis loaded and failed at PoolErrorLogTest:46. Pass lazy=1 in the DSN so both backends defer the connection. testTheBackendsOwnMessageIsCarried pinned the Predis exception class (ConnectionException); ext-redis surfaces the adapter's InvalidArgumentException. The contract is that the caught throwable's class is carried, so assert exceptionClass is not 'unknown'. PoolErrorLogger: guard the mixed $level/$key casts PHPStan flagged.
A standalone StorageMemcachedEtagModule install bound its MemcachedAdapter without the PSR-3 logger, so a dead ETag store failed silently on the validator side - the silent-failure class this PR exists to kill.
# Conflicts: # docs/reading-the-log.ja.md # docs/reading-the-log.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #198 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 483 496 +13
===========================================
Files 94 96 +2
Lines 1281 1311 +30
===========================================
+ Hits 1281 1311 +30 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…lters
Windows words a refused connection differently ('actively refused it'), so
pinning the POSIX wording failed the windows jobs. The contract is that the
backend's own message is carried: assert it is non-empty and the exception
class is not 'unknown'.
The same intent covers the two lines codecov flagged: a new test pins that
non-failure levels are ignored and a failure with no operation word is
recorded as operation 'unknown'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
symfony/cacheadapters never throw at the application: an unreachable store answers a read as a miss and a write asfalse. SoResourceStorage::guard()catches nothing, nocache_erroris recorded, and a Redis restart reads as a run of ordinary cold reads.Measured against a real adapter pointed at a closed port, before this change:
The write side says
saved: false. The read side says nothing - and a read is what a request hits first.What this adds
The adapters report backend failures to a PSR-3 logger. The pools are now given the cache log through one:
Distinct from
cache_error, which is an exception this package caught with the resource URI in hand; here only the pool key is known. Both guides say which is which.Wired where the pools are built -
StorageRedisDsnModuleandStorageMemcachedModule, viaInjectionPointsonsetLogger- so an application gets it by installing the module it already installs.Notes
operationis derived from the adapter's wording (fetch→ read,save/write/delete/… → write) and isunknownwhen the wording is not one this package recognises: a mislabelled side is worse than an unlabelled one. The schema does notenumit, for the same reason ascache_policy.expiry- the demo-coverage gate would demand a fabricated demo for a value that exists for future wordings.Demonstrated in
demo/run-degraded.phpsession J against a realRedisTagAwareAdapteronredis://127.0.0.1:1: connection refused, no service to install, no timeout.Stacked on #196 (which is stacked on #191). Merge those first.