fix: log fetch errors instead of silently swallowing them#6
Conversation
Both fetch calls used empty .catch() — failures were invisible in dev and production logs. Now logs with console.error. Also removes void keyword from fetch calls per Gemini code review feedback (PR #5).
Two new tests verify that failed fetch calls log errors via console.error rather than silently swallowing them.
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 significantly enhances the application's error handling by ensuring that network request failures are no longer silently ignored. By logging these errors, it improves debugging capabilities and provides better visibility into potential issues during data fetching, leading to a more stable and maintainable application. Additionally, it refines the code style by removing unnecessary keywords. 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
|
Documents the fetch error logging step (remove void, log catch errors) that was flagged in the Gemini code review on PR #5.
![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 is a good improvement, as it ensures that fetch errors are logged instead of being silently ignored. The addition of tests to cover these failure scenarios is also great.
I've added one suggestion to improve the useEffect hook by adding a cleanup function with an AbortController. This is a best practice in React for handling asynchronous operations in effects to prevent potential memory leaks and race conditions if the component unmounts before the requests are complete.
| useEffect(() => { | ||
| void fetch('/api/movies') | ||
| fetch('/api/movies') | ||
| .then(res => res.json()) | ||
| .then((data: Movie[]) => { | ||
| setMovies(data); | ||
| setLoading(false); | ||
| }) | ||
| .catch(() => { | ||
| .catch((err: unknown) => { | ||
| console.error('Failed to fetch movies:', err); | ||
| setLoading(false); | ||
| }); | ||
|
|
||
| void fetch('/api/movies/meta/genres') | ||
| fetch('/api/movies/meta/genres') | ||
| .then(res => res.json()) | ||
| .then((data: string[]) => setGenres(data)) | ||
| .catch(() => {}); | ||
| .catch((err: unknown) => { | ||
| console.error('Failed to fetch genres:', err); | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
The useEffect hook initiates asynchronous operations (fetches) but does not provide a cleanup function. This can lead to attempts to update state on an unmounted component if the component unmounts before the fetches complete, which will cause a memory leak and a warning from React.
It's a best practice to clean up effects that involve subscriptions or async requests. You can use an AbortController to cancel the fetch requests when the component unmounts. This will also require checking for AbortError in your catch blocks to avoid logging canceled requests as errors.
useEffect(() => {
const controller = new AbortController();
const { signal } = controller;
fetch('/api/movies', { signal })
.then(res => res.json())
.then((data: Movie[]) => {
setMovies(data);
setLoading(false);
})
.catch((err: any) => {
if (err.name !== 'AbortError') {
console.error('Failed to fetch movies:', err);
setLoading(false);
}
});
fetch('/api/movies/meta/genres', { signal })
.then(res => res.json())
.then((data: string[]) => setGenres(data))
.catch((err: any) => {
if (err.name !== 'AbortError') {
console.error('Failed to fetch genres:', err);
}
});
return () => {
controller.abort();
};
}, []);
References docs/DESIGN_SYSTEM_GUIDE.md instead of the Memoria gist. Updates page-by-page breakdown and checklist to match the light green system (warm neutrals, green-to-teal gradient, Instrument Serif, DM Sans).
…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.
…ount (#7) * fix(Home): add AbortController cleanup to prevent memory leaks on unmount 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. * test(Home): assert AbortError is silently ignored, not logged as error 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. * chore: add coverage to .gitignore, delete stale docs copy files coverage/ directories were untracked. Also removes accidental *copy.md files that were left over from a previous session. * fix(Home): check res.ok before parsing JSON on both fetch calls 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. * test(Home): cover res.ok HTTP error check and update mock responses 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
.catch(() => {})on both fetch calls inHome.tsxwithconsole.errorlogging, so failures surface in dev tools and production logsvoidkeyword fromfetch(...)calls — flagged by Gemini as non-idiomatic (PR perf: remove blocking while loop, add useMemo and useCallback #5 inline comment, line 23)Tests
logs console.error when movies fetch fails— asserts the error message and error object are passed toconsole.errorlogs console.error when genres fetch fails— same for the genres endpointTest plan
pnpm --filter @stagepass/web test:coverage— 23/23 passing🤖 Generated with Claude Code