fix(Home): add AbortController cleanup to prevent memory leaks on unmount#7
Conversation
…ount useEffect had no cleanup — if the component unmounted before fetches completed, React would attempt state updates on an unmounted component. AbortController cancels both fetches on cleanup. AbortError is silently ignored (not treated as a fetch failure) per Gemini review on PR #6.
When a fetch is aborted (e.g. component unmount), the AbortError should be swallowed — it is not a real failure. Verifies console.error is NOT called when all fetches reject with an AbortError.
Summary of ChangesHello @alichherawalla, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
coverage/ directories were untracked. Also removes accidental *copy.md files that were left over from a previous session.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly adds an AbortController to the useEffect hook in the Home component to cancel fetch requests on unmount, preventing potential memory leaks. The accompanying test effectively validates this new behavior. My review includes suggestions to improve the robustness of the fetch logic by adding checks for HTTP error responses, which is a critical part of handling data fetching.
| const { signal } = controller; | ||
|
|
||
| fetch('/api/movies', { signal }) | ||
| .then(res => res.json()) |
There was a problem hiding this comment.
It's a good practice to check for HTTP error responses, as fetch only rejects on network errors, not on HTTP error statuses like 4xx or 5xx. Without this check, a non-OK response could lead to an attempt to parse a non-JSON body, causing a cryptic error.
.then(res => {
if (!res.ok) {
throw new Error(`HTTP error! status: ${res.status}`);
}
return res.json();
})
|
|
||
| fetch('/api/movies/meta/genres') | ||
| fetch('/api/movies/meta/genres', { signal }) | ||
| .then(res => res.json()) |
There was a problem hiding this comment.
Similar to the movies fetch, it's important to handle HTTP error statuses here as well to make the data fetching more robust.
.then(res => {
if (!res.ok) {
throw new Error(`HTTP error! status: ${res.status}`);
}
return res.json();
})
fetch() only rejects on network errors — HTTP 4xx/5xx responses resolve with a non-OK status. Without this check a 500 would try to parse an error body as JSON and throw a cryptic error. Addresses high-priority Gemini feedback on PR #7.
Adds two tests that verify a non-OK HTTP response (4xx/5xx) is treated as an error and logged via console.error. Updates all existing mocks to include ok: true to match the new res.ok guard.
1 participant
Summary
AbortControllertouseEffectinHome.tsx, passingsignalto both fetch callscontroller.abort()on unmountProblem
The
useEffecthook had no cleanup. IfHomeunmounted before either fetch completed (e.g. quick navigation away), React would attemptsetMovies/setLoadingon an unmounted component — causing a memory leak and "Can't perform a React state update on an unmounted component" warning.Test plan
does not log console.error when fetch is aborted— new test asserts AbortError is silently ignoredpnpm --filter @stagepass/web test:coverage— 24/24 green🤖 Generated with Claude Code