Skip to content

ui-react: delete confirm dialogs silently swallow mutation errors #6165

@luizhf42

Description

@luizhf42

Summary

When a user confirms a destructive action in the React UI, the mutation's error is silently swallowed. On failure the confirmation dialog stays open with no visible explanation — no toast, no inline error, no alert. The user sees nothing happen and has to guess whether their click registered, whether the network dropped, or whether they lack permission.

Affected pages

  • ui-react/apps/console/src/pages/firewall-rules/index.tsx — delete firewall rule
  • ui-react/apps/console/src/pages/public-keys/index.tsx — delete public key
  • ui-react/apps/console/src/pages/team/ApiKeysTab.tsx — delete API key

The same bug pattern may exist in other pages that use ConfirmDialog with an uncaught await mutateAsync inside onConfirm — worth grepping as part of the fix.

Root cause

ui-react/apps/console/src/components/common/ConfirmDialog.tsx:77-87 intentionally swallows errors thrown from onConfirm and documents that consumers are expected to pre-catch:

const handleConfirm = async () => {
  setConfirming(true);
  try {
    await onConfirm();
  } catch {
    // Errors are not surfaced here. Consumers manage their own error state
    // and should pass an already-caught onConfirm handler (e.g. KeyDeleteDialog).
  } finally {
    setConfirming(false);
  }
};

The three pages above violate that contract. Example from firewall-rules/index.tsx:320-324:

onConfirm={async () => {
  await deleteRule.mutateAsync({ path: { id: deleteTarget!.id } });
  if (rules.length === 1 && page > 1) setPage(page - 1);
  setDeleteTarget(null);
}}

On mutation rejection the await throws, the success-path lines (page decrement, setDeleteTarget(null)) never run, the exception propagates to ConfirmDialog, gets swallowed, the spinner stops, and the dialog stays open with no error shown.

Reference implementation

ui-react/apps/console/src/pages/secure-vault/KeyDeleteDialog.tsx already does this correctly — it keeps local error state, wraps the mutation in a try/catch, and renders the error as a child of ConfirmDialog:

const [error, setError] = useState<string | null>(null);

const handleConfirm = async () => {
  setError(null);
  try {
    await removeKey(entry.id);
    onClose();
  } catch (err) {
    setError(err instanceof Error ? err.message : "Failed to delete key.");
  }
};

// …

<ConfirmDialog  onConfirm={handleConfirm}>
  {error && <p className="text-xs text-accent-red">{error}</p>}
</ConfirmDialog>

Proposed fix

Mirror the KeyDeleteDialog pattern in each of the three affected pages: add local error state, wrap the mutation in try/catch, render the error via ConfirmDialog's children prop, and only run the success-path code (page decrement, closing the dialog) inside the success branch.

There is no global toast/snackbar system in ui-react yet, so this is the smallest self-contained fix. Introducing a toast system is a larger, independent piece of work that would benefit this and every other mutation in the app — worth considering as a follow-up but out of scope for this issue.

Acceptance criteria

  • Deleting a firewall rule, public key, or API key while the API is unreachable (or returns 4xx/5xx) displays a human-readable error inside the confirmation dialog.
  • On error, the dialog stays open with the error visible and the confirm button re-enabled so the user can retry or cancel.
  • On success, the existing behavior (page decrement when the last row of a page is deleted, dialog close) is preserved.
  • New or updated vitest coverage asserts the error-visible state — mock the mutation hook to reject and assert the error string is rendered inside the dialog.

Notes for whoever picks this up

  • Grep mutateAsync inside onConfirm={ across apps/console/src/pages to find any other sites with the same pattern before closing.
  • The existing KeyDeleteDialog.tsx is the north-star reference — copy its shape, don't reinvent.
  • The error string from the React Query mutation's rejection is whatever the axios/fetch layer throws; err instanceof Error ? err.message : "Failed to …" is the fallback KeyDeleteDialog already uses and is fine to keep.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/uijavascriptPull requests that update Javascript codekind/bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions