fix(graphql): handle partial failure errors gracefully with localized fallbacks (fixes #1626) - #2412
Conversation
github-actions
Bot
added
bug
📝 WalkthroughWalkthroughThe PR adds a shared GraphQL client with typed errors, partial-data handling, configurable requests, and OpenTelemetry reporting. The cursor-events hook re-exports it. The admin users route renders available profile data when GraphQL returns partial errors. ChangesGraphQL partial-error handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant AdminUsers
participant fetchGraphQL
participant GraphQLAPI
participant OpenTelemetry
AdminUsers->>fetchGraphQL: Request profile data
fetchGraphQL->>GraphQLAPI: POST GraphQL query
GraphQLAPI-->>fetchGraphQL: Return data and optional errors
fetchGraphQL->>OpenTelemetry: Record GraphQL errors
fetchGraphQL-->>AdminUsers: Return data or GraphQLPartialError
AdminUsers-->>AdminUsers: Render available data and show warning
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
@krushit1307 The implementation for handling partial GraphQL errors is complete and ready for review! 🚀 Technical Analysis of Changes
ECSoC26 JustificationThis PR represents a Level 3 (Core/Arch/Perf) contribution because it directly modifies the foundational data-fetching pipeline and error-handling architecture across the frontend client. It addresses a core architectural flaw where a minor localized resolver timeout in the backend would cascade into a fatal Please add the following labels if you agree:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib/graphql-client.ts`:
- Around line 35-38: Use one partial-response contract across the client, hook,
and tests: in src/lib/graphql-client.ts lines 35-38 retain GraphQLPartialError,
keep its `@throws` documentation accurate at lines 84-86, and throw
GraphQLPartialError with both json.errors and json.data at lines 116-122. Update
src/lib/graphql-client.test.ts lines 56-70 to assert the typed rejection and
both payloads. In src/hooks/useCursorEventsQuery.ts line 96, adapt
GraphQLPartialError by returning its data to React Query, and update
src/hooks/useCursorEventsQuery.test.ts lines 81-110 to verify partial-data
recovery through that adapter.
In `@src/routes/admin.users.tsx`:
- Around line 131-136: Update the GraphQLPartialError handling in the route’s
data-loading flow to track profiles and totalProfiles as nullable partial
fields, clearing each failed field instead of retaining prior state. Apply safe
replacements for missing fields, such as an empty profile list and a neutral
pagination total, and render section-specific fallbacks while preserving
successfully returned partial data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5bbbb8bd-e02f-4da0-b52b-1928c5556d2a
📒 Files selected for processing (5)
src/hooks/useCursorEventsQuery.test.tssrc/hooks/useCursorEventsQuery.tssrc/lib/graphql-client.test.tssrc/lib/graphql-client.tssrc/routes/admin.users.tsx
| /** | ||
| * A custom error class carrying the partial `data` alongside the | ||
| * GraphQL `errors` array. Components can check | ||
| * `instanceof GraphQLPartialError` and decide to render partial UI. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use one partial-response contract across the client, hook, and tests.
The shared documentation requires GraphQLPartialError, but the implementation and tests use normal resolution. This prevents callers from receiving both partial data and error details.
src/lib/graphql-client.ts#L35-L38: retain the documented typed-error contract.src/lib/graphql-client.ts#L84-L86: keep the@throws GraphQLPartialErrorcontract accurate.src/lib/graphql-client.ts#L116-L122: thrownew GraphQLPartialError(json.errors, json.data).src/lib/graphql-client.test.ts#L56-L70: assert the typed rejection and both stored payloads.src/hooks/useCursorEventsQuery.ts#L96-L96: add an adapter that returnsGraphQLPartialError.datafor React Query.src/hooks/useCursorEventsQuery.test.ts#L81-L110: test the adapter’s partial-data recovery instead of the shared client’s incorrect resolution behavior.
📍 Affects 4 files
src/lib/graphql-client.ts#L35-L38(this comment)src/lib/graphql-client.ts#L84-L86src/lib/graphql-client.ts#L116-L122src/lib/graphql-client.test.ts#L56-L70src/hooks/useCursorEventsQuery.ts#L96-L96src/hooks/useCursorEventsQuery.test.ts#L81-L110
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/lib/graphql-client.ts` around lines 35 - 38, Use one partial-response
contract across the client, hook, and tests: in src/lib/graphql-client.ts lines
35-38 retain GraphQLPartialError, keep its `@throws` documentation accurate at
lines 84-86, and throw GraphQLPartialError with both json.errors and json.data
at lines 116-122. Update src/lib/graphql-client.test.ts lines 56-70 to assert
the typed rejection and both payloads. In src/hooks/useCursorEventsQuery.ts line
96, adapt GraphQLPartialError by returning its data to React Query, and update
src/hooks/useCursorEventsQuery.test.ts lines 81-110 to verify partial-data
recovery through that adapter.
| // Partial failure: render what we got, warn the user | ||
| if (err instanceof GraphQLPartialError) { | ||
| const partial = err.data as GraphQLResponse; | ||
| if (partial?.profiles) setProfiles(partial.profiles); | ||
| if (partial?.totalProfiles != null) setTotal(partial.totalProfiles); | ||
| toast.warning("Some user data failed to load. Showing partial results."); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear failed sections instead of retaining stale results.
If the profiles resolver fails, this branch retains rows from the previous page or sort. If totalProfiles fails, it retains the previous pagination total.
Represent partial fields as nullable. Set a safe replacement for each failed field and render a section-specific fallback.
Proposed state handling
- const partial = err.data as GraphQLResponse;
- if (partial?.profiles) setProfiles(partial.profiles);
- if (partial?.totalProfiles != null) setTotal(partial.totalProfiles);
+ const partial = err.data as Partial<GraphQLResponse>;
+ setProfiles(partial.profiles ?? []);
+ setTotal(partial.totalProfiles ?? 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Partial failure: render what we got, warn the user | |
| if (err instanceof GraphQLPartialError) { | |
| const partial = err.data as GraphQLResponse; | |
| if (partial?.profiles) setProfiles(partial.profiles); | |
| if (partial?.totalProfiles != null) setTotal(partial.totalProfiles); | |
| toast.warning("Some user data failed to load. Showing partial results."); | |
| // Partial failure: render what we got, warn the user | |
| if (err instanceof GraphQLPartialError) { | |
| const partial = err.data as Partial<GraphQLResponse>; | |
| setProfiles(partial.profiles ?? []); | |
| setTotal(partial.totalProfiles ?? 0); | |
| toast.warning("Some user data failed to load. Showing partial results."); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/admin.users.tsx` around lines 131 - 136, Update the
GraphQLPartialError handling in the route’s data-loading flow to track profiles
and totalProfiles as nullable partial fields, clearing each failed field instead
of retaining prior state. Apply safe replacements for missing fields, such as an
empty profile list and a neutral pagination total, and render section-specific
fallbacks while preserving successfully returned partial data.
|
Hi @krushit1307, I noticed this PR was merged/closed but seems to be missing the |
|
Hi @krushit1307! Thank you so much for reviewing and merging this PR! As this PR contributes substantial feature implementation and code quality enhancements, could you please add the applicable ECSoC '26 labels to this PR when you have a moment?
Thank you for your time and for maintaining this repository! |
2 participants
Pull Request
Description
This pull request introduces centralized GraphQL partial failure handling. Instead of swallowing GraphQL
errorsinto a pure success state (which caused components to blindly map overnullvalues and crash to an Error Boundary), the sharedfetchGraphQLutility now appropriately throws a customGraphQLPartialError. Callers (such asuseCursorEventsQueryandadmin.users.tsx) now gracefully catch this error and render the available partial data with proper localized fallbacks.Type of Change
Related Issue
Closes #1626
Testing
Describe the testing performed.
Comprehensive tests were added for the new
fetchGraphQLclient, validating it against partial successes, complete network errors, and standard GraphQL payloads. The test foruseCursorEventsQuerywas updated to explicitly mock and handle a partial GraphQL failure gracefully.Screenshots
N/A
Checklist
Summary by CodeRabbit