Skip to content

Commit 616c16a

Browse files
fix: bound docs manifest fan-out to stop long hangs on redirect-miss docs requests (#1026)
* feat: add concurrency handling for document fetching and redirect collection * format * fix: keep docs server imports out of client bundle path * chore: finalize CI fix validation --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 09622c9 commit 616c16a

2 files changed

Lines changed: 260 additions & 45 deletions

File tree

src/utils/docs.functions.ts

Lines changed: 128 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,14 @@ import { createServerFn } from '@tanstack/react-start'
33
import { setResponseHeader } from '@tanstack/react-start/server'
44
import removeMarkdown from 'remove-markdown'
55
import * as v from 'valibot'
6-
import {
7-
extractFrontMatter,
8-
fetchApiContents,
9-
fetchRepoFile,
10-
isRecoverableGitHubContentError,
11-
shouldUseLocalDocsFiles,
12-
} from '~/utils/documents.server'
136
import { extractFrameworksFromMarkdown } from './markdown/filterFrameworkContent'
14-
import { getCachedDocsArtifact } from './github-content-cache.server'
157
import { buildRedirectManifest, type RedirectManifestEntry } from './redirects'
168
import { isValidRepoPath, MAX_REPO_PATH_LENGTH } from './repo-path'
179
import { removeLeadingSlash } from './utils'
1810
import type { DocsRedirectManifest } from './docs-redirects'
11+
import type { GitHubFileNode } from './documents.server'
1912

20-
type DocsTreeNode = {
13+
export type DocsTreeNode = {
2114
path: string
2215
children?: Array<DocsTreeNode>
2316
}
@@ -92,10 +85,45 @@ const docsRedirectInput = v.object({
9285
docsPaths: v.array(v.pipe(v.string(), v.maxLength(512))),
9386
})
9487

88+
// Matches RAW_FETCH_CONCURRENCY in github-example.server.ts.
89+
const DOCS_MANIFEST_FETCH_CONCURRENCY = 6
90+
91+
export async function mapWithConcurrency<T, TResult>(
92+
values: Array<T>,
93+
concurrency: number,
94+
fn: (value: T) => Promise<TResult>,
95+
) {
96+
const results = new Array<TResult>(values.length)
97+
let index = 0
98+
99+
const workers = Array.from(
100+
{ length: Math.min(concurrency, values.length) },
101+
async () => {
102+
while (index < values.length) {
103+
const currentIndex = index
104+
index += 1
105+
results[currentIndex] = await fn(values[currentIndex])
106+
}
107+
},
108+
)
109+
110+
await Promise.all(workers)
111+
112+
return results
113+
}
114+
95115
const temporarilyUnavailableMarkdown = `# Content temporarily unavailable
96116
97117
We are having trouble fetching this document from GitHub right now. Please try again in a minute.`
98118

119+
async function loadDocumentsServerModule() {
120+
return import('./documents.server')
121+
}
122+
123+
async function loadGitHubContentCacheServerModule() {
124+
return import('./github-content-cache.server')
125+
}
126+
99127
function buildUnavailableFile(filePath: string) {
100128
if (filePath.toLowerCase().endsWith('.md')) {
101129
return temporarilyUnavailableMarkdown
@@ -109,6 +137,9 @@ async function readRepoFileOrFallback(
109137
branch: string,
110138
filePath: string,
111139
) {
140+
const { fetchRepoFile, isRecoverableGitHubContentError } =
141+
await loadDocumentsServerModule()
142+
112143
try {
113144
return await fetchRepoFile(repo, branch, filePath)
114145
} catch (error) {
@@ -146,6 +177,64 @@ function isDocsManifest(value: unknown): value is DocsManifest {
146177
)
147178
}
148179

180+
// Extracted so tests can inject a fake fetchFile without hitting real
181+
// GitHub network/cache.
182+
export async function collectRedirectEntriesForFile(
183+
node: DocsTreeNode,
184+
opts: {
185+
docsRoot: string
186+
fetchFile: (filePath: string) => Promise<string | null>
187+
onCanonicalPath: (canonicalPath: string) => void
188+
},
189+
): Promise<Array<RedirectManifestEntry>> {
190+
const { extractFrontMatter, isRecoverableGitHubContentError } =
191+
await loadDocumentsServerModule()
192+
const canonicalPath = getCanonicalDocsPath(node.path, opts.docsRoot)
193+
194+
if (canonicalPath === null) {
195+
return []
196+
}
197+
198+
opts.onCanonicalPath(canonicalPath)
199+
200+
let file: string | null
201+
try {
202+
file = await opts.fetchFile(node.path)
203+
} catch (error) {
204+
if (!isRecoverableGitHubContentError(error)) {
205+
throw error
206+
}
207+
208+
return []
209+
}
210+
211+
if (!file) {
212+
return []
213+
}
214+
215+
const frontMatter = extractFrontMatter(file)
216+
const entries: Array<RedirectManifestEntry> = []
217+
218+
for (const redirectFrom of frontMatter.data.redirectFrom ?? []) {
219+
const normalizedRedirect = normalizeDocsRedirectPath(
220+
redirectFrom,
221+
opts.docsRoot,
222+
)
223+
224+
if (!normalizedRedirect || normalizedRedirect === canonicalPath) {
225+
continue
226+
}
227+
228+
entries.push({
229+
from: normalizedRedirect,
230+
to: canonicalPath,
231+
source: node.path,
232+
})
233+
}
234+
235+
return entries
236+
}
237+
149238
async function buildDocsManifest({
150239
repo,
151240
branch,
@@ -155,6 +244,7 @@ async function buildDocsManifest({
155244
branch: string
156245
docsRoot: string
157246
}): Promise<DocsManifest> {
247+
const { fetchApiContents, fetchRepoFile } = await loadDocumentsServerModule()
158248
const nodes = await fetchApiContents(repo, branch, docsRoot)
159249

160250
if (!nodes) {
@@ -165,46 +255,23 @@ async function buildDocsManifest({
165255
node.path.endsWith('.md'),
166256
)
167257
const paths = new Set<string>()
168-
const redirects: Array<RedirectManifestEntry> = []
169258

170-
for (const node of markdownFiles) {
171-
const canonicalPath = getCanonicalDocsPath(node.path, docsRoot)
172-
173-
if (canonicalPath === null) {
174-
continue
175-
}
176-
177-
paths.add(canonicalPath)
178-
179-
const file = await fetchRepoFile(repo, branch, node.path)
180-
181-
if (!file) {
182-
continue
183-
}
184-
185-
const frontMatter = extractFrontMatter(file)
186-
187-
for (const redirectFrom of frontMatter.data.redirectFrom ?? []) {
188-
const normalizedRedirect = normalizeDocsRedirectPath(
189-
redirectFrom,
259+
// A recoverable error on one file must not fail the whole manifest build
260+
// (see collectRedirectEntriesForFile).
261+
const redirectsByFile = await mapWithConcurrency(
262+
markdownFiles,
263+
DOCS_MANIFEST_FETCH_CONCURRENCY,
264+
(node) =>
265+
collectRedirectEntriesForFile(node, {
190266
docsRoot,
191-
)
192-
193-
if (!normalizedRedirect || normalizedRedirect === canonicalPath) {
194-
continue
195-
}
196-
197-
redirects.push({
198-
from: normalizedRedirect,
199-
to: canonicalPath,
200-
source: node.path,
201-
})
202-
}
203-
}
267+
fetchFile: (filePath) => fetchRepoFile(repo, branch, filePath),
268+
onCanonicalPath: (canonicalPath) => paths.add(canonicalPath),
269+
}),
270+
)
204271

205272
return {
206273
paths: Array.from(paths),
207-
redirects: buildRedirectManifest(redirects, {
274+
redirects: buildRedirectManifest(redirectsByFile.flat(), {
208275
label: `docs redirects for ${repo}@${branch}:${docsRoot}`,
209276
}),
210277
}
@@ -219,6 +286,7 @@ async function buildDocsPathManifest({
219286
branch: string
220287
docsRoot: string
221288
}): Promise<DocsManifest> {
289+
const { fetchApiContents } = await loadDocumentsServerModule()
222290
const nodes = await fetchApiContents(repo, branch, docsRoot)
223291

224292
if (!nodes) {
@@ -242,6 +310,11 @@ export const fetchDocsManifest = createServerFn({ method: 'GET' })
242310
.validator(docsManifestInput)
243311
.handler(async ({ data }) => {
244312
const { repo, branch, docsRoot } = data
313+
const [{ shouldUseLocalDocsFiles }, { getCachedDocsArtifact }] =
314+
await Promise.all([
315+
loadDocumentsServerModule(),
316+
loadGitHubContentCacheServerModule(),
317+
])
245318

246319
if (shouldUseLocalDocsFiles()) {
247320
return buildDocsManifest({ repo, branch, docsRoot })
@@ -262,6 +335,11 @@ export const fetchDocsPathManifest = createServerFn({ method: 'GET' })
262335
.validator(docsManifestInput)
263336
.handler(async ({ data }) => {
264337
const { repo, branch, docsRoot } = data
338+
const [{ shouldUseLocalDocsFiles }, { getCachedDocsArtifact }] =
339+
await Promise.all([
340+
loadDocumentsServerModule(),
341+
loadGitHubContentCacheServerModule(),
342+
])
265343

266344
if (shouldUseLocalDocsFiles()) {
267345
return buildDocsPathManifest({ repo, branch, docsRoot })
@@ -281,6 +359,8 @@ export const fetchDocsPathManifest = createServerFn({ method: 'GET' })
281359
export const fetchDocsRedirect = createServerFn({ method: 'GET' })
282360
.validator(docsRedirectInput)
283361
.handler(async ({ data }) => {
362+
const { isRecoverableGitHubContentError } =
363+
await loadDocumentsServerModule()
284364
let manifest: DocsManifest
285365

286366
try {
@@ -329,6 +409,7 @@ export const fetchDocs = createServerFn({ method: 'GET' })
329409
throw notFound()
330410
}
331411

412+
const { extractFrontMatter } = await loadDocumentsServerModule()
332413
const frontMatter = extractFrontMatter(file)
333414
const description =
334415
frontMatter.userDescription ?? removeMarkdown(frontMatter.excerpt ?? '')
@@ -386,7 +467,9 @@ export const fetchRepoDirectoryContents = createServerFn({
386467
.validator(repoDirectoryInput)
387468
.handler(async ({ data }: { data: RepoDirectoryRequest }) => {
388469
const { repo, branch, startingPath } = data
389-
let githubContents: Awaited<ReturnType<typeof fetchApiContents>>
470+
const { fetchApiContents, isRecoverableGitHubContentError } =
471+
await loadDocumentsServerModule()
472+
let githubContents: Array<GitHubFileNode> | null
390473

391474
try {
392475
githubContents = await fetchApiContents(repo, branch, startingPath)

0 commit comments

Comments
 (0)