refactor: handle error results in sendStrictRequest - #2109
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the Web UI’s TanStack Query integration so hooks and service calls consistently rely on sendStrictRequest, allowing error results (including 200 responses carrying { error: ... }) to be surfaced via promise rejection and handled by TanStack Query’s error state rather than per-hook isError boilerplate.
Changes:
- Migrate multiple TanStack Query hooks and service endpoints to return “data-or-throw” by relying on
sendStrictRequestand removing hook-levelisErrorchecks. - Update admin/dashboard UI code to treat successful mutation responses as
SuccessResult(noas SuccessResultcasts). - Add/adjust unit tests to reflect rejection-based failures and to cover
sendStrictRequestbehavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| webui/test/unit/server-request.spec.ts | Adds unit coverage for sendStrictRequest and legacy sendNonRetriableRequest behavior. |
| webui/test/unit/pages/admin-dashboard/publisher-forget-user-button.spec.tsx | Updates test to expect rejection-based failures for strict requests. |
| webui/test/unit/components/extension/extension-version-delete-dialog.spec.tsx | Adds dialog tests expecting rejected removals and special handling for conflict (409). |
| webui/src/pages/user/extensions/use-user-extension.ts | Simplifies query function by removing isError boilerplate. |
| webui/src/pages/home/use-home-data.ts | Removes redundant error-result checks; relies on service rejection semantics. |
| webui/src/pages/admin-dashboard/use-publisher-admin.ts | Converts publisher admin mutations to direct service calls (reject-on-failure). |
| webui/src/pages/admin-dashboard/use-namespace-admin.ts | Simplifies namespace admin query/mutations to direct service calls. |
| webui/src/pages/admin-dashboard/use-extension-admin.ts | Simplifies extension admin query/mutation wrappers to direct service calls. |
| webui/src/pages/admin-dashboard/publisher-details.tsx | Treats mutation success response as typed SuccessResult (data.success). |
| webui/src/pages/admin-dashboard/namespace-change-dialog.tsx | Removes SuccessResult cast; uses result.success directly. |
| webui/src/pages/admin-dashboard/customers/use-customers.ts | Switches several customer mutations to direct service calls (no isError checks). |
| webui/src/hooks/use-infinite-search.ts | Simplifies infinite search query to return service result directly. |
| webui/src/extension-registry-service.ts | Migrates multiple endpoints to sendStrictRequest and narrows return types (drops ` |
| webui/.agents/skills/tanstack-query-conventions/SKILL.md | Updates internal guidance to reflect strict-request/error-handling conventions. |
| webui/.agents/skills/migrate-to-tanstack/SKILL.md | Updates migration guidance to reflect strict-request semantics and test expectations. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jordi Gómez Hidalgo <31970428+gnugomez@users.noreply.github.com>
sendRequest re-invoked itself without the caller's retry flag after waiting out a 429, silently falling back to the default retry=true. A sendStrictRequest/sendNonRetriableRequest call that hit a 429 could end up having its follow-up request retried again by fetch-retry itself, duplicating what the query client already owns. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Thomas Neidhart <thomas.neidhart@eclipse-foundation.org>
netomi
approved these changes
Aug 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is just a quality-live improvement of the react query implementation, typically react query should be a thin layer wrapping the fetch client that would translate the promise into react-state, initially we created the
sendNonRetriableRequestto allow react query to handle retries, that is working great, but we were manually throwing errors that the on every query/mutation hook.This patch now moves all Tanstack Query hooks to use sendStrictRequest, it remains non-retirable, but at the same time it throws errors as exceptions, so react query will be able to catch it and convert feed it to the error state variable without any further boilerplate code.