02 Invalidate Cache v5 Upgrade#2
Conversation
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
… tests, CI - Bump harper ^5.0.11 -> ^5.0.28 and @harperfast/integration-testing ^0.3.1 -> ^0.4.0 - Apply the documented harness escape hatch (harperBinPath resolved from harper's exported main entry, since its `exports` map only exposes ".") - Standardize the test script to harper-integration-test-run (drop the local-only --isolation=none / install-script env workaround; CI runs on Linux loopback) - Add cache-contract assertions: a real 304 conditional hit, and that POST invalidate evicts the entry (this.invalidate(target), the v5 eviction contract) and forces a re-fetch - Regenerate a clean lockfile so npm ci passes across Node 22/24/26 (ws native optional deps present) - Add pinned-hash Integration Tests CI workflow at repo root - Branding: package.json author "HarperDB Inc." -> "Harper Inc." Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Harper derives the ETag/Last-Modified validator for a sourced cache table from the cached record's stored version (server/REST.js: the `isFinite(lastModification)` branch). The source `get()` returns a plain JSON object (no `status`/`headers`), so Harper manages the cached response and emits its own validators — but only on a cache HIT. The first read of an id is a cache MISS resolved with version 0 (the source never sets `sourceContext.lastModified`), and the record is committed asynchronously, so the priming response carries no validator. The test was asserting the ETag on that miss response, which legitimately has none. Add a `fetchUntilCached` helper that polls the read until Harper serves it from cache with a validator, then assert the real 304 conditional hit and the invalidate()-forces-refetch behavior off that validator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
BboyAkers
left a comment
There was a problem hiding this comment.
Code review: 4 findings (1 correctness bug in source, 3 test quality issues)
| const res1 = await fetch(`${httpURL}/JokeCache/2`, { | ||
| headers: { Authorization: auth }, | ||
| }); | ||
| const body1 = await res1.json() as Record<string, unknown>; |
There was a problem hiding this comment.
Test gap: res1.status never checked — false green on first-fetch failure
body1 is populated from res1.json() without first asserting res1.status === 200. If the joke API is unreachable or returns an error on the first fetch, body1.setup is undefined. The second fetch may also return an error, making body2.setup also undefined. strictEqual(undefined, undefined) passes — the test greens out silently with no joke actually cached.
Add a status guard before consuming the first body:
| const body1 = await res1.json() as Record<string, unknown>; | |
| strictEqual(res1.status, 200, `first fetch should succeed, got ${res1.status}`); | |
| const body1 = await res1.json() as Record<string, unknown>; |
| @@ -1,3 +1,5 @@ | |||
| import { Resource, tables } from 'harper'; | |||
|
|
|||
There was a problem hiding this comment.
Bug (line 17): this.invalidate(target) not awaited
The invalidate() method returns void | Promise<void> (Harper ResourceInterface.d.ts). Its implementation calls when(allowed, callback) — if the permission check (target.checkPermission) is async, invalidate() returns a Promise that is silently dropped here. The HTTP 200 response is sent before cache eviction commits, so a client that re-fetches immediately with the stale ETag can still receive a 304.
On line 17, change:
this.invalidate(target);to:
await this.invalidate(target);(The enclosing static async post already handles async correctly — only the await on invalidate is missing.)
| import { Resource, tables } from 'harper'; | ||
|
|
||
| class JokeAPI extends Resource { | ||
| async get() { |
There was a problem hiding this comment.
Bug (line 7): HTTP error responses from the joke API are cached as valid jokes
response.json() is called unconditionally regardless of response.status. If the upstream joke API returns a 404 or 500 (ID out of range, rate-limited, etc.), the error payload (e.g. {"message": "Id not found"}) is stored in the JokeCache table and served to clients for the full 60-second TTL. Subsequent reads return the cached error — the record has no setup or punchline fields.
Fix: throw on non-OK responses so Harper does not cache the failed fetch:
async get() {
const id = this.getId();
const response = await fetch(`https://official-joke-api.appspot.com/jokes/${id}`);
if (!response.ok) {
throw new Error(`Joke API error: ${response.status}`);
}
return response.json();
}| last = res; | ||
| await new Promise((r) => setTimeout(r, 50)); | ||
| } | ||
| return { res: last!, etag: null as string | null, lastModified: null as string | null }; |
There was a problem hiding this comment.
Test gap: fetchUntilCached exhaustion is silent — weak assertion in the else branch
If all 20 polls (1 second total) exhaust without Harper returning a validator, the function returns { res: last!, etag: null, lastModified: null }. In the 304 test (line 112), ok(etag || lastModified, ...) then fails with a generic assertion error rather than a timeout-style message, making CI failures hard to diagnose.
More critically, in the invalidation test the else branch (line 154) only checks that the re-fetch returns status 200 — it skips the entire point of the test (that the prior ETag no longer produces a 304). A slow CI runner that never gets a validator silently passes without exercising the invalidation path at all.
Consider throwing a descriptive error when the loop exhausts:
throw new Error(`fetchUntilCached: no validator after 20 attempts for id=${id} (last status: ${last?.status})`);Or increase the attempt limit and delay for CI environments.
| const res1 = await fetch(`${httpURL}/JokeCache/2`, { | ||
| headers: { Authorization: auth }, | ||
| }); | ||
| const body1 = await res1.json() as Record<string, unknown>; |
There was a problem hiding this comment.
Test gap: res1.status never checked — false green when the first fetch fails
body1 is populated from res1.json() at line 70 without first asserting res1.status === 200. If the joke API is unreachable or returns an error body on the first fetch, body1.setup is undefined. If the second fetch also fails, body2.setup is also undefined, and strictEqual(undefined, undefined) passes — the cache-consistency assertion greens out without any joke ever being cached.
Add a status check before consuming the body:
| const body1 = await res1.json() as Record<string, unknown>; | |
| strictEqual(res1.status, 200, `first fetch for id=2 should succeed, got ${res1.status}`); | |
| const body1 = await res1.json() as Record<string, unknown>; |
…sert res1.status, throw on fetchUntilCached timeout Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Review follow-up (autonomous agent): Fixed all 4 blocking findings: |
Summary
Completes the v5 upgrade for the 02 Invalidate Cache stage of this progressive caching guide. This repo is structured as a multi-branch guide (each numbered branch is a stage), and the upgrade is split across two PRs that mirror that structure:
01-cached-api-v5-upgrade→01-cached-api: upgrades the basic read-through cache stage (addsimport { Resource, tables } from 'harper', apackage.jsonwithharper@^5, and a lockfile). No tests.02-invalidate-example-v5-upgrade→02-invalidate-example: the furthest-along stage (full app + invalidate handler + tests). This is the canonical target where the integration test, harness fix, and CI live.This PR was not force-pushed over; it extends the existing branch. PR #1 is left intact.
Changes in this PR
harper^5.0.11→^5.0.28;@harperfast/integration-testing^0.3.1→^0.4.0.import { Resource, tables } from 'harper'), already present on the stage branch. Cache eviction already uses the v5 contractthis.invalidate(target)(notdelete(), which would throw on a sourced table because the source implements nodelete()). The sourceget()reads onlythis.getId()and does not touchthis.request, so it is not affected by the v5 source-context change.harperBinPathfrom harper's exported main entry, because harper'sexportsmap only exposes".", sorequire.resolve('harper/dist/bin/harper.js')throwsERR_PACKAGE_PATH_NOT_EXPORTED. Replaced the prior local-only--isolation=none+HARPER_INTEGRATION_TEST_INSTALL_SCRIPTworkaround with the standardharper-integration-test-runscript.integrationTests/joke-cache.test.tsasserts the cache contract — a real 304 conditional hit and invalidation → re-fetch (POST{action:"invalidate"}evicts the entry so the prior validator no longer yields a 304).Last-Modifiedvalidator from the cached record's stored version (server/REST.js, theisFinite(lastModification)branch). Because the sourceget()returns a plain JSON object (nostatus/headers), Harper manages the cached response and emits its own validators — but only on a cache hit. The first read of an id resolves with version0(the source never setssourceContext.lastModified) and the record is committed asynchronously, so the priming response carries no validator. Fix: afetchUntilCachedhelper polls the read until Harper serves it from cache with a validator, then the test asserts the 304 and the invalidate-forces-refetch behavior off that validator. The resource shape was already correct; this was a test-timing bug, not an app bug.npm cipasses across the Node[22, 24, 26]matrix (wsnative optional depsbufferutil/utf-8-validate/node-gyp-buildare present; nofile:/.tgzrefs)..github/workflows/integration-tests.ymlat the repo root (Node 22/24/26, actions pinned to commit hashes).package.jsonauthorHarperDB Inc.→Harper Inc.. Livedocs.harperdb.ioURLs left unchanged.Migration items N/A for this repo
Frozen records,
Table.get()return shape, transactions/context, child-process spawning,blob.save(), install scripts, and VM module-loader options — none of these are exercised by this small read-through cache example.Test results
EADDRNOTAVAILon 127.0.0.2+); this is environmental (macOS loopback aliases not configured), not a code issue. The harness reached the bind stage, confirming theharperBinPathfix resolves the CLI correctly.ubuntu-latest, which supports the full 127.0.0.0/8 range.Known issues / flags
harperBinPathescape hatch is unnecessary.kris/edge-api-cache-template→main) is a human-authored rework that turns this repo into a staged "Edge API Cache → Data Layer" template and explicitly asks whether it should supersede this joke example or live in its own repo. That is an unresolved product/scope decision, independent of this v5 upgrade.🤖 Generated with Claude Code