fix(dashboard): surface quota pool delete failures instead of failing silently - #8829
Merged
diegosouzapw merged 2 commits intoJul 28, 2026
Merged
Conversation
… silently
The handler awaited the DELETE and never looked at the response:
if (!confirm(t("removeConfirm"))) return;
await fetch(`/api/quota/pools/${id}`, { method: "DELETE" });
await mutate();
When the request failed — a 401 from an expired session, a 500, a dropped
connection — the page revalidated, the card stayed exactly where it was, and
nothing was shown. From the operator's side the click was indistinguishable
from a misclick, so the natural reaction is to click again. The page had no
error surface at all, unlike the sibling flow in the API manager which already
checked res.ok and rendered the message.
Adds a dismissible alert above the header, fed by both failure paths: a
non-ok response (appending the API's message when it sends one) and a thrown
request, which has no response to read. The alert clears on the next attempt,
so a transient failure does not leave a stale error on screen.
Reported against two boxes on 2026-07-28. The delete itself was working there
— the audit log shows the pools were removed — which is exactly what this
change makes visible either way.
Tests (vitest/jsdom, 5): non-ok response, thrown request, success path stays
quiet and still revalidates, a dismissed confirmation issues no DELETE at all,
and an earlier error clears once a later delete succeeds.
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
… silently (diegosouzapw#8829) The handler awaited the DELETE and never looked at the response: if (!confirm(t("removeConfirm"))) return; await fetch(`/api/quota/pools/${id}`, { method: "DELETE" }); await mutate(); When the request failed — a 401 from an expired session, a 500, a dropped connection — the page revalidated, the card stayed exactly where it was, and nothing was shown. From the operator's side the click was indistinguishable from a misclick, so the natural reaction is to click again. The page had no error surface at all, unlike the sibling flow in the API manager which already checked res.ok and rendered the message. Adds a dismissible alert above the header, fed by both failure paths: a non-ok response (appending the API's message when it sends one) and a thrown request, which has no response to read. The alert clears on the next attempt, so a transient failure does not leave a stale error on screen. Reported against two boxes on 2026-07-28. The delete itself was working there — the audit log shows the pools were removed — which is exactly what this change makes visible either way. Tests (vitest/jsdom, 5): non-ok response, thrown request, success path stays quiet and still revalidates, a dismissed confirmation issues no DELETE at all, and an earlier error clears once a later delete succeeds.
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.
The bug
handleRemovePoolawaited the DELETE and never looked at the response:When the request failed — a 401 from an expired session, a 500, a dropped connection — the page revalidated, the card stayed exactly where it was and nothing was shown. From the operator's side that is indistinguishable from a misclick, so the natural reaction is to click again.
The page had no error surface at all: no
pageError, no banner, no toast. The sibling flow in the API manager (handleRegenerateKey) already checksres.okand renders the message — this page never got the same treatment.The fix
A dismissible alert above the header, fed by both failure paths:
error.message/error/message), falling back to the generic string;The alert clears at the start of the next attempt, so a transient failure does not leave a stale error on screen after a later delete succeeds.
Two new keys in the
quotaSharenamespace (removeFailed,dismiss), added across all 43 locale files. I deliberately did not runi18n:sync-ui— it wanted to add 938 unrelated__MISSING__keys from other features, which does not belong in this diff. Non-Portuguese locales get the English source string.Origin
Reported against two boxes on 2026-07-28. Worth being explicit: on investigation the delete itself was working — the audit log shows
quota.pool.deletedfor both pools, and a clean-browser reproduction on the homolog box completed the flow end to end. This change does not fix the delete; it makes the outcome visible either way, which is what was missing when the operator concluded it had not worked.Tests
tests/unit/ui/quota-share-delete-error.test.tsx(vitest/jsdom, 5) — written first, confirmed red for the right reason (removeFailedabsent from the DOM):mutate();method: "DELETE", since the component legitimately fetches side data on mount;The pre-existing
quota-share-page.test.tsxstill passes (8/8).Gates
quota-share-delete-error.test.tsxquota-share-page.test.tsx(regression)typecheck:corelint:json --max-warnings 0ProviderAccountRoutingCard.tsx:87— untouched here. Not suppressed in this PR to avoid a duplicate diff: #8827 already adds that allowlist entry, so this goes green once that merges (or rebases onto it).