Skip to content

Add a fetch governor: dedupe, negative-cache and back off repeated proxy fetches - #79

Merged
jonocodes merged 1 commit into
mainfrom
feature/fetch-governor
Oct 3, 2026
Merged

jonocodes merged 1 commit into
mainfrom
feature/fetch-governor

Conversation

@jonocodes

Copy link
Copy Markdown
Owner

What

Adds a fetch governor in front of every proxied request. All article and image fetches already funnel through fetchWithTimeout (src/utils/article/tools.ts), so the governor lives there and covers everything:

  • In-flight coalescing — concurrent fetches of the same URL share one request.
  • Persisted negative cache — outcomes are classified (permanent 4xx / transient timeout, 5xx, 429) and stored in a new Dexie table (fetchCache, db v6) so a known-bad URL is not retried until its nextAttemptAt passes.
  • Backoff — transient failures retry with exponential backoff + jitter; permanent failures (404/410) wait days.
  • Circuit breaker — an HTTP 429 (e.g. Cloudflare 1027, free-tier cap) opens a breaker and short-circuits all proxied fetches for a cooldown.
  • An explicit Refetch calls clearFetchFailures() so a user-forced retry isn't blocked by a backoff window.

It also closes the reader half of #75: an image that fails during ingest is replaced with a local placeholder (applyImagePlaceholder), with the original kept in data-orig-src, so a saved article never re-requests a dead remote URL.

Why

On 2026-10-02 the shared CORS worker served ~234k invocations against a 10–50/day baseline and hit the Workers Free daily cap. ~10 image URLs returning 404 were each fetched ~24k times over 14h: the upstream 404 sends cache-control: no-cache, and the app had no de-dupe, no negative cache, and no backoff. Ticket: #78.

Stacked PR

Based on feature/proxy-failure-context (#77) because it uses FetchError.requestUrl/status. Retarget to main once #77 merges.

Testing

  • Unit: src/utils/net/fetchGovernor.test.ts (classification, coalescing, permanent/transient, backoff, breaker, clear/clearAll) and lib/__tests__/ingestion-placeholder.test.ts.
  • E2E: tests/e2e/ingest-failed-image.spec.ts against a new fixture with one good and one 404 image, asserting the placeholder + no remote src.
  • npx jest → 327 passed; tsc --noEmit, eslint, and vite build clean.
  • E2E run via flox: flox activate -c "node scripts/run-e2e.js tests/e2e/smoke.spec.ts tests/e2e/ingest-failed-image.spec.ts" → 4 passed. docs/DEVELOPMENT.md documents this.

Closes #78. Refs #75.

All proxied requests now pass through a governor that coalesces concurrent
fetches of the same URL, negative-caches failures in Dexie, backs off
transient errors with exponential jitter, and opens a circuit breaker on
HTTP 429 so a capped CORS proxy (Cloudflare 1027) cannot be hammered.

Images that fail during ingest are replaced with a local placeholder so a
saved article never keeps a remote URL that every read would re-request.

Refs #75
@netlify

netlify Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for savrlist ready!

Name Link
🔨 Latest commit 75dccf9
🔍 Latest deploy log https://app.netlify.com/projects/savrlist/deploys/6ac124f77ce89e00082afebc
😎 Deploy Preview https://deploy-preview-79--savrlist.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@jonocodes
jonocodes changed the base branch from feature/proxy-failure-context to main October 3, 2026 15:57
@jonocodes
jonocodes merged commit eaba6ae into main Oct 3, 2026
6 checks passed
jonocodes added a commit that referenced this pull request Oct 4, 2026
Conflicts:
- ArticleComponent.tsx: #80 moved sanitising into sanitizeArticleHtml
  (video-embed iframe allow-list) but still passed a fresh `{ __html }`
  literal each render, so the DOM-rebuild bug remained on main. Keep #80's
  sanitiser and this branch's memoization: useMemo(() => ({ __html:
  sanitizeArticleHtml(html) }), [html]).
- CHANGELOG.md: keep both; this branch's entries move under #80's
  2026-10-03 heading since they land after it.

With embeds now rendered the rebuild also reloaded every player on each
save: on main's code 3 embeds were loaded 18 times over 3 reading pauses.
Added a scroll-stability test pinning one load per embed (fails on main's
version of the line, passes with the memoization).

Adjusted for #79, which swaps failed images for a local placeholder:
- The failed-download dimensions test now expects the placeholder and
  data-orig-src, and still asserts no width/height is claimed.
- The shared jest mock of ~/utils/article/tools gains a FetchError class.
  ingestion.ts now does `e instanceof FetchError` on a failed image, and
  without it the check throws; no existing test ran a failed download
  through ingestHtml, so the gap was latent.
- Comment and changelog wording: only articles saved before #79 still
  hold remote URLs for failed images.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

Politely refetch: dedupe, negative-cache and back off repeated proxy requests

1 participant