🧹 Refactor duplicated response parsing logic#20
Conversation
- Extract manual fetch response checking and JSON parsing into `handleResponse` in `src/lib/utils.ts`. - Update `src/app/revoke/page.tsx` to use the new utility in `handleRevoke` and `handleRotate`. - Update `src/components/PasteEditor.tsx` to use the new utility in `handleCreate`. - Use TypeScript generics to maintain type safety for returned data. - Improve error handling with robust JSON parsing and fallback messages. Co-authored-by: instax-dutta <54683866+instax-dutta@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughA refactoring that extracts shared response parsing and error handling logic into a new utility function Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/utils.ts`:
- Around line 15-19: The code around the response.json() try/catch swallows JSON
parse errors (in the block where data = await response.json()), and later
returns an empty object cast to T, which hides parsing failures; update the
logic in the function handling the fetch/response (reference variables:
response, data) to surface JSON parse errors instead of swallowing them — if
response.ok but response.json() throws, capture the parsing error and throw a
new Error (or reject) that includes the original error message and response
metadata (status/headers/body text) so callers see a clear parsing/fetch failure
rather than receiving an empty {} cast to T.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2d88a6f-e5a3-4de0-a630-10b444e2482c
📒 Files selected for processing (3)
src/app/revoke/page.tsxsrc/components/PasteEditor.tsxsrc/lib/utils.ts
| try { | ||
| data = await response.json(); | ||
| } catch (e) { | ||
| // If JSON parsing fails despite the content-type, we'll stick with the default error | ||
| } |
There was a problem hiding this comment.
Do not swallow JSON parse failures on successful responses.
Line 15-19 currently ignores parse errors and Line 26 returns an empty object cast to T. That can surface as downstream runtime errors (e.g., missing required fields) instead of a clear fetch/parsing failure at the source.
Proposed fix
export async function handleResponse<T>(response: Response, defaultErrorMessage: string): Promise<T> {
- let data: any = {};
- const contentType = response.headers.get('content-type');
- if (contentType && contentType.includes('application/json')) {
+ let data: any = {};
+ const contentType = response.headers.get('content-type');
+ const isJson = Boolean(contentType && contentType.includes('application/json'));
+
+ if (isJson) {
try {
data = await response.json();
- } catch (e) {
- // If JSON parsing fails despite the content-type, we'll stick with the default error
+ } catch {
+ if (response.ok) {
+ throw new Error(defaultErrorMessage);
+ }
}
}
if (!response.ok) {
throw new Error(data.error || defaultErrorMessage);
}
+ if (!isJson) {
+ throw new Error(defaultErrorMessage);
+ }
+
return data as T;
}Also applies to: 26-26
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils.ts` around lines 15 - 19, The code around the response.json()
try/catch swallows JSON parse errors (in the block where data = await
response.json()), and later returns an empty object cast to T, which hides
parsing failures; update the logic in the function handling the fetch/response
(reference variables: response, data) to surface JSON parse errors instead of
swallowing them — if response.ok but response.json() throws, capture the parsing
error and throw a new Error (or reject) that includes the original error message
and response metadata (status/headers/body text) so callers see a clear
parsing/fetch failure rather than receiving an empty {} cast to T.
1 participant
🎯 What: The code health issue addressed was duplicated response parsing logic across multiple client-side components. I extracted this pattern into a new
handleResponsehelper function insrc/lib/utils.ts.💡 Why: This improves maintainability by centralizing the logic for status checking, content-type verification, and error extraction from fetch responses. It prevents drift and makes the calling code cleaner and easier to read.
✅ Verification:
handleResponseusing the nativenode:testrunner.handleResponsecorrectly parses JSON on success and throws descriptive errors on failure.node --experimental-strip-types --check.✨ Result: Reduced code duplication and established a consistent pattern for client-side API interactions.
PR created automatically by Jules for task 2327660267229171752 started by @instax-dutta
Summary by CodeRabbit