Skip to content

fix docs#801

Closed
LadyBluenotes wants to merge 4 commits intoTanStack:mainfrom
LadyBluenotes:fix-examples
Closed

fix docs#801
LadyBluenotes wants to merge 4 commits intoTanStack:mainfrom
LadyBluenotes:fix-examples

Conversation

@LadyBluenotes
Copy link
Copy Markdown
Member

@LadyBluenotes LadyBluenotes commented Mar 31, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Improved API error reporting with clearer hints, request/rate‑limit diagnostics, and structured logging on 403.
    • Avoid sending placeholder auth token; only include Authorization when a valid token is present.
    • Gracefully handle remote-content fetch failures for examples by logging a warning and falling back instead of crashing.
  • New Features

    • Serve stale cached content as an optional per-call fallback when remote fetches fail.
    • Adaptive cache-control headers for example routes (shorter for “latest”, longer for fixed versions) and a user-facing “source browser temporarily unavailable” message.

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 31, 2026

Deploy Preview for tanstack ready!

Name Link
🔨 Latest commit 54d3370
🔍 Latest deploy log https://app.netlify.com/projects/tanstack/deploys/69cc0836a4206e0008c0dad7
😎 Deploy Preview https://deploy-preview-801--tanstack.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 30 (🔴 down 14 from production)
Accessibility: 90 (no change from production)
Best Practices: 83 (no change from production)
SEO: 97 (no change from production)
PWA: 70 (no change from production)
View the detailed breakdown and full score reports

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

📝 Walkthrough

Walkthrough

Adds a stale-on-error in-memory cache and exposes staleOnError to fetchCached; changes GitHub fetches to omit placeholder Authorization, enrich non-OK error capture (request/rate-limit headers + truncated body), log 403s, and return stale cache entries when upstream fetches fail.

Changes

Cohort / File(s) Summary
Caching / stale-on-error
src/utils/cache.server.ts
Add global LRU docStaleCache; extend fetchCached with optional staleOnError?: boolean; store stale copy without TTL on success and return stale copy when fetch fails and staleOnError is enabled.
GitHub fetch & error handling
src/utils/documents.server.ts
Make Authorization conditional on a non-placeholder token; set staleOnError: true for certain cached calls; on non-OK responses (except 404→null) read/truncate body, capture x-github-request-id and x-ratelimit-* headers, log structured 403s, and throw Errors containing status + header/body context.
Route headers & loader/queries
src/routes/$libraryId/$version.docs.framework.$framework.examples.$.tsx
Reordered component option, added computed headers (vary/cache-control/cdn-cache-control) depending on “latest” vs pinned version; loader returns githubContentsAvailable and uses fallbackPath; switched some queries from suspense to useQuery gated by githubContentsAvailable, and render now hides CodeExplorer when GitHub contents are unavailable.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Server
  participant FreshCache as "docCache"
  participant StaleCache as "docStaleCache"
  participant GitHub

  Client->>Server: Request document
  Server->>FreshCache: check key
  alt fresh hit
    FreshCache-->>Server: return fresh
    Server-->>Client: document
  else cache miss
    Server->>Server: fetchCached(fn, staleOnError=true)
    Server->>GitHub: fetchApiContentsRemote (conditional Authorization)
    alt GitHub 200
      GitHub-->>Server: 200 + body
      Server->>FreshCache: store with TTL
      Server->>StaleCache: store stale copy (if staleOnError)
      Server-->>Client: document
    else GitHub non-OK
      GitHub-->>Server: non-OK + headers + body
      Server->>Server: capture headers, truncate body, log (403 special)
      alt staleOnError && StaleCache has entry
        StaleCache-->>Server: stale value
        Server-->>Client: stale document (warning)
      else
        Server-->>Client: throw error
      end
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hop through logs and stash a spare,
When GitHub grumbles, stale treats I share.
Tokens checked, headers tucked with care,
If fresh fetch fails, a cached crumb's there.
Nibble on — the docs remain fair. 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix docs' is vague and generic, using non-descriptive language that fails to communicate the specific changes made across three files (caching, error handling, and conditional rendering). Provide a more specific title that highlights the primary change, such as 'Add stale-on-error caching for docs and improve error handling' or 'Implement graceful fallback for unavailable GitHub docs source'.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/utils/documents.server.ts (1)

630-636: Consider extending error handling to recursive directory fetches.

The enhanced error handling added for the initial fetch (lines 561-601) isn't applied to the recursive fetch within buildFileTree. If a subdirectory request hits a 403 or 429, it will silently produce a confusing warning about "Expected an array" rather than providing the detailed rate-limit diagnostics.

This is pre-existing behavior and outside the scope of this PR, but could be addressed in a follow-up if nested fetches frequently fail.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/documents.server.ts` around lines 630 - 636, The recursive
directory fetch inside buildFileTree (the fetch to file._links.self using
fetchOptions that assigns directoryFilesResponse/directoryFiles) lacks the
robust error handling used for the initial fetch; update this block to mirror
that logic: check directoryFilesResponse.ok and if false read response.text() /
json() to include the error body in processLogger.warn/error, detect 403/429 and
surface rate-limit headers (x-ratelimit-limit, x-ratelimit-remaining,
x-ratelimit-reset) in the log, and throw or return a clear error instead of
allowing a misleading "Expected an array" warning so callers get the same
diagnostic detail as the top-level fetch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/utils/documents.server.ts`:
- Around line 630-636: The recursive directory fetch inside buildFileTree (the
fetch to file._links.self using fetchOptions that assigns
directoryFilesResponse/directoryFiles) lacks the robust error handling used for
the initial fetch; update this block to mirror that logic: check
directoryFilesResponse.ok and if false read response.text() / json() to include
the error body in processLogger.warn/error, detect 403/429 and surface
rate-limit headers (x-ratelimit-limit, x-ratelimit-remaining, x-ratelimit-reset)
in the log, and throw or return a clear error instead of allowing a misleading
"Expected an array" warning so callers get the same diagnostic detail as the
top-level fetch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 005c5729-375d-489c-af4a-a8885314464c

📥 Commits

Reviewing files that changed from the base of the PR and between 23bb962 and 3259b34.

📒 Files selected for processing (1)
  • src/utils/documents.server.ts

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/utils/cache.server.ts`:
- Around line 45-50: When serving a stale fallback in fetchCached (i.e. when
opts.staleOnError is true and docStaleCache.has(opts.key)), write that stale
value back into the primary cache (docCache) with a refreshed TTL instead of
only returning it; update docCache.set(opts.key, staleValue) (and any expiration
metadata your cache uses) so subsequent requests hit the cache until the
refreshed TTL expires and don’t immediately re-invoke opts.fn(), preventing
repeated upstream failures. Ensure you reference opts.key, docStaleCache,
docCache, fetchCached, and opts.fn when making this change.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3dc235d6-b464-42c5-8cc8-ed37fed298f5

📥 Commits

Reviewing files that changed from the base of the PR and between 3259b34 and 12dd0e6.

📒 Files selected for processing (2)
  • src/utils/cache.server.ts
  • src/utils/documents.server.ts

Comment on lines +45 to +50
if (opts.staleOnError && docStaleCache.has(opts.key)) {
console.warn(
`[fetchCached] Serving stale value for key '${opts.key}' after fetch error`,
)
return docStaleCache.get(opts.key) as T
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Stale fallback does not suppress repeated upstream failures.

When fallback is used, the stale value is returned but not written back to docCache. After TTL expiry, each request still re-hits opts.fn() and fails again, which can amplify external outages.

Proposed fix
   } catch (error) {
     if (opts.staleOnError && docStaleCache.has(opts.key)) {
+      const staleValue = docStaleCache.get(opts.key) as T
+      // Short rehydrate window to avoid hammering failing upstreams
+      docCache.set(opts.key, staleValue, {
+        ttl: Math.min(opts.ttl, 30_000),
+      })
       console.warn(
         `[fetchCached] Serving stale value for key '${opts.key}' after fetch error`,
       )
-      return docStaleCache.get(opts.key) as T
+      return staleValue
     }

     throw error
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (opts.staleOnError && docStaleCache.has(opts.key)) {
console.warn(
`[fetchCached] Serving stale value for key '${opts.key}' after fetch error`,
)
return docStaleCache.get(opts.key) as T
}
if (opts.staleOnError && docStaleCache.has(opts.key)) {
const staleValue = docStaleCache.get(opts.key) as T
// Short rehydrate window to avoid hammering failing upstreams
docCache.set(opts.key, staleValue, {
ttl: Math.min(opts.ttl, 30_000),
})
console.warn(
`[fetchCached] Serving stale value for key '${opts.key}' after fetch error`,
)
return staleValue
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/cache.server.ts` around lines 45 - 50, When serving a stale
fallback in fetchCached (i.e. when opts.staleOnError is true and
docStaleCache.has(opts.key)), write that stale value back into the primary cache
(docCache) with a refreshed TTL instead of only returning it; update
docCache.set(opts.key, staleValue) (and any expiration metadata your cache uses)
so subsequent requests hit the cache until the refreshed TTL expires and don’t
immediately re-invoke opts.fn(), preventing repeated upstream failures. Ensure
you reference opts.key, docStaleCache, docCache, fetchCached, and opts.fn when
making this change.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/utils/documents.server.ts (2)

546-555: Extract the placeholder sentinel to a shared constant.

The string 'USE_A_REAL_KEY_IN_PRODUCTION' is hardcoded here and in src/utils/env.functions.ts (line 26). If one changes without the other, unauthenticated requests would resume silently—and staleOnError would mask the resulting failures.

♻️ Suggested refactor

Create a shared constant:

// src/utils/constants.ts (or similar)
export const GITHUB_TOKEN_PLACEHOLDER = 'USE_A_REAL_KEY_IN_PRODUCTION'

Then use it in both locations:

+import { GITHUB_TOKEN_PLACEHOLDER } from './constants'
+
 const githubToken = env.GITHUB_AUTH_TOKEN
 const hasConfiguredGitHubToken =
-  Boolean(githubToken) && githubToken !== 'USE_A_REAL_KEY_IN_PRODUCTION'
+  Boolean(githubToken) && githubToken !== GITHUB_TOKEN_PLACEHOLDER
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/documents.server.ts` around lines 546 - 555, Extract the literal
sentinel 'USE_A_REAL_KEY_IN_PRODUCTION' into a shared exported constant (e.g.
GITHUB_TOKEN_PLACEHOLDER) and replace all occurrences with that constant so both
modules use the same source of truth; specifically add the constant to a
new/shared module (e.g. constants.ts) and update the usages in
src/utils/documents.server.ts (symbols: githubToken, hasConfiguredGitHubToken,
fetchOptions) and src/utils/env.functions.ts to import and compare against
GITHUB_TOKEN_PLACEHOLDER instead of the hardcoded string.

290-290: Enabling staleOnError provides resilience but may mask configuration issues.

Per context snippet 1, the stale cache has no TTL and catches all errors—including 401/403 from a misconfigured GITHUB_AUTH_TOKEN. This means broken auth could serve indefinitely stale docs without surfacing the root cause until the 300-item LRU evicts entries.

Consider adding monitoring/alerting on repeated stale-cache hits or auth failures so misconfigurations don't go unnoticed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/documents.server.ts` at line 290, The config sets staleOnError:
true which can mask auth/config issues; update the code that constructs the
cache/fetcher (the place using staleOnError) to emit telemetry and logs whenever
a stale-cache hit occurs or an upstream fetch fails (especially on 401/403
related to GITHUB_AUTH_TOKEN), e.g. increment a metric and log a warning with
the failure status and token-related context, and add a simple alert rule (high
rate of stale hits or repeated 401/403) so operators are notified when stale
responses are being served.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/utils/documents.server.ts`:
- Around line 546-555: Extract the literal sentinel
'USE_A_REAL_KEY_IN_PRODUCTION' into a shared exported constant (e.g.
GITHUB_TOKEN_PLACEHOLDER) and replace all occurrences with that constant so both
modules use the same source of truth; specifically add the constant to a
new/shared module (e.g. constants.ts) and update the usages in
src/utils/documents.server.ts (symbols: githubToken, hasConfiguredGitHubToken,
fetchOptions) and src/utils/env.functions.ts to import and compare against
GITHUB_TOKEN_PLACEHOLDER instead of the hardcoded string.
- Line 290: The config sets staleOnError: true which can mask auth/config
issues; update the code that constructs the cache/fetcher (the place using
staleOnError) to emit telemetry and logs whenever a stale-cache hit occurs or an
upstream fetch fails (especially on 401/403 related to GITHUB_AUTH_TOKEN), e.g.
increment a metric and log a warning with the failure status and token-related
context, and add a simple alert rule (high rate of stale hits or repeated
401/403) so operators are notified when stale responses are being served.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f544aecf-f9ed-4887-9452-87e59a06f4ec

📥 Commits

Reviewing files that changed from the base of the PR and between 12dd0e6 and a6229f4.

📒 Files selected for processing (2)
  • src/routes/$libraryId/$version.docs.framework.$framework.examples.$.tsx
  • src/utils/documents.server.ts

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/routes/$libraryId/$version.docs.framework.$framework.examples.$.tsx (1)

80-80: Minor naming inconsistency: variable name suggests filename but receives full path.

The variable explorerCandidateStartingFileName now receives fallbackPath, which is a full path (e.g., src/main.tsx), not just a filename. The code works correctly since determineStartingFilePath compares against full paths, but the naming could be clearer.

Suggested rename for clarity
-      const explorerCandidateStartingFileName = fallbackPath
+      const explorerCandidateStartingPath = fallbackPath

And update the reference on line 86 accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/`$libraryId/$version.docs.framework.$framework.examples.$.tsx at
line 80, The variable explorerCandidateStartingFileName is misnamed because it
holds a full path (fallbackPath); rename it to explorerCandidateStartingFilePath
and update all references (including the call to determineStartingFilePath) so
names reflect that it contains a path rather than just a filename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/routes/`$libraryId/$version.docs.framework.$framework.examples.$.tsx:
- Line 80: The variable explorerCandidateStartingFileName is misnamed because it
holds a full path (fallbackPath); rename it to explorerCandidateStartingFilePath
and update all references (including the call to determineStartingFilePath) so
names reflect that it contains a path rather than just a filename.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 92051cc9-c295-44bf-a0ac-38180b499b8c

📥 Commits

Reviewing files that changed from the base of the PR and between a6229f4 and 54d3370.

📒 Files selected for processing (1)
  • src/routes/$libraryId/$version.docs.framework.$framework.examples.$.tsx

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