Repository navigation
[2.x] fix: Build the text formatter during cache:clear, not on the next request - #4990
Merged
Merged
Conversation
…quest Clearing the cache deletes the compiled formatter, so the next request that renders a post rebuilds it. On a forum with many extensions that compile is large, and it runs inside whichever web request happens to trigger it - where the memory limit is often tighter than on the CLI. If it exceeds that limit the request dies on a PHP memory fatal, which is not catchable and not logged: the visitor gets a blank 500 and nothing explains why. Rebuild the formatter at the end of cache:clear instead, where the limit is usually generous, so the render path finds it already cached. It is best-effort - if the warm build fails the clear still succeeds and the formatter rebuilds lazily as before, so this only ever removes a failure mode, never adds one.
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
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.
Changes proposed in this pull request:
cache:cleardeletes the compiled text formatter, so the next request that renders a post rebuilds it.Formatter::getComponent()compiles the renderer lazily insiderememberForever('flarum.formatter', …), which only runs when the cache is cold — i.e. inside whichever request first renders after the clear.On a forum with many extensions that compiled renderer is large (a real report: ~174 KB, with the cold build peaking over a 256 MB web
memory_limitand taking ~30s). When it runs in a web request whose SAPI memory limit is tighter than the CLI's, it can hit a PHP memory fatal — which is not an exception (Flarum's error handler never sees it) and not a kernel OOM (nothing indmesg), so the visitor gets a blank HTTP 500 and nothing is logged. Clearing the cache from the admin panel is the common trigger, and the failure lands on the next visitor, not the admin who cleared it.This rebuilds the formatter at the end of
cache:clear, where the memory limit is usually generous, so the render path finds it already cached:Formatter::warm()compiles and caches the formatter on demand.CacheClearCommandcalls it after clearing, best-effort: wrapped intry/catchso a warm-build failure never fails the clear itself — the cache is already cleared and the render path rebuilds lazily as before. This only ever removes a failure mode, never adds one.Reviewers should focus on:
warm()failing can't fail the clear —cache:clearstill returns success and the formatter still rebuilds lazily.Necessity
cache:clearare core.Confirmed
composer test).cache:clearleaves the formatter cache warm, and that it still succeeds.Required changes: