SEP-1845: Report failed UI actions in SEP's own component tree so a refused action is never silent

changelog.d/SEP-1845.fixed.md

@@ -0,0 +1 @@
+Failed UI actions now report the reason inline, from SEP's own component tree, instead of relying on a toast the host application has to provide: a refused action (HTTP 403 for a non-admin), a 409, a 5xx or a network failure all render the server's own message where the action was fired. Create and edit forms show a persistent banner, confirmation dialogs close on confirm and surface the failure on the page behind them, and stop-task, delete, execute, sync, connectivity-check and snippet approve/download failures are no longer silent.

frontend/packages/api/src/errors.ts

@@ -116,6 +116,52 @@ export function parseFieldErrors(error: unknown): FieldValidationError[] {
   return result;
 }
 
+/**
+ * Recover the server's reason from a request made with ``responseType: 'blob'``.
+ *
+ * Axios hands back the error body in the response type it was asked for, so a
+ * download's 403 arrives as a ``Blob`` rather than parsed JSON and
+ * ``messageFromPayload`` cannot see its ``detail`` — leaving the synthesized
+ * ``HTTP 403``. Read the blob, parse it, and return an ``ApiError`` carrying
+ * the reason (and the parsed payload, so a 422's ``detail`` array is still
+ * reachable through {@link parseFieldErrors}).
+ *
+ * Async by necessity — ``Blob.text()`` is a promise — so callers await it in
+ * their own catch rather than getting it from an interceptor.
+ *
+ * Falls back to the unmodified error whenever the body is not a readable JSON
+ * blob: an HTML error page, an opaque binary body, or a network failure with no
+ * response at all.
+ */
+export async function normalizeBlobError(error: unknown): Promise<ApiError> {
+  const apiError = normalizeAxiosError(error);
+  const body = apiError.data;
+  if (typeof Blob === 'undefined' || !(body instanceof Blob)) {
+    return apiError;
+  }
+
+  let payload: unknown;
+  try {
+    payload = JSON.parse(await body.text());
+  } catch {
+    return apiError;
+  }
+
+  const message = messageFromPayload(payload, apiError.message);
+  return new ApiError(
+    {
+      kind: apiError.kind,
+      status: apiError.status,
+      code: apiError.code,
+      message,
+      url: apiError.url,
+      method: apiError.method,
+      data: payload,
+    },
+    apiError.original,
+  );
+}
+
 export function normalizeAxiosError(error: unknown): ApiError {
   if (error instanceof ApiError) {
     return error;

frontend/packages/api/src/index.ts

@@ -34,7 +34,7 @@ export type { MintedToken } from './client';
 export { createQueryClient, defaultQueryClientConfig } from './queryClient';
 
 // Errors
-export { ApiError, normalizeAxiosError, parseFieldErrors } from './errors';
+export { ApiError, normalizeAxiosError, normalizeBlobError, parseFieldErrors } from './errors';
 export type { ApiErrorDetails, ApiErrorKind, FieldValidationError } from './errors';
 
 // Auth context (provider lives in @sep/shell; the context lives here so the

frontend/packages/api/tests/errors.test.ts

@@ -16,7 +16,7 @@
  */
 
 import { describe, expect, it } from 'vitest';
-import { ApiError, parseFieldErrors } from '../src/errors';
+import { ApiError, normalizeBlobError, parseFieldErrors } from '../src/errors';
 
 function http422(detail: unknown): ApiError {
   return new ApiError({ kind: 'http', status: 422, message: 'HTTP 422', data: { detail } });
@@ -75,3 +75,54 @@ describe('parseFieldErrors', () => {
     expect(parseFieldErrors('boom')).toEqual([]);
   });
 });
+
+describe('normalizeBlobError', () => {
+  function blobFailure(status: number, body: string, type = 'application/json'): ApiError {
+    return new ApiError({
+      kind: 'http',
+      status,
+      message: `HTTP ${status}`,
+      data: new Blob([body], { type }),
+      url: '/apps/snippets/snippet/download',
+      method: 'GET',
+    });
+  }
+
+  it("recovers a refusal's reason from a blob response body", async () => {
+    const recovered = await normalizeBlobError(
+      blobFailure(
+        403,
+        JSON.stringify({ detail: "You don't have permission to perform this action" }),
+      ),
+    );
+
+    expect(recovered.message).toBe("You don't have permission to perform this action");
+    expect(recovered.status).toBe(403);
+    expect(recovered.method).toBe('GET');
+  });
+
+  it("keeps a 422's detail array reachable through parseFieldErrors", async () => {
+    const recovered = await normalizeBlobError(
+      blobFailure(
+        422,
+        JSON.stringify({ detail: [{ loc: ['body', 'name'], msg: 'field required' }] }),
+      ),
+    );
+
+    expect(parseFieldErrors(recovered)).toEqual([{ path: 'name', message: 'field required' }]);
+  });
+
+  it('leaves the error untouched when the body is not JSON', async () => {
+    const original = blobFailure(502, '<html>Bad gateway</html>', 'text/html');
+
+    const recovered = await normalizeBlobError(original);
+
+    expect(recovered.message).toBe('HTTP 502');
+  });
+
+  it('passes through an error with no blob body', async () => {
+    const original = new ApiError({ kind: 'network', message: 'Network error' });
+
+    expect(await normalizeBlobError(original)).toBe(original);
+  });
+});

frontend/packages/apps/dipper/src/DipperApp.test.tsx

@@ -25,8 +25,9 @@ import {
   useDipperAppSchema,
 } from './hooks';
 
-const { stopMutate, authMock } = vi.hoisted(() => ({
+const { stopMutate, stopState, authMock } = vi.hoisted(() => ({
   stopMutate: vi.fn(),
+  stopState: { error: null as unknown },
   /** Flipped per test to cover the read-only (non-admin) rendering. */
   authMock: { canMutate: true },
 }));
@@ -82,10 +83,12 @@ vi.mock('@sep/framework', async () => {
       data,
       onViewLogs,
       onStopTask,
+      actionError,
     }: {
       data: Array<{ id: number; status: string; task?: { name: string } }>;
       onViewLogs: (entry: unknown) => void;
       onStopTask?: (entry: { id: number }) => void;
+      actionError?: unknown;
     }) => (
       <div>
         <span>history rows: {data.length}</span>
@@ -97,12 +100,24 @@ vi.mock('@sep/framework', async () => {
             Stop {String(data[0].id)}
           </button>
         ) : null}
+        {actionError ? (
+          <div data-testid="task-history-action-error">
+            {actionError instanceof Error ? actionError.message : String(actionError)}
+          </div>
+        ) : null}
       </div>
     ),
     TaskLogViewer: ({ taskHistoryId }: { taskHistoryId: number }) => (
       <div>logs for {taskHistoryId}</div>
     ),
-    useStopTaskHistory: () => ({ mutate: stopMutate, isPending: false }),
+    useStopTaskHistory: () => ({
+      mutate: stopMutate,
+      isPending: false,
+      error: stopState.error,
+      reset: () => {
+        stopState.error = null;
+      },
+    }),
     ReadOnlyNotice: ({ action, testId }: { action?: string; testId?: string }) => (
       <div data-testid={testId}>no permission to {action}</div>
     ),
@@ -119,6 +134,7 @@ describe('DipperApp', () => {
 
   beforeEach(() => {
     vi.clearAllMocks();
+    stopState.error = null;
     authMock.canMutate = true;
     mockAppSchema.mockReturnValue({
       data: {
@@ -206,6 +222,67 @@ describe('DipperApp', () => {
       42,
       expect.objectContaining({ onSuccess: expect.any(Function) }),
     );
+    expect(screen.queryByTestId('task-history-action-error')).not.toBeInTheDocument();
+  });
+
+  it("reports a failed stop above the history with the server's own reason", () => {
+    stopState.error = new Error("You don't have permission to perform this action");
+
+    render(<DipperApp />);
+
+    expect(screen.getByTestId('task-history-action-error')).toHaveTextContent(
+      "You don't have permission to perform this action",
+    );
+  });
+});
+
+describe('DipperApp — write access', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    stopState.error = null;
+    authMock.canMutate = true;
+    mockAppSchema.mockReturnValue({
+      data: { display_name: 'Dipper', description: 'Collect' },
+      isLoading: false,
+      error: null,
+    } as unknown as ReturnType<typeof useDipperAppSchema>);
+    mockFormSchema.mockReturnValue({
+      data: { forms: [] },
+      isLoading: false,
+      error: null,
+    } as unknown as ReturnType<typeof useDipperFormSchema>);
+    mockHistory.mockReturnValue({
+      data: { items: [] },
+      isLoading: false,
+      error: null,
+      refetch: vi.fn(),
+    } as unknown as ReturnType<typeof useDipperHistory>);
+    mockExecution.mockReturnValue({
+      mutate: vi.fn(),
+      isPending: false,
+      isError: false,
+      error: null,
+    } as unknown as ReturnType<typeof useDipperExecution>);
+  });
+
+  it('renders the execute form for a session that may mutate', () => {
+    render(<DipperApp />);
+
+    fireEvent.click(screen.getByRole('button', { name: 'Select service' }));
+
+    expect(screen.getByRole('button', { name: 'Execute' })).toBeInTheDocument();
+    expect(screen.queryByTestId('dipper-execute-read-only')).not.toBeInTheDocument();
+  });
+
+  it('renders no execute form for a non-admin, keeping the history readable', () => {
+    authMock.canMutate = false;
+    render(<DipperApp />);
+
+    fireEvent.click(screen.getByRole('button', { name: 'Select service' }));
+
+    expect(screen.getByTestId('dipper-execute-read-only')).toBeInTheDocument();
+    expect(screen.queryByRole('button', { name: 'Execute' })).not.toBeInTheDocument();
+    expect(screen.getByText('Execution history')).toBeInTheDocument();
   });
 });
 

frontend/packages/apps/dipper/src/DipperApp.tsx

@@ -235,6 +235,8 @@ export function DipperApp() {
             }
           }}
           isStopping={stop.isPending}
+          actionError={stop.error}
+          onDismissActionError={stop.reset}
         />
       )}
 

frontend/packages/apps/inventory/src/ConnectivityControl.test.tsx

@@ -144,12 +144,34 @@ describe('ConnectivityControl', () => {
     await screen.findByText(/missing node or port information/i);
   });
 
-  it('shows a generic snackbar when the request never reaches the server', async () => {
+  it('reports in-tree when the request never reaches the server', async () => {
     const user = userEvent.setup();
     vi.spyOn(apiClient, 'post').mockRejectedValue(new Error('network down'));
     renderControl('mysql');
     await user.click(await screen.findByRole('button', { name: CHECK_BUTTON }));
-    await screen.findByText(/could not be started/i);
+    expect(await screen.findByTestId('connectivity-action-error')).toHaveTextContent(
+      /network down/i,
+    );
+  });
+
+  it("reports a refusal in-tree with the server's own reason", async () => {
+    const user = userEvent.setup();
+    await stubPostError(403, "You don't have permission to perform this action");
+    renderControl('mysql');
+    await user.click(await screen.findByRole('button', { name: CHECK_BUTTON }));
+    expect(await screen.findByTestId('connectivity-action-error')).toHaveTextContent(
+      "You don't have permission to perform this action",
+    );
+  });
+
+  it('reports nothing when the probe connects', async () => {
+    const user = userEvent.setup();
+    stubPostResult({ success: true });
+    renderControl('mysql');
+    await user.click(await screen.findByRole('button', { name: CHECK_BUTTON }));
+    await screen.findByText(/connectivity check passed/i);
+    expect(screen.queryByTestId('connectivity-action-error')).not.toBeInTheDocument();
+    expect(screen.queryByTestId('connectivity-probe-failure')).not.toBeInTheDocument();
   });
 
   it('re-enables the button after a failed check so it can be retried', async () => {

frontend/packages/apps/inventory/src/ConnectivityControl.tsx

@@ -15,9 +15,11 @@
  * along with this program. If not, see <https://www.gnu.org/licenses/>.
  */
 
-import { Box, Button, CircularProgress, Tooltip } from '@mui/material';
+import { useState } from 'react';
+import { Alert, Box, Button, CircularProgress, Tooltip } from '@mui/material';
 import NetworkCheckIcon from '@mui/icons-material/NetworkCheck';
-import { ApiError, useAuth } from '@sep/api';
+import { useAuth } from '@sep/api';
+import { ActionErrorAlert, useActionError } from '@sep/framework';
 import { useSnackbar } from 'notistack';
 import { useCheckServiceConnectivity } from './hooks';
 
@@ -45,29 +47,29 @@ export function ConnectivityControl({
   const { canMutate } = useAuth();
   const checkConnectivity = useCheckServiceConnectivity(serviceId);
   const { enqueueSnackbar } = useSnackbar();
+  const checkError = useActionError();
+  const [probeFailure, setProbeFailure] = useState<string | null>(null);
 
   const isConnectable =
     typeof serviceType === 'string' && CONNECTABLE_SERVICE_TYPES.has(serviceType);
   const isPending = checkConnectivity.isPending;
 
   function handleCheck() {
+    setProbeFailure(null);
+    checkError.clearError();
     checkConnectivity.mutate(undefined, {
       onSuccess: (result) => {
         if (result.success) {
           enqueueSnackbar('Connectivity check passed', { variant: 'success' });
         } else {
-          enqueueSnackbar(`Connectivity check failed: ${result.error ?? 'Unknown error'}`, {
-            variant: 'error',
-          });
+          // A reachability failure is a successful request reporting a bad
+          // result, so it renders here rather than through the error alert.
+          setProbeFailure(result.error ?? 'Unknown error');
         }
       },
-      onError: (err) => {
-        const message =
-          err instanceof ApiError
-            ? err.message
-            : 'Connectivity check could not be started. Please try again.';
-        enqueueSnackbar(message, { variant: 'error' });
-      },
+      // The request itself failing (refused, 5xx, network) reports in-tree and
+      // nowhere else: one signal, and one that does not need a host snackbar.
+      onError: (err) => checkError.reportError(err),
     });
   }
 
@@ -96,6 +98,25 @@ export function ConnectivityControl({
           </Button>
         </Box>
       </Tooltip>
+
+      <ActionErrorAlert
+        error={checkError.error}
+        onClose={checkError.clearError}
+        fallback="Connectivity check could not be started. Please try again."
+        sx={{ mt: 2 }}
+        testId="connectivity-action-error"
+      />
+
+      {probeFailure !== null && (
+        <Alert
+          severity="error"
+          onClose={() => setProbeFailure(null)}
+          sx={{ mt: 2 }}
+          data-testid="connectivity-probe-failure"
+        >
+          Connectivity check failed: {probeFailure}
+        </Alert>
+      )}
     </Box>
   );
 }