iiitl / iiitl/Opensource_Compass
Add Abort Controller to `SuggestedRepos` Fetch and Add Error State
- Dominant language
- TypeScript
- Stars
- 0
- Forks
- 16
- PR merge metrics
- No merged PRs in 30d
Description
### 📋 Description
In `frontend/app/(auth)/recommendations/components/suggestedrepos.tsx`, when the component loads it fires up to 6 individual `fetchRepository(owner, name)` calls with `Promise.all`. Two real problems exist:
**Problem 1 — No Abort on unmount (memory leak + React warning)**
If the user navigates away from the Recommendations page while those 6 requests are in-flight, the `useEffect` callback has no cleanup. When the async calls eventually resolve, they call `setDetailedRepos(...)` and `setLoading(false)` on an **unmounted component**. In development mode, React shows:
```
Warning: Can't perform a React state update on an unmounted component.
```
This is a memory leak + React warning that pollutes the console for every navigation away from the page.
**Problem 2 — No error state rendered to the user**
The `try/catch` in the effect catches errors and logs to `console.error` only — the user sees an endless skeleton/loading state if the fetch fails, with no ability to retry.
```tsx
} catch (err) {
console.error("Failed to load suggested repos", err);
// ← no error state set! UI just stays loading forever
} finally {
setLoading(false);
}
```
---
### 📍 Files to Change
- `frontend/app/(auth)/recommendations/components/suggestedrepos.tsx`
---
### ✅ What To Do
**Fix 1 — Add `AbortController` for request cancellation on unmount:**
```tsx
useEffect(() => {
const controller = new AbortController();
const loadRepos = async () => {
const otherRepos = repos.filter(r => r !== topRepoId).slice(0, 6);
if (otherRepos.length === 0) {
setLoading(false);
return;
}
setLoading(true);
try {
const promises = otherRepos.map(async (repoId) => {
const [owner, name] = repoId.split('/');
if (!owner || !name) return null; // guard against bad repo_id format
try {
return await fetchRepository(owner, name, controller.signal); // pass signal
} catch (e) {
if ((e as any)?.name === 'AbortError') return null; // ignore abort
console.error(`Failed to fetch suggested repo ${repoId}`, e);
return null;
}
});
const results = await Promise.all(promises);
// Don't update state if the component has already unmounted
if (!controller.signal.aborted) {
setDetailedRepos(results.filter((r): r is Repository => r !== null));
setError(null);
}
} catch (err) {
if (!controller.signal.aborted) {
setError('Failed to load suggested repositories.');
}
} finally {
if (!controller.signal.aborted) {
setLoading(false);
}
}
};
loadRepos();
// Cleanup: abort all in-flight requests when component unmounts or deps change
return () => controller.abort();
}, [repos, topRepoId]);
```
**Fix 2 — Add error state and retry UI:**
```tsx
const [error, setError] = useState(null);
// In the JSX, after the loading block:
{error && !loading && (
{error}
{
setError(null);
setLoading(true);
// re-trigger effect by forcing deps to change
// simplest approach: add a retry counter to state
}}
className="text-sm text-[#2f81f7] hover:underline"
>
Retry
)}
```
For the retry to work cleanly, add a `retryCount` state:
```tsx
const [retryCount, setRetryCount] = useState(0);
// Add retryCount to useEffect dependencies:
}, [repos, topRepoId, retryCount]);
// In error UI:
setRetryCount(c => c + 1)}>Retry
```
**Fix 3 — Guard against bad `repo_id` format (ties into issue #34):**
```tsx
const [owner, name] = repoId.split('/');
if (!owner || !name) {
console.warn(`Skipping suggested repo with invalid format: "${repoId}"`);
return null;
}
```
---
### 🏁 Acceptance Criteria
- [ ] An `AbortController` is created in `useEffect` and cancelled in the cleanup function
- [ ] `setDetailedRepos` and `setLoading` are not called after the component unmounts
- [ ] Navigating away from Recommendations while repos are loading produces no React unmount warnings
- [ ] An `error` state is shown to the user when all fetches fail
- [ ] A "Retry" button triggers the fetch again
- [ ] Repo IDs without a `/` (missing owner) are skipped with a console warning, not passed to `fetchRepository`
---
### 💡 Technical Hints
- To pass `signal` to `fetchRepository`, you may need to update the function signature in `frontend/lib/api/github-service.ts` to accept an optional `signal?: AbortSignal` and forward it to `fetch(url, { signal })`
- Check if `AbortError` correctly — use `err instanceof DOMException && err.name === 'AbortError'` or check `controller.signal.aborted` after the `Promise.all`
- To verify the fix: open the Recommendations page, immediately navigate to the Discover page. Before: React warnings in console. After: no warnings
---
### 🚀 Getting Started
1. Fork the repository
2. Create a branch: `git checkout -b fix/issue-37-suggestedrepos-abort`
3. Update `frontend/app/(auth)/recommendations/components/suggestedrepos.tsx`
4. Optionally update `frontend/lib/api/github-service.ts` to accept `AbortSignal`
5. Test by navigating away mid-load and checking the browser console
6. Open a PR!
Contributor guide
Research direction
Start with frontend/app/(auth)/recommendations/components/suggestedrepos.tsx and trace its useEffect, fetchRepository calls, loading state, and rendered states. Check frontend/lib/api/github-service.ts if the fetch wrapper needs an optional AbortSignal. Done means unmount cleanup prevents state updates, malformed repository IDs are skipped, failures show an error with Retry, and navigation away produces no warnings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- api, frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100