HarperFast / HarperFast/harper

sourcedFrom: allowStaleWhileRevalidate silently ignored when defined on the source class

Open
#1,468 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## Summary

For a `sourcedFrom` read-through cache, `allowStaleWhileRevalidate` is **silently ignored when it's defined on the source/origin class** — which is the natural/example placement. As a result the cache gives the user **no stale-first serving and no last-good-on-origin-error**, with no error or warning that SWR is inactive.

Confirmed on **main `541a3d33d` (v5.1.11)**, both storage engines.

## Root cause

`ensureLoadedFromSource()` (≈`Table.js:4055`) checks:

```js
resource?.allowStaleWhileRevalidate?.(entry, id)
```

where `resource` is the **table resource instance** (`this` at the call site). So `allowStaleWhileRevalidate` only fires when it lives on the **table** class/prototype. Defining it on the source class passed to `sourcedFrom(...)` — the intuitive placement — is silently skipped, and the cache falls through to a blocking refresh that propagates the origin error.

## Reproduction

```
npm run test:integration -- "integrationTests/qa-scratch/qa223-swr.test.ts"
```

Diagnostic output (both engines):

- SWR on **source** class, normal expiry → `BLOCKING → FRESH` (waited on origin; should be stale-first).
- SWR on **source** class, origin error → `ERROR-500` propagated (should serve last-good stale).
- SWR on the **table** class (correct placement) → `STALE-FIRST` + background refresh (Harper responded ~300ms before the async origin call completed) and last-good-on-error. **So the mechanism works; only the discovery/placement is broken.**

## Secondary (minor, RocksDB)

Even with correct table-class placement, the origin-error case occasionally still 500s: the eviction background task evicts the stale entry within the ~200ms post-TTL window → `entry.value` is null → the SWR check returns false → blocks. An eviction-vs-SWR timing race.

## Impact

Medium. A team wiring a read-through cache for resilience (serve last-good when the origin is down) following the natural placement gets **no** protection and a hard 500 on origin failure, silently.

## Open question / suggested fix

This needs a product call on the intended API surface:
1. **Honor `allowStaleWhileRevalidate` on the source resource** (most intuitive), or
2. **Document** that it must live on the table class, and emit a warning when a `sourcedFrom` cache has no SWR configured.

Either way, also address the RocksDB eviction-vs-SWR race (widen the stale-resident window, or let SWR consult last-good even after eviction).

---
*Surfaced by the QA-explorer campaign (scenario QA-223, root-causing QA-220), re-confirmed on `main` v5.1.11. Severity: medium. Filed for @kris.*

— Claude (Opus 4.8, 1M)

Contributor guide

Open the contributing guide

Research direction

Start at ensureLoadedFromSource() in Table.js around line 4055 and run integrationTests/qa-scratch/qa223-swr.test.ts. Trace how sourcedFrom resolves source and table resources, then inspect the RocksDB eviction path around the stale-entry window. Done means the intended SWR placement is explicit and covered for stale-first, origin-error, both storage engines, and the reported eviction race.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js, typescript
Domain
backend, databases, performance, testing-qa
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.