fix: stop caching error responses - #624
Conversation
app:last-modified floors every page's Last-Modified at $config:EDITORIAL_DATE_TIME. That floor is deliberate: it guarantees that any copy a client cached before the editorial date is treated as stale, so raising the constant re-generates every page still being served from a cache. This commit does not change that. The trouble is that the same treatment reaches error responses. view.xql runs for error pages too, so a 404 is sent with Last-Modified set to that constant, and because hsg-shell sets no Cache-Control, Expires or ETag, nothing marks the response uncacheable. Caches fall back to heuristic freshness and store it. From there the 404 is self-confirming: every revalidation compares the client's If-Modified-Since against the same constant and returns 304, so the cached 404 is renewed rather than re-checked. It cannot clear until someone raises the editorial date, and only a force-reload escapes it in the meantime. An error response describes a moment, not the editorial state of the site, so it has no business in a scheme designed to describe content. Errors are raised two ways, and both are handled here. The controller calls local:serve-not-found-page or local:serve-bad-request-page, which now pass an error-page parameter through local:render-page. A publication route instead renders normally and signals failure during templating by setting the hsg-shell.errcode request attribute, which app:handle-error turns into a 4xx; view.xql now checks for that attribute after rendering. In both cases the response is sent with Cache-Control: no-store and takes no part in Last-Modified negotiation. Successful responses are untouched: they keep the floor, keep sending Last-Modified and keep revalidating to 304, so the sitewide invalidation lever works exactly as before. One case is knowingly left. A conditional request for a publication-route 404 still returns 304, because view.xql honors If-Modified-Since and short-circuits before rendering, so it cannot yet know the request will not resolve. That is a question of resolution order rather than of the Last-Modified value. With no-store in place a client will not have cached the 404, so it will not send the conditional request that would produce the stale 304. Not addressed here: the same floor means editing a document and uploading it to a local instance does not move Last-Modified, so a warm cache keeps showing the previous rendering until a force-reload. That is awkward during local preview and is better solved by a development caching mode, separately. Adds tests/bats/cache-headers.bats, covering both error paths and guarding that successful responses still send Last-Modified and still revalidate to 304. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
[This response was prompted by Joe, drafted by Claude Code, and reviewed by Joe.] Flagging a revision to the description above, since the original went out by email. Two corrections. First, the original implied that any cached 404 sticks indefinitely; in fact only pages floored at This is a development problem, not a production one. The description now says so, and adds the precondition — a transient 404 on a page whose content predates the editorial date — along with the evidence for both. The scope of the change itself is unaffected. |
The suite assumed a populated instance. CI installs only hsg-shell and the few libraries it depends on, and without the publication data packages hsg-shell cannot render any page at all: every route answers 400 from app:handle-error's default, so all seven tests failed there rather than reporting anything useful. Skip the suite when the instance is not populated, detected by whether the landing page returns 200. It then runs against a full local instance and stays quiet in CI. Worth noting that CI cannot currently exercise any hsg-shell page for the same reason. The existing smoke tests check the container, the JVM and the logs, none of which touch a rendered page, which is why they pass while every route is answering 400. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
[This PR was prompted by Joe, drafted by Claude Code, and reviewed by Joe.]
Background: the editorial date floor is doing its job
app:last-modifiedresolves a page's timestamp and then floors it at$config:EDITORIAL_DATE_TIME:That floor is deliberate and worth preserving. It guarantees that any copy a client cached before the editorial date is treated as stale, so raising the constant re-generates every page still being served from a cache. For a successful response this is exactly the behavior we want, and this PR does not change it.
The problem: the floor also reaches error responses
view.xqlruns for error pages too. So a 404 is sent withLast-Modifiedset to that same constant, and because hsg-shell sets noCache-Control,Expires, orETag, nothing marks the response uncacheable. Caches fall back to heuristic freshness and store it.From there the 404 is self-confirming. Every revalidation compares the client's
If-Modified-Sinceagainst the constant and gets304 Not Modified, so the cached 404 is renewed rather than re-checked.The part that makes this consequential is that a 304 carries no body. The server cannot tell whether the client cached a 200 or a 404 — it only compares dates. So a client that once cached a 404 keeps re-serving it.
An error response describes a moment, not the editorial state of the site, so it has no business in a scheme designed to describe content.
When this actually bites
Not every 404 sticks. It depends on which timestamp the URL reports:
/Tue, 14 Jul 2026— its own/countries/afghanistanMon, 29 Jun 2026— the editorial floor/historicaldocumentsMon, 29 Jun 2026— the editorial floorA page whose content is newer than the editorial date reports its own timestamp, so a deployment moves it and any cached response — including a cached 404 — is replaced on the next revalidation. Those URLs are self-healing.
A page floored at the editorial date reports a constant. Redeploying
rdcrdoes not change an unmodified article's commit time, and redeployinghsg-shelldoes not touchrdcrcontent at all, so the value never moves and revalidation returns 304 indefinitely. Since most content predates 29 June, that describes most of the site.So the precondition is specific: a transient 404 on a page whose content predates the editorial date.
This is a development problem, not a production one. In production, instances are taken out of the load balancer while packages are installed, so no request is served during the window when a package's URLs would 404, and nothing is there to be cached. A local instance has no such protection, and requests do land during deploys and uploads. While preparing this PR I saw exactly that class of transient:
/exist/apps/hsg-shell/returned 400 and then 200 on retry, and a first request after a restart returned 400 before settling.The fix is still worth having — a cached error that can only be cleared by a force-reload is a genuine defect, and the change is confined to error paths — but it should not be read as fixing a production problem.
Reproduction
No
Cache-Control, and the 404 revalidates as304.The fix
Errors are raised two different ways, and both needed handling:
local:serve-not-found-pageandlocal:serve-bad-request-page, used at 25 call sites. These now pass anerror-pageparameter through the existinglocal:render-pageparameter mechanism.hsg-shell.errcoderequest attribute, whichapp:handle-errorturns into a 4xx. Seven modules do this, and it is the path behindcountries/…andhistoricaldocuments/….view.xqlnow checks for that attribute after rendering.In both cases the response is sent with
Cache-Control: no-storeand takes no part inLast-Modifiednegotiation.Successful responses are untouched. They keep the editorial-date floor, keep sending
Last-Modified, and keep revalidating to304, so the sitewide invalidation lever still works exactly as before. The change removes error responses from that scheme and nothing else.The duplicated
templates:applyblock is factored into alocal:render()function rather than copy-pasted into the new branch.Tests
tests/bats/cache-headers.bats— 7 tests, alongside the existingsmoke-test.batsand using the same harness. It honors$HSG_BASEso it can run against a container or a local instance.Three of the tests exist specifically to guard the behavior described at the top: that successful responses remain cacheable, still send
Last-Modified, and still revalidate to304. If a future change to error handling leaks into the content path, they should catch it.I verified the tests catch the bug by stashing the fix, rebuilding, redeploying, and re-running:
The three new assertions fail without the fix; the four regression guards hold either way.
Knowingly left open
A conditional request for a publication-route 404 still returns
304.view.xqlhonorsIf-Modified-Sinceand short-circuits before rendering, so at that point it cannot yet know the request will not resolve to a document. This is a question of resolution order rather than of theLast-Modifiedvalue, and addressing it would mean determining that a request is an error before the short-circuit — a larger change than this one.In practice it is now unreachable through normal browser behavior: with
no-storethe client never caches the 404, so it never sends the conditional request that would produce the stale304.Related, but not addressed here
The local-preview symptom that originally led here — editing a document, uploading it, and reloading to find the old page — is not fixed by this PR, because that is a successful response rather than an error. It is handled by a development caching mode in a separate PR, which makes our setup directions hold as written.
Verifying locally
After deploying the package, clear eXist's compiled query cache — otherwise the previously compiled controller keeps serving and the change appears to have no effect:
Then: