[Bugfix][CachedRouting] Make route caching best-effort so boot never fails - #28
Merged
Merged
Conversation
A partially-written or corrupt cache file (e.g. a torn write from concurrent processes booting right after a deploy) made `FileStore::getPayload()` call `unserialize()` on garbage. The emitted warning is promoted to an ErrorException by the framework error handler, so the read threw before the caller could react — surfacing as a 500 during boot when the route cache file was being built concurrently. Wrap the unserialize in try/catch: on failure, forget the entry and return a miss so the caller rebuilds. Adds a CachedRouting test that a corrupt cache file triggers a rebuild rather than an error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The production stack trace showed the real failure is a cache WRITE, not a corrupt read: `file_put_contents(.../app_storage/cache/..): No such file or directory` — the cache directory is not creatable/writable by php-fpm, so `FileStore::put()` throws and the exception propagates out of `Router::cache()` during boot, returning a 500. Route caching is only an optimization, so no cache I/O failure should break boot. Wrap both the read and the write in `Router::cache()`: - read failure (corrupt/unreadable entry) -> treat as a miss and rebuild - write failure (unwritable dir, disk full) -> ignore; routes stay defined in memory, just uncached Reverts the earlier FileStore-level read guard (superseded by the read try/catch here). Adds a test that an unwritable cache path still boots with routes defined, alongside the corrupt-cache rebuild test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agissept
changed the title
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.
Summary
Fixes the deploy-time 500 on
/ready(and theopcache_clearpost-deploy task) that appeared after route cache moved to the local file store (4.2.89). The production stack trace showed the failure is a cache write, not a corrupt read:The cache directory (
app_storage/cache/) is not creatable/writable by the php-fpm user on the nodes, soFileStore::createCacheDirectory()swallows themakeDirectory()failure andfile_put_contents()then throws — and that exception propagates out ofRouter::cache()during boot, surfacing as a 500.Route caching is only an optimization, so no cache I/O failure should ever break boot. This wraps both the read and the write in
Router::cache()so failures degrade gracefully instead of crashing.Changes
testBootStillWorksWhenCacheIsUnwritable(unwritable cache path still boots with routes defined) andtestRebuildsWhenCachedFileIsCorrupt(corrupt entry rebuilds instead of erroring).Context
This makes the app resilient, but the underlying root cause is operational: the
app_storage/cache/directory must exist and be writable by php-fpm on every node (previously nothing wrote there because the route cache lived in Redis). That should be fixed in provisioning/deploy in parallel; with this change a missing/unwritable cache dir degrades to "routes rebuilt per request" instead of a 500.References
🤖 Generated with Claude Code