Cache HTTP responses based on RFC 9111 cacheable status codes

[kriszyp]

Replaces the blanket >=300 error throw with a proper cacheable-status check so that responses like 404, 405, and 410 are stored in the cache (with their status code preserved in the record) rather than being propagated as uncached errors. Status 200 is omitted from the stored record since it is the implied default. Non-cacheable codes (e.g. 500, 302) continue to propagate to the client without being cached.

Adds tests for cacheable 404, non-cacheable 500, and 200 behaviour.

Addresses #153

[claude]

Nit: The test name and comment claim to verify that status 200 is not stored in the cached record, but the assertions only check client-visible behaviour (response.status === 200, x-custom-header value). Those assertions would pass identically whether status 200 is stored or omitted. The internal-representation invariant isn't observable from HTTP responses alone. The test is still useful as a regression guard for 200 responses, but the description slightly over-sells what it proves.

[claude]

Review: PR #407 — Cache HTTP responses based on RFC 9111 cacheable status codes

No blockers found.

Surfaces traced:

• CACHEABLE_STATUS_CODES set (Table.ts:94): matches RFC 9111 Section 3.3's heuristically-cacheable list — [200, 203, 204, 206, 300, 301, 308, 404, 405, 410, 414, 501]. 308 is heuristically cacheable per RFC 9110 §15.4.9, so its inclusion is correct.
• 304 revalidation path preserved unchanged; only the >= 300 bucket is restructured.
• Status propagation for cached non-200 records: status !== 200 → updatedRecord.status = status stores the code. On the serve path (REST.ts:162–190), responseData.headers is truthy for the cached plain-object, so it enters the response-with-headers branch; responseData.status ??= ... is a no-op since status: 404 is already set. Clients receive the correct non-200 code from cache.
• stale-if-error: 500 still throws ServerError(statusCode: 500), so the existing stale-if-error catch block (Table.ts:4099–4103) is unaffected.
• Test coverage: 404-cached, 500-not-cached, and 200-still-works cases are all exercised. The server-timing: miss assertion correctly distinguishes cache hits from misses.

One nit posted inline on the 200 test — the description overstates what the assertions verify; see thread.

[claude]

The test claims to verify non-caching but only asserts the status code, which would be 500 whether the response was cached or not.

A previous version of this test (commit 5b3fdbdfc) had assert(response2.headers['server-timing'].includes('miss')) here, but it was removed because 500 responses apparently don't carry a Server-Timing header. That leaves the core claim — that the source is called again rather than served from cache — completely unverified.

The 404 test uses server-timing to confirm a cache hit; this test should use an equivalent mechanism to confirm a cache miss. One reliable approach is a call counter on the fixture source function:

it('does not cache a non-cacheable 500 response', async () => {
    let callCount = 0;
    const origSource = tables.CacheOfHttp._source; // or however the source is accessible
    // wrap / intercept to count calls, then restore in after()

    const response = await axios.get('http://localhost:9926/CacheOfHttp/server-error', { validateStatus: () => true });
    assert.equal(response.status, 500);
    const response2 = await axios.get('http://localhost:9926/CacheOfHttp/server-error', { validateStatus: () => true });
    assert.equal(response2.status, 500);
    assert.equal(callCount, 2, 'source should be called for every request — 500 must not be cached');
});

Without a check like this, a regression that cached 500 responses would silently pass this test.

[claude]

The set correctly mirrors the RFC 9111 §4.2.2 heuristically-cacheable list, but this is a silent breaking change for source functions that currently return Response objects with 2xx codes not in the set — specifically 201, 202, 205, 207, 208, 226.

Before this PR, every status < 300 fell through to the caching branch. After it, any 2xx code absent from CACHEABLE_STATUS_CODES (201, 202, 205, 207, …) hits the !CACHEABLE_STATUS_CODES.has(status) branch and throws ServerError. A source returning 201 Created would now propagate an error to the client instead of caching the response.

RFC 9111 is the right authority here, but the change is undocumented in the PR body and could break integrators. Worth a note in the PR description (or a CHANGELOG entry) so it's visible in release notes.

[claude]

Review — PR #407: Cache HTTP responses based on RFC 9111 cacheable status codes

What I traced

• resources/Table.ts (cache resolution path): The new CACHEABLE_STATUS_CODES set exactly matches RFC 9111 §4.2.2's heuristically-cacheable list. The restructured branch correctly handles 304 revalidation first, then caches the listed codes, and propagates everything else as a ServerError. The status !== 200 optimisation (omitting 200 from the stored record) is consistent with the pre-existing REST.ts logic at line 189 (responseData.status ??= status ?? 200), which fills 200 as the implied default when serving cached records — so the stored 404 status will round-trip correctly.
• server/REST.ts (response serving): Pre-existing code at lines 162–190 already reads responseData.status and propagates it to the HTTP reply. No changes needed there; the mechanism works with the new stored status field.
• Test fixture (testApp/resources.js): The two new cases (not-found → 404, server-error → 500) are minimal and correct.

Findings

---

1. Test for non-caching of 500 doesn't verify the core claim (blocker)

File: unitTests/apiTests/basicREST-test.mjs:459–468

What: The test 'does not cache a non-cacheable 500 response' only asserts response2.status === 500. That assertion is true whether the response was cached or served fresh from source. An earlier version of this test (commit 5b3fdbdfc) had assert(response2.headers['server-timing'].includes('miss')), but it was removed because 500 responses apparently don't carry a Server-Timing header. This leaves the "non-caching" property completely unverified — a regression that cached 500s would pass silently.

Why it matters: The 404 test correctly uses server-timing to confirm a cache hit. Symmetrically, the 500 test must confirm a cache miss. Without it, the test gives false confidence in the non-caching branch.

Suggested fix: Add a call counter to the test fixture's source function (increment inside case 'server-error') and assert it equals 2 after two requests. See the inline comment for a sketch. (Alternatively, returning Server-Timing on error responses would restore the original assertion.)

---

2. Silent breaking change for non-RFC 2xx codes (non-blocker, worth documenting)

File: resources/Table.ts:94

What: Before this PR, every status < 300 was cached. After it, 2xx codes absent from CACHEABLE_STATUS_CODES — specifically 201, 202, 205, 207, 208, 226 — now throw ServerError instead of being cached. RFC 9111 is the right authority for the choice, but the change is undocumented.

Why it matters: A source function returning 201 Created (not unusual for POST-backed sources) would now propagate an error rather than caching. This belongs in the PR description or a CHANGELOG entry so it surfaces in release notes.

---

No other blockers found. The core logic change is correct and the RFC 9111 set is accurate.