Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/hooks/useGitHubData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,28 @@ export const useGitHubData = (getOctokit: () => any) => {
setError('');

try {
const [issueRes, prRes] = await Promise.all([
const [issueRes, prRes] = await Promise.allSettled([
fetchPaginated(octokit, username, 'issue', page, perPage),
fetchPaginated(octokit, username, 'pr', page, perPage),
]);

setIssues(issueRes.items);
setPrs(prRes.items);
setTotalIssues(issueRes.total);
setTotalPrs(prRes.total);
if (issueRes.status === 'fulfilled') {
setIssues(issueRes.value.items);
setTotalIssues(issueRes.value.total);
}

if (prRes.status === 'fulfilled') {
setPrs(prRes.value.items);
setTotalPrs(prRes.value.total);
}

if (
issueRes.status === 'rejected' ||
prRes.status === 'rejected'
) {
setError('Some GitHub data could not be fetched completely.');
}
Comment on lines +39 to +59
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Inspect rejection reasons to preserve specific error handling and fix rate limiting detection.

The switch to Promise.allSettled prevents specific error details from reaching the catch block. Critical issues:

  1. Rate limiting detection is broken: Line 61 always sets rateLimited(false), but the catch block's 403 handling (lines 64-66) is now unreachable since Promise.allSettled never throws. Users hitting rate limits won't be prompted to add a PAT.

  2. Misleading error when both requests fail: When both issueRes and prRes are rejected, "Some GitHub data could not be fetched completely" incorrectly implies partial success. It should indicate complete failure.

  3. Loss of specific error context: Rejected promises contain error details in their reason property (403 rate limit, 401 permission, 404 not found, validation errors), but these aren't inspected.

🔧 Proposed fix to inspect rejection reasons
 const [issueRes, prRes] = await Promise.allSettled([
   fetchPaginated(octokit, username, 'issue', page, perPage),
   fetchPaginated(octokit, username, 'pr', page, perPage),
 ]);

+// Check for rate limiting or critical errors in rejections
+const rejections = [issueRes, prRes].filter(r => r.status === 'rejected');
+for (const rejection of rejections) {
+  const err: any = rejection.reason;
+  if (err.status === 403) {
+    setError('GitHub API rate limit exceeded. Please provide a PAT to continue.');
+    setRateLimited(true);
+    setLoading(false);
+    return;
+  }
+}
+
 if (issueRes.status === 'fulfilled') {
   setIssues(issueRes.value.items);
   setTotalIssues(issueRes.value.total);
 }

 if (prRes.status === 'fulfilled') {
   setPrs(prRes.value.items);
   setTotalPrs(prRes.value.total);
 }

-if (
-  issueRes.status === 'rejected' ||
-  prRes.status === 'rejected'
-) {
-  setError('Some GitHub data could not be fetched completely.');
+// Set appropriate error message based on success/failure mix
+const bothFailed = issueRes.status === 'rejected' && prRes.status === 'rejected';
+const partialFailure = issueRes.status === 'rejected' || prRes.status === 'rejected';
+
+if (bothFailed) {
+  // Extract specific error message from one of the rejections
+  const err: any = issueRes.reason || prRes.reason;
+  const errorMessage = err.message?.toLowerCase() || "";
+  
+  if (errorMessage.includes("do not exist")) {
+    setError('User not found. Please check the spelling of the GitHub username.');
+  } else if (errorMessage.includes("validation failed")) {
+    setError('Invalid GitHub username or insufficient permissions.');
+  } else if (err.status === 401 || errorMessage.includes("permission")) {
+    setError('Private repository detected. Please input PAT.');
+  } else if (err.status === 404) {
+    setError('Resource not found.');
+  } else {
+    setError('Unable to fetch GitHub data. Please verify the username, token, or network connection.');
+  }
+} else if (partialFailure) {
+  setError('Some GitHub data could not be fetched completely.');
 }

 setRateLimited(false);
📝 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
const [issueRes, prRes] = await Promise.allSettled([
fetchPaginated(octokit, username, 'issue', page, perPage),
fetchPaginated(octokit, username, 'pr', page, perPage),
]);
setIssues(issueRes.items);
setPrs(prRes.items);
setTotalIssues(issueRes.total);
setTotalPrs(prRes.total);
if (issueRes.status === 'fulfilled') {
setIssues(issueRes.value.items);
setTotalIssues(issueRes.value.total);
}
if (prRes.status === 'fulfilled') {
setPrs(prRes.value.items);
setTotalPrs(prRes.value.total);
}
if (
issueRes.status === 'rejected' ||
prRes.status === 'rejected'
) {
setError('Some GitHub data could not be fetched completely.');
}
const [issueRes, prRes] = await Promise.allSettled([
fetchPaginated(octokit, username, 'issue', page, perPage),
fetchPaginated(octokit, username, 'pr', page, perPage),
]);
// Check for rate limiting or critical errors in rejections
const rejections = [issueRes, prRes].filter(r => r.status === 'rejected');
for (const rejection of rejections) {
const err: any = rejection.reason;
if (err.status === 403) {
setError('GitHub API rate limit exceeded. Please provide a PAT to continue.');
setRateLimited(true);
setLoading(false);
return;
}
}
if (issueRes.status === 'fulfilled') {
setIssues(issueRes.value.items);
setTotalIssues(issueRes.value.total);
}
if (prRes.status === 'fulfilled') {
setPrs(prRes.value.items);
setTotalPrs(prRes.value.total);
}
// Set appropriate error message based on success/failure mix
const bothFailed = issueRes.status === 'rejected' && prRes.status === 'rejected';
const partialFailure = issueRes.status === 'rejected' || prRes.status === 'rejected';
if (bothFailed) {
// Extract specific error message from one of the rejections
const err: any = issueRes.reason || prRes.reason;
const errorMessage = err.message?.toLowerCase() || "";
if (errorMessage.includes("do not exist")) {
setError('User not found. Please check the spelling of the GitHub username.');
} else if (errorMessage.includes("validation failed")) {
setError('Invalid GitHub username or insufficient permissions.');
} else if (err.status === 401 || errorMessage.includes("permission")) {
setError('Private repository detected. Please input PAT.');
} else if (err.status === 404) {
setError('Resource not found.');
} else {
setError('Unable to fetch GitHub data. Please verify the username, token, or network connection.');
}
} else if (partialFailure) {
setError('Some GitHub data could not be fetched completely.');
}
setRateLimited(false);
🤖 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 `@src/hooks/useGitHubData.ts` around lines 39 - 59, In useGitHubData,
Promise.allSettled now swallows throw behavior so inspect the settled results'
reason fields (issueRes.reason and prRes.reason) to restore specific handling:
if either reason.status === 403 call rateLimited(true) and set an appropriate
PAT prompt, if either is 401/404/validation use those codes to set precise error
messages, and if both are rejected treat it as a complete failure (setError to
indicate total failure) vs. a partial failure when only one rejected; update the
logic around setIssues/setPrs, setTotalIssues/setTotalPrs accordingly so
fulfilled results are used while rejected ones are examined for their .reason to
decide rateLimited(), specific error text, or generic error.


setRateLimited(false);
} catch (err: any) {
const errorMessage = err.message?.toLowerCase() || "";
Expand All @@ -53,14 +66,16 @@ export const useGitHubData = (getOctokit: () => any) => {
setRateLimited(true);
} else if (errorMessage.includes("do not exist")){
setError('User not found. Please check the spelling of the GitHub username.');
} else if (err.status === 401 || errorMessage.includes("permission")){
}
else if (errorMessage.includes("validation failed")) {
setError('Invalid GitHub username or insufficient permissions.');
}
else if (err.status === 401 || errorMessage.includes("permission")){
setError('Private repository detected. Please input PAT.');
}else if(err.status===404){
setError('Resource not found.');
}
else if (errorMessage.includes("validation failed")) {
setError('Invalid GitHub username or insufficient permissions.');
}

else {
setError(
'Unable to fetch GitHub data. Please verify the username, token, or network connection.'
Expand Down