fix: bound docs manifest fan-out to stop long hangs on redirect-miss docs requests#1026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds a concurrency-limited docs redirect collector, refactors manifest building to use it, switches several docs server helpers to dynamic imports, and adds tests for concurrency and recoverable error handling. ChangesDocs manifest concurrency
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant buildDocsManifest
participant mapWithConcurrency
participant collectRedirectEntriesForFile
participant fetchFile
buildDocsManifest->>mapWithConcurrency: markdown files, concurrency limit
mapWithConcurrency->>collectRedirectEntriesForFile: node, fetchFile, onCanonicalPath
collectRedirectEntriesForFile->>fetchFile: fetch markdown content
fetchFile-->>collectRedirectEntriesForFile: content or GitHubContentError
collectRedirectEntriesForFile-->>mapWithConcurrency: redirect entries or empty list
mapWithConcurrency-->>buildDocsManifest: flattened redirect entries
Suggested reviewers: 🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/docs-manifest-concurrency.test.ts (1)
58-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove stray
console.logdebug statements.These top-level
console.logcalls at Line 58 and Line 132 are debug artifacts unrelated tonode:testassertions; the test runner already reports pass/fail per test.🧹 Remove debug logs
-console.log('docs manifest concurrency tests passed')-console.log('buildDocsManifest per-file fault tolerance tests passed')Also applies to: 132-132
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/docs-manifest-concurrency.test.ts` at line 58, Remove the stray top-level debug logging from the test module: the `console.log` calls in `tests/docs-manifest-concurrency.test.ts` are unrelated to `node:test` assertions and should be deleted. Update the test file around the affected statements so only the actual test cases and assertions remain, without any extra logging in the module scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/docs-manifest-concurrency.test.ts`:
- Line 58: Remove the stray top-level debug logging from the test module: the
`console.log` calls in `tests/docs-manifest-concurrency.test.ts` are unrelated
to `node:test` assertions and should be deleted. Update the test file around the
affected statements so only the actual test cases and assertions remain, without
any extra logging in the module scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a84bdd7e-3a39-4016-a8a2-36da1218abdb
📒 Files selected for processing (2)
src/utils/docs.functions.tstests/docs-manifest-concurrency.test.ts
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
tanstack-com | ed986b5 | Commit Preview URL Branch Preview URL |
Jul 02 2026, 07:44 PM |
Problem
Any docs request for a path missing from the lightweight path manifest (typo, dead link, aggregation page, or literally any nonexistent slug) falls through to
buildDocsManifest, which fetches every markdown file in the library's docs tree one at a time to read redirect frontmatter. Fortanstack/querythat's 494 sequential GitHub round-trips with no concurrency and no per-file fault tolerance.Reproduced against production: a docs path with no
.mdsibling hangs 45-90s+ with zero bytes returned, both with and withoutAccept: text/markdown. A single recoverable GitHub error (rate limit, 5xx, transient network failure) on any one file also rejected the entire manifest build, which could cascade into a real 404 for completely unrelated docs pages.Fix
RAW_FETCH_CONCURRENCYused elsewhere in this codebase) instead of fully sequential.Not in scope
There's a second, separate slow path (
fetchApiContentsRemoteFromContentsApi, a sequential per-directory GitHub Contents API fallback) that this PR does not touch. Flagging for a follow-up rather than guessing at a fix without production visibility into how often it fires.Summary by CodeRabbit
New Features
Bug Fixes
Tests