Skip to content

Add Abort Controller to SuggestedRepos Fetch and Add Error StateΒ #59

@Vedant1703

Description

@Vedant1703

πŸ“‹ 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.

} 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:

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:

const [error, setError] = useState<string | null>(null);

// In the JSX, after the loading block:
{error && !loading && (
  <div className="text-center py-8">
    <p className="text-[#f85149] text-sm mb-3">{error}</p>
    <button
      onClick={() => {
        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
    </button>
  </div>
)}

For the retry to work cleanly, add a retryCount state:

const [retryCount, setRetryCount] = useState(0);

// Add retryCount to useEffect dependencies:
}, [repos, topRepoId, retryCount]);

// In error UI:
<button onClick={() => setRetryCount(c => c + 1)}>Retry</button>

Fix 3 β€” Guard against bad repo_id format (ties into issue #34):

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!

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions