Skip to content

Commit 1b8c761

Browse files
wgordon17wgordon17
committed
fix(api): adds catch-all for unexpected response shapes in pagination
1 parent e8c1b88 commit 1b8c761

2 files changed

Lines changed: 219 additions & 165 deletions

File tree

‎src/app/services/api.ts‎

Lines changed: 183 additions & 165 deletions
Original file line numberDiff line numberDiff line change
@@ -396,78 +396,89 @@ async function graphqlSearchIssues(
396396
let cursor: string | null = null;
397397

398398
while (true) {
399-
let response: GraphQLIssueSearchResponse;
400-
let isPartial = false;
401399
try {
402-
response = await octokit.graphql<GraphQLIssueSearchResponse>(
403-
ISSUES_SEARCH_QUERY,
404-
{ q: queryString, cursor }
405-
);
406-
} catch (err) {
407-
// GraphqlResponseError contains partial data — extract valid nodes before recording error
408-
const partial = extractGraphQLPartialData<GraphQLIssueSearchResponse>(err);
409-
if (partial) {
410-
response = partial;
411-
isPartial = true;
412-
const { message } = extractRejectionError(err);
413-
errors.push({
414-
repo: `search-batch-${chunkIdx + 1}/${chunks.length}`,
415-
statusCode: null,
416-
message,
417-
retryable: true,
418-
});
419-
} else {
420-
const { statusCode, message } = extractRejectionError(err);
421-
errors.push({
422-
repo: `search-batch-${chunkIdx + 1}/${chunks.length}`,
423-
statusCode,
424-
message,
425-
retryable: statusCode === null || statusCode >= 500,
400+
let response: GraphQLIssueSearchResponse;
401+
let isPartial = false;
402+
try {
403+
response = await octokit.graphql<GraphQLIssueSearchResponse>(
404+
ISSUES_SEARCH_QUERY,
405+
{ q: queryString, cursor }
406+
);
407+
} catch (err) {
408+
const partial = extractGraphQLPartialData<GraphQLIssueSearchResponse>(err);
409+
if (partial) {
410+
response = partial;
411+
isPartial = true;
412+
const { message } = extractRejectionError(err);
413+
errors.push({
414+
repo: `search-batch-${chunkIdx + 1}/${chunks.length}`,
415+
statusCode: null,
416+
message,
417+
retryable: true,
418+
});
419+
} else {
420+
const { statusCode, message } = extractRejectionError(err);
421+
errors.push({
422+
repo: `search-batch-${chunkIdx + 1}/${chunks.length}`,
423+
statusCode,
424+
message,
425+
retryable: statusCode === null || statusCode >= 500,
426+
});
427+
break;
428+
}
429+
}
430+
431+
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
432+
433+
for (const node of response.search.nodes) {
434+
if (!node || node.databaseId == null || !node.repository) continue;
435+
if (seen.has(node.databaseId)) continue;
436+
seen.add(node.databaseId);
437+
issues.push({
438+
id: node.databaseId,
439+
number: node.number,
440+
title: node.title,
441+
state: node.state,
442+
htmlUrl: node.url,
443+
createdAt: node.createdAt,
444+
updatedAt: node.updatedAt,
445+
userLogin: node.author?.login ?? "",
446+
userAvatarUrl: node.author?.avatarUrl ?? "",
447+
labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })),
448+
assigneeLogins: node.assignees.nodes.map((a) => a.login),
449+
repoFullName: node.repository.nameWithOwner,
450+
comments: node.comments.totalCount,
426451
});
427-
break;
428452
}
429-
}
430453

431-
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
432-
433-
for (const node of response.search.nodes) {
434-
if (!node || node.databaseId == null || !node.repository) continue;
435-
if (seen.has(node.databaseId)) continue;
436-
seen.add(node.databaseId);
437-
issues.push({
438-
id: node.databaseId,
439-
number: node.number,
440-
title: node.title,
441-
state: node.state,
442-
htmlUrl: node.url,
443-
createdAt: node.createdAt,
444-
updatedAt: node.updatedAt,
445-
userLogin: node.author?.login ?? "",
446-
userAvatarUrl: node.author?.avatarUrl ?? "",
447-
labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })),
448-
assigneeLogins: node.assignees.nodes.map((a) => a.login),
449-
repoFullName: node.repository.nameWithOwner,
450-
comments: node.comments.totalCount,
451-
});
452-
}
454+
if (isPartial) break;
453455

454-
// Don't paginate after partial error — pageInfo may be unreliable
455-
if (isPartial) break;
456+
if (issues.length >= 1000 && !capReached) {
457+
capReached = true;
458+
const total = response.search.issueCount;
459+
console.warn(`[api] Issue search results capped at 1000 (${total} total)`);
460+
pushNotification(
461+
"search/issues",
462+
`Issue search results capped at 1,000 of ${total.toLocaleString()} total — some items are hidden`,
463+
"warning"
464+
);
465+
break;
466+
}
456467

457-
if (issues.length >= 1000 && !capReached) {
458-
capReached = true;
459-
const total = response.search.issueCount;
460-
console.warn(`[api] Issue search results capped at 1000 (${total} total)`);
461-
pushNotification(
462-
"search/issues",
463-
`Issue search results capped at 1,000 of ${total.toLocaleString()} total — some items are hidden`,
464-
"warning"
465-
);
468+
if (!response.search.pageInfo.hasNextPage || !response.search.pageInfo.endCursor) break;
469+
cursor = response.search.pageInfo.endCursor;
470+
} catch (err) {
471+
// Catch-all for unexpected runtime errors (malformed response shapes, TypeErrors, etc.)
472+
// Preserves any issues collected so far rather than losing the entire fetch
473+
const { message } = extractRejectionError(err);
474+
errors.push({
475+
repo: `search-batch-${chunkIdx + 1}/${chunks.length}`,
476+
statusCode: null,
477+
message,
478+
retryable: false,
479+
});
466480
break;
467481
}
468-
469-
if (!response.search.pageInfo.hasNextPage || !response.search.pageInfo.endCursor) break;
470-
cursor = response.search.pageInfo.endCursor;
471482
}
472483
}
473484

@@ -531,118 +542,125 @@ async function graphqlSearchPRs(
531542
let cursor: string | null = null;
532543

533544
while (true) {
534-
let response: GraphQLPRSearchResponse;
535-
let isPartial = false;
536545
try {
537-
response = await octokit.graphql<GraphQLPRSearchResponse>(
538-
PR_SEARCH_QUERY,
539-
{ q: queryString, cursor }
540-
);
541-
} catch (err) {
542-
const partial = extractGraphQLPartialData<GraphQLPRSearchResponse>(err);
543-
if (partial) {
544-
response = partial;
545-
isPartial = true;
546-
const { message } = extractRejectionError(err);
547-
errors.push({
548-
repo: `pr-search-batch-${chunkIdx + 1}/${chunks.length}`,
549-
statusCode: null,
550-
message,
551-
retryable: true,
552-
});
553-
} else {
554-
const { statusCode, message } = extractRejectionError(err);
555-
errors.push({
556-
repo: `pr-search-batch-${chunkIdx + 1}/${chunks.length}`,
557-
statusCode,
558-
message,
559-
retryable: statusCode === null || statusCode >= 500,
560-
});
561-
break;
546+
let response: GraphQLPRSearchResponse;
547+
let isPartial = false;
548+
try {
549+
response = await octokit.graphql<GraphQLPRSearchResponse>(
550+
PR_SEARCH_QUERY,
551+
{ q: queryString, cursor }
552+
);
553+
} catch (err) {
554+
const partial = extractGraphQLPartialData<GraphQLPRSearchResponse>(err);
555+
if (partial) {
556+
response = partial;
557+
isPartial = true;
558+
const { message } = extractRejectionError(err);
559+
errors.push({
560+
repo: `pr-search-batch-${chunkIdx + 1}/${chunks.length}`,
561+
statusCode: null,
562+
message,
563+
retryable: true,
564+
});
565+
} else {
566+
const { statusCode, message } = extractRejectionError(err);
567+
errors.push({
568+
repo: `pr-search-batch-${chunkIdx + 1}/${chunks.length}`,
569+
statusCode,
570+
message,
571+
retryable: statusCode === null || statusCode >= 500,
572+
});
573+
break;
574+
}
562575
}
563-
}
564-
565-
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
566576

567-
for (const node of response.search.nodes) {
568-
if (!node || node.databaseId == null || !node.repository) continue;
569-
if (prMap.has(node.databaseId)) continue;
570-
571-
const pendingLogins = node.reviewRequests.nodes
572-
.map((n) => n.requestedReviewer?.login)
573-
.filter((l): l is string => l != null);
574-
const actualLogins = node.latestReviews.nodes
575-
.map((n) => n.author?.login)
576-
.filter((l): l is string => l != null);
577-
// Normalize logins to lowercase to avoid case-sensitive duplicates
578-
const reviewerLogins = [...new Set([...pendingLogins, ...actualLogins].map(l => l.toLowerCase()))];
579-
580-
const rawState =
581-
node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null;
582-
const checkStatus = mapCheckStatus(rawState);
583-
584-
// Store headRepository info for fork detection
585-
if (node.headRepository) {
586-
const parts = node.headRepository.nameWithOwner.split("/");
587-
if (parts.length === 2) {
588-
headRepoInfoMap.set(node.databaseId, {
589-
owner: node.headRepository.owner.login,
590-
repoName: parts[1],
591-
});
577+
if (response.rateLimit) updateGraphqlRateLimit(response.rateLimit);
578+
579+
for (const node of response.search.nodes) {
580+
if (!node || node.databaseId == null || !node.repository) continue;
581+
if (prMap.has(node.databaseId)) continue;
582+
583+
const pendingLogins = node.reviewRequests.nodes
584+
.map((n) => n.requestedReviewer?.login)
585+
.filter((l): l is string => l != null);
586+
const actualLogins = node.latestReviews.nodes
587+
.map((n) => n.author?.login)
588+
.filter((l): l is string => l != null);
589+
const reviewerLogins = [...new Set([...pendingLogins, ...actualLogins].map(l => l.toLowerCase()))];
590+
591+
const rawState =
592+
node.commits.nodes[0]?.commit?.statusCheckRollup?.state ?? null;
593+
const checkStatus = mapCheckStatus(rawState);
594+
595+
if (node.headRepository) {
596+
const parts = node.headRepository.nameWithOwner.split("/");
597+
if (parts.length === 2) {
598+
headRepoInfoMap.set(node.databaseId, {
599+
owner: node.headRepository.owner.login,
600+
repoName: parts[1],
601+
});
602+
} else {
603+
headRepoInfoMap.set(node.databaseId, null);
604+
}
592605
} else {
593-
// Malformed nameWithOwner — treat as deleted fork (no fallback)
594606
headRepoInfoMap.set(node.databaseId, null);
595607
}
596-
} else {
597-
headRepoInfoMap.set(node.databaseId, null);
598-
}
599608

600-
prMap.set(node.databaseId, {
601-
id: node.databaseId,
602-
number: node.number,
603-
title: node.title,
604-
state: node.state,
605-
draft: node.isDraft,
606-
htmlUrl: node.url,
607-
createdAt: node.createdAt,
608-
updatedAt: node.updatedAt,
609-
userLogin: node.author?.login ?? "",
610-
userAvatarUrl: node.author?.avatarUrl ?? "",
611-
headSha: node.headRefOid,
612-
headRef: node.headRefName,
613-
baseRef: node.baseRefName,
614-
assigneeLogins: node.assignees.nodes.map((a) => a.login),
615-
reviewerLogins,
616-
repoFullName: node.repository.nameWithOwner,
617-
checkStatus,
618-
additions: node.additions,
619-
deletions: node.deletions,
620-
changedFiles: node.changedFiles,
621-
comments: node.comments.totalCount,
622-
reviewThreads: node.reviewThreads.totalCount,
623-
labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })),
624-
reviewDecision: mapReviewDecision(node.reviewDecision),
625-
totalReviewCount: node.latestReviews.totalCount,
626-
});
627-
}
609+
prMap.set(node.databaseId, {
610+
id: node.databaseId,
611+
number: node.number,
612+
title: node.title,
613+
state: node.state,
614+
draft: node.isDraft,
615+
htmlUrl: node.url,
616+
createdAt: node.createdAt,
617+
updatedAt: node.updatedAt,
618+
userLogin: node.author?.login ?? "",
619+
userAvatarUrl: node.author?.avatarUrl ?? "",
620+
headSha: node.headRefOid,
621+
headRef: node.headRefName,
622+
baseRef: node.baseRefName,
623+
assigneeLogins: node.assignees.nodes.map((a) => a.login),
624+
reviewerLogins,
625+
repoFullName: node.repository.nameWithOwner,
626+
checkStatus,
627+
additions: node.additions,
628+
deletions: node.deletions,
629+
changedFiles: node.changedFiles,
630+
comments: node.comments.totalCount,
631+
reviewThreads: node.reviewThreads.totalCount,
632+
labels: node.labels.nodes.map((l) => ({ name: l.name, color: l.color })),
633+
reviewDecision: mapReviewDecision(node.reviewDecision),
634+
totalReviewCount: node.latestReviews.totalCount,
635+
});
636+
}
628637

629-
// Don't paginate after partial error — pageInfo may be unreliable
630-
if (isPartial) break;
638+
if (isPartial) break;
639+
640+
if (prMap.size >= 1000 && !prCapReached) {
641+
prCapReached = true;
642+
const total = response.search.issueCount;
643+
console.warn(`[api] PR search results capped at 1000 (${total} total)`);
644+
pushNotification(
645+
"search/prs",
646+
`PR search results capped at 1,000 of ${total.toLocaleString()} total — some items are hidden`,
647+
"warning"
648+
);
649+
break;
650+
}
631651

632-
if (prMap.size >= 1000 && !prCapReached) {
633-
prCapReached = true;
634-
const total = response.search.issueCount;
635-
console.warn(`[api] PR search results capped at 1000 (${total} total)`);
636-
pushNotification(
637-
"search/prs",
638-
`PR search results capped at 1,000 of ${total.toLocaleString()} total — some items are hidden`,
639-
"warning"
640-
);
652+
if (!response.search.pageInfo.hasNextPage || !response.search.pageInfo.endCursor) break;
653+
cursor = response.search.pageInfo.endCursor;
654+
} catch (err) {
655+
const { message } = extractRejectionError(err);
656+
errors.push({
657+
repo: `pr-search-batch-${chunkIdx + 1}/${chunks.length}`,
658+
statusCode: null,
659+
message,
660+
retryable: false,
661+
});
641662
break;
642663
}
643-
644-
if (!response.search.pageInfo.hasNextPage || !response.search.pageInfo.endCursor) break;
645-
cursor = response.search.pageInfo.endCursor;
646664
}
647665
}
648666
}

0 commit comments

Comments
 (0)