Skip to content

02 Invalidate Cache v5 Upgrade#2

Open
BboyAkers wants to merge 6 commits into
02-invalidate-examplefrom
02-invalidate-example-v5-upgrade
Open

02 Invalidate Cache v5 Upgrade#2
BboyAkers wants to merge 6 commits into
02-invalidate-examplefrom
02-invalidate-example-v5-upgrade

Conversation

@BboyAkers

@BboyAkers BboyAkers commented May 8, 2026

Copy link
Copy Markdown
Member

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:

  • PR 01 Cached API v5 Upgrade #101-cached-api-v5-upgrade01-cached-api: upgrades the basic read-through cache stage (adds import { Resource, tables } from 'harper', a package.json with harper@^5, and a lockfile). No tests.
  • PR 02 Invalidate Cache v5 Upgrade #2 (this PR) — 02-invalidate-example-v5-upgrade02-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

  • Dependency bump: harper ^5.0.11^5.0.28; @harperfast/integration-testing ^0.3.1^0.4.0.
  • v5 migration: the only app-level change is the import swap (import { Resource, tables } from 'harper'), already present on the stage branch. Cache eviction already uses the v5 contract this.invalidate(target) (not delete(), which would throw on a sourced table because the source implements no delete()). The source get() reads only this.getId() and does not touch this.request, so it is not affected by the v5 source-context change.
  • Harness fix (documented escape hatch): resolve harperBinPath from harper's exported main entry, because harper's exports map only exposes ".", so require.resolve('harper/dist/bin/harper.js') throws ERR_PACKAGE_PATH_NOT_EXPORTED. Replaced the prior local-only --isolation=none + HARPER_INTEGRATION_TEST_INSTALL_SCRIPT workaround with the standard harper-integration-test-run script.
  • Tests: integrationTests/joke-cache.test.ts asserts 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).
  • 304 test fix (this iteration): the conditional-request test was reading the ETag from the priming request, which is a cache miss. Harper derives the ETag/Last-Modified validator from the cached record's stored version (server/REST.js, the isFinite(lastModification) branch). Because the source get() returns a plain JSON object (no status/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 version 0 (the source never sets sourceContext.lastModified) and the record is committed asynchronously, so the priming response carries no validator. Fix: a fetchUntilCached helper 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.
  • Lockfile: regenerated clean so npm ci passes across the Node [22, 24, 26] matrix (ws native optional deps bufferutil/utf-8-validate/node-gyp-build are present; no file:/.tgz refs).
  • CI: added .github/workflows/integration-tests.yml at the repo root (Node 22/24/26, actions pinned to commit hashes).
  • Branding: package.json author HarperDB Inc.Harper Inc.. Live docs.harperdb.io URLs 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

  • Local: blocked at the macOS loopback bind (EADDRNOTAVAIL on 127.0.0.2+); this is environmental (macOS loopback aliases not configured), not a code issue. The harness reached the bind stage, confirming the harperBinPath fix resolves the CLI correctly.
  • CI: validated by the new GitHub Actions workflow on ubuntu-latest, which supports the full 127.0.0.0/8 range.

Known issues / flags

  • Upstream: harper should export its bin path (or the harness should resolve via the package root) so the harperBinPath escape hatch is unnecessary.
  • Separate scope decision (not addressed here): PR Edge API Cache → Data Layer template #3 (kris/edge-api-cache-templatemain) 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

@BboyAkers BboyAkers changed the title added package.json updated resources.js Harper v5 Upgrade Changes May 8, 2026
@BboyAkers BboyAkers changed the title Harper v5 Upgrade Changes 01 Invalidate Cache v5 Upgrade May 8, 2026
@BboyAkers BboyAkers changed the title 01 Invalidate Cache v5 Upgrade 02 Invalidate Cache v5 Upgrade May 8, 2026
BboyAkers and others added 4 commits May 18, 2026 16:21
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 BboyAkers left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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>;

Comment thread resources.js
@@ -1,3 +1,5 @@
import { Resource, tables } from 'harper';

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Comment thread resources.js
import { Resource, tables } from 'harper';

class JokeAPI extends Resource {
async get() {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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();
}

Comment thread integrationTests/joke-cache.test.ts Outdated
last = res;
await new Promise((r) => setTimeout(r, 50));
}
return { res: last!, etag: null as string | null, lastModified: null as string | null };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Suggested change
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>
@BboyAkers

Copy link
Copy Markdown
Member Author

Review follow-up (autonomous agent): Fixed all 4 blocking findings: await this.invalidate(target) to prevent 304 races; response.ok guard in JokeAPI.get() to prevent caching error payloads; strictEqual(res1.status, 200) assertion before consuming body1; fetchUntilCached now throws on timeout (60-poll budget) instead of silently returning null validators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant