lxbme opened a new pull request, #3426:
URL: https://github.com/apache/apisix-dashboard/pull/3426
**Why submit this pull request?**
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
**What changes will this PR take into?**
### Problem
This addresses one item from the tracking issue #3417 (Error handling /
resilience):
> `secrets/detail.$manager.$id.tsx` is the only non-suspense detail page; on
fetch failure it renders a blank editable form where Save would PUT an empty
object over the real secret, rather than displaying an error state.
Root cause: the page is the only detail page (1 of 12) on plain `useQuery`,
gated by `if (isLoading)` alone. In react-query v5, `isLoading` becomes `false`
after a failed fetch while `isError` is never checked, so the failure path
falls through to the form renderer with `secretData === undefined`: the
`useEffect` skips `form.reset` and the user gets a fully blank form that
becomes editable via the Edit button.
### Verified severity (tested against a live Admin API)
- A fully blank submit is actually blocked twice — the zod discriminated
union rejects an empty `manager`, and APISIX-side schema validation rejects
empty strings (e.g. the vault `uri` pattern) — so the tracking issue's literal
claim is slightly overstated for the all-blank case.
- The real data-loss path is indirect: a transient fetch failure shows a
blank form that reads as "this secret is empty/lost"; a user who re-enters
plausible values (e.g. a stale token) passes all validation and silently
overwrites the production secret.
- A guaranteed harm needs no user action at all: any transient network error
makes the page misrepresent existing data as empty, and a later focus-triggered
refetch that succeeds will `form.reset` over anything typed meanwhile.
### Change
Two-line conversion in `src/routes/secrets/detail.$manager.$id.tsx`,
mirroring `ssls/detail.$id.tsx` (the closest sibling): `useQuery` →
`useSuspenseQuery`. Data is now guaranteed present at render time, so the
blank-form class is eliminated structurally; a fetch failure throws to the root
error page added by #3418 (`RootError`: "Something went wrong" + error message
+ Retry with query-error-reset — its comment explicitly anticipates detail
pages throwing from `useSuspenseQuery`).
Everything else stays sibling-identical: the `isLoading` skeleton (covers
refetch states under suspense), the `form.reset` effect, and the mutation. No
explicit `<Suspense>` wrapper, matching ssls.
### Tests
New e2e spec `e2e/tests/regression/secrets.detail-fetch-error.spec.ts`
(fault injection via `route.abort('failed')`, written first and verified
failing against the old behavior):
- creates a real vault secret via the Admin API, aborts only the detail GET,
opens the page;
- asserts the root error page appears (title + Retry button; generous
timeout because react-query retries 3 times before surfacing the error);
- asserts the blank editable form is unreachable (no Edit button, no URI
field);
- asserts the backend secret is untouched afterwards.
Also ran locally: all secrets suites (crud-all-fields, crud-required-fields,
list) and the full `e2e/tests/regression/` folder — all green.
**Related issues**
Part of #3417 (item: "secrets detail is the only non-suspense detail page"
under Error handling / resilience). Does **not** close the tracking issue.
**Checklist:**
- [x] Did you explain what problem does this PR solve? Or what new features
have been added?
- [x] Have you added corresponding test cases?
- [ ] Have you modified the corresponding document?
- [x] Is this PR backward compatible? If it is not backward compatible,
please discuss on the mailing list first
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
