- Issue created by @catch
- π¬π§United Kingdom catch
The actual condition plugin lives in system module.
- π¬π§United Kingdom alexpott πͺπΊπ
How does this work? Doesn't this need to be something we can determine from the request? Which we can't do for the response code isn't this going to end up in the page cache too?
- π¬π§United Kingdom catch
When we get a 404 or 403, HttpExceptionSubscriberBase sets the exception on the request.
By the time we reach the rendering of the 404 or 403 page, this is available, so the cache context will work.
It won't do anything if someone has a response subscriber that adds a 418 or 201 status code or weird things like that, those would all count as 200.
We could rename it to
ExceptionStatusCodeCacheContext
and then when there is no exception status code, return '0' instead of '200'. This would match how the visibility condition works a lot closer. - π¬π§United Kingdom alexpott πͺπΊπ
I've tested this solution with easy breadcrumb as described by π Adding url.path to breadcrumb cache context results in every 404 page getting a different dynamic page cache entry Active and it works fantastically. This is a fantastic change which will improve the cacheability of any block that is using the response status condition.
- π¬π§United Kingdom catch
Just in case anyone lands here due to large cache tables, while this should fix the dynamic page cache, it's very easy to end up with a large internal page cache database too.
One reason is π Page cache creates vast amounts of unneeded cache data Active for query strings.
For 404s/403s the page cache supports much shorter TTLs, which means 404/403 pages can be expired on cron or set to not cache at all. This was added in #2699613: Set a shorter TTL for 404 responses in page_cache module β .
- πΊπΈUnited States smustgrave
Not sure the right status but with the new service won't we need a CR?
Even though a task does this kind of change need test coverage?