fix: negative-cache property/Connect JSON fetch failures - #407
fix: negative-cache property/Connect JSON fetch failures#407JakeSCahill wants to merge 2 commits into
Conversation
The property-tooltip script caches successful JSON fetches in localStorage for 24 hours but never caches failures, so when the referenced attachment does not exist (version drift between release tags and generated JSON), every page view re-requests a URL that is guaranteed to 404 (~17k requests/day). The Bloblang script has the same problem, plus a hardcoded fallback-version chain that multiplies the misses. Property tooltips now store a failure marker (1 hour TTL, versioned by latest-redpanda-tag) and resolve to an empty lookup while it is fresh. The Bloblang loader tracks per-URL failures for 1 hour and skips URLs that recently returned an error response. Preview mode is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Deploy Preview for docs-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes add localStorage-backed failure caching for Connect JSON and property JSON fetches. Connect requests skip URLs recorded as recently failed and record non-OK responses outside preview mode. Property data cache entries now distinguish failed fetches from successful results, using a one-hour failure TTL instead of the 24-hour success TTL, and failed fetches store version and timestamp metadata. Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant tryFetchConnectJSON
participant localStorage
participant ConnectJSONEndpoint
tryFetchConnectJSON->>localStorage: Read recent URL failures
localStorage-->>tryFetchConnectJSON: Return failure status
tryFetchConnectJSON->>ConnectJSONEndpoint: Fetch URL when not recently failed
ConnectJSONEndpoint-->>tryFetchConnectJSON: Return non-OK response
tryFetchConnectJSON->>localStorage: Record URL failure
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Right idea — an uncacheable failure retried on every page view is the actual mechanism behind the 404 storm, and negative caching is the correct defense-in-depth alongside redpanda-data/docs-extensions-and-macros#224. The Bloblang half looks good. The property-tooltips half has a problem worth fixing before merge. Fix before mergeThe failure marker writes to the same key as the successful cache, and it's written for far more than 404s. localStorage.setItem(
CACHE_KEY,
JSON.stringify({ version: cacheVersion, timestamp: Date.now(), failed: true })
)Two things compound here:
Together that means one transient blip does this: valid 24h cache destroyed, and for the next hour every page sharing that This also doesn't match the PR body, which says "Only deterministic HTTP errors are marked — transient network errors still retry." That's accurate for Two changes fix it:
SuggestionBloblang marks on any if (!response.ok) {
if (!isPreviewMode()) markFetchFailure(url)
return null
}Less severe than above — the failure is scoped to one URL and can't evict anything else — but a transient CDN 5xx still disables Bloblang tooltips for an hour. Same narrowing to 404/410 applies. Verified
Minor noteThe Bloblang fallback URL is built as |
Review findings: the failure marker shared CACHE_KEY with successful data and was written from the generic catch, so one transient blip (5xx, offline, parse error) wiped a valid 24h cache and left tooltips dead for an hour for that user. - The marker now lives under its own key (redpanda-properties-missing) and can never overwrite cached data. Valid data is preferred on read. - It is written only for HTTP 404/410 (the resource does not exist for this version); transient failures are not cached and simply retry on the next page view. - A successful load clears the marker.
|
Fixed per the review: the missing-resource marker now lives under its own key ( |
2 participants
Problem
Two of the top 404 sources on docs.redpanda.com come from tooltip data fetches that retry a missing file on every page view:
19-property-tooltips.jscaches successful properties-JSON fetches for 24h in localStorage, but failures are never cached — when the referenced attachment doesn't exist (release tag drifted ahead of the generated JSON), every streaming page view fires a guaranteed 404 (~17.6k/day forredpanda-properties-v26.1.14.json).16-bloblang-interactive.jsbuilds the Connect JSON URL fromlatest-connect-versionand, on failure, walks a hardcoded fallback-version list — up to 6 404s per page view (~6.4k/day forconnect-4.102.0.json), with no caching of failures.Fix
failedmarker in the existing localStorage cache entry (1h TTL vs 24h for successes, so a fix deploy is picked up quickly; still versioned bylatest-redpanda-tag). While fresh, resolve to an empty lookup without fetching.This is defense-in-depth for the storm; the root cause (meta tags referencing JSON that was never generated) is fixed at build time by redpanda-data/docs-extensions-and-macros#224.
Testing
node --checkandnpx eslinton both files (only pre-existing max-len warning remains)🤖 Generated with Claude Code