From d40cf0df4b160b4d74a4fd5939b43601f139a401 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 23:44:08 +0000 Subject: [PATCH 1/2] fix(frontend): give API transport failures messages users can act on The API client built its errors from developer text: a bare "HTTP 502" when the server sent no message, "Request to /api/x timed out after 90000ms" on timeout, the browser's "Failed to fetch" when the network dropped, and "Unexpected token '<'" when a proxy answered a 2xx with an HTML page. Dozens of toasts and inline errors interpolate err.message, so users saw that text with no next step. Login, password reset and first-admin setup call fetch() directly and had the same problems; setupAdmin parsed a 502 HTML body with no catch. The new api/errors.ts maps each case to a sentence (can't reach CUDly, took too long, temporarily unavailable, unexpected response, plus 401, 403, 404 and 429 wording). The client uses it at the source, so every existing caller benefits without changes, and the direct-fetch auth calls use the same helpers. A specific server message still wins (and the MFA sentinel codes still work); only the backend's catch-all "Internal server error" is replaced. The raw text stays on the new ApiError.detail field for support, statuses are unchanged (0 for transport failures), and a caller's own abort is rethrown untouched. Refs #748 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/api-errors.test.ts | 86 ++++++++++++++++ frontend/src/__tests__/api.test.ts | 77 +++++++++++++- frontend/src/api/auth.ts | 33 +++--- frontend/src/api/client.ts | 41 ++++++-- frontend/src/api/errors.ts | 119 ++++++++++++++++++++++ frontend/src/api/types.ts | 4 + 6 files changed, 331 insertions(+), 29 deletions(-) create mode 100644 frontend/src/__tests__/api-errors.test.ts create mode 100644 frontend/src/api/errors.ts diff --git a/frontend/src/__tests__/api-errors.test.ts b/frontend/src/__tests__/api-errors.test.ts new file mode 100644 index 000000000..1936e9ff9 --- /dev/null +++ b/frontend/src/__tests__/api-errors.test.ts @@ -0,0 +1,86 @@ +/** + * Issue #748: user-facing wording for API and transport failures. + */ +import { + BAD_RESPONSE_MESSAGE, + NETWORK_ERROR_MESSAGE, + friendlyError, + httpStatusMessage, + isGenericServerMessage, + isNetworkFailure, + readJson, + responseError, +} from '../api/errors'; + +function response(status: number, body: () => Promise): Response { + return { ok: status < 400, status, url: '/api/auth/login', json: body } as unknown as Response; +} + +describe('isNetworkFailure', () => { + test.each(['Failed to fetch', 'NetworkError when attempting to fetch resource.', 'Load failed'])( + 'recognizes the browser fetch TypeError "%s"', (msg) => { + expect(isNetworkFailure(new TypeError(msg))).toBe(true); + }); + + test('ignores other errors', () => { + expect(isNetworkFailure(new TypeError('x is not a function'))).toBe(false); + expect(isNetworkFailure(new Error('Failed to fetch'))).toBe(false); + }); +}); + +describe('friendlyError', () => { + test('maps a raw fetch failure and a JSON parse error', () => { + expect(friendlyError(new TypeError('Failed to fetch'))).toBe(NETWORK_ERROR_MESSAGE); + expect(friendlyError(new SyntaxError("Unexpected token '<'"))).toBe(BAD_RESPONSE_MESSAGE); + }); + + test('keeps an error message the server or client already made readable', () => { + expect(friendlyError(Object.assign(new Error('plan name already in use'), { status: 409 }))).toBe('plan name already in use'); + }); + + test('falls back to the status text, then a generic sentence', () => { + expect(friendlyError(Object.assign(new Error(''), { status: 503 }))).toBe(httpStatusMessage(503)); + expect(friendlyError(undefined)).toBe('Something went wrong. Try again in a moment.'); + expect(friendlyError('quota exceeded')).toBe('quota exceeded'); + }); +}); + +describe('isGenericServerMessage', () => { + test('matches only the backend catch-all', () => { + expect(isGenericServerMessage('Internal server error')).toBe(true); + expect(isGenericServerMessage('internal server error.')).toBe(true); + expect(isGenericServerMessage('internal server error: db down')).toBe(false); + }); +}); + +describe('responseError', () => { + test('prefers a specific server error, then message', async () => { + await expect(responseError(response(400, () => Promise.resolve({ error: 'bad email' })), 'Login failed')) + .resolves.toMatchObject({ message: 'bad email', status: 400 }); + await expect(responseError(response(400, () => Promise.resolve({ message: 'weak password' })), 'x')) + .resolves.toMatchObject({ message: 'weak password' }); + }); + + test('uses the friendly status text for a 5xx with no or a generic body', async () => { + await expect(responseError(response(502, () => Promise.reject(new SyntaxError('<'))), 'Login failed')) + .resolves.toMatchObject({ message: httpStatusMessage(502), status: 502, detail: 'HTTP 502 from /api/auth/login' }); + await expect(responseError(response(500, () => Promise.resolve({ error: 'Internal server error' })), 'Login failed')) + .resolves.toMatchObject({ message: httpStatusMessage(500) }); + }); + + test('uses the caller fallback for a 4xx with no message', async () => { + await expect(responseError(response(401, () => Promise.resolve({})), 'Login failed')) + .resolves.toMatchObject({ message: 'Login failed', status: 401 }); + }); +}); + +describe('readJson', () => { + test('returns the parsed body', async () => { + await expect(readJson(response(200, () => Promise.resolve({ a: 1 })))).resolves.toEqual({ a: 1 }); + }); + + test('turns a parse failure into friendly text', async () => { + await expect(readJson(response(200, () => Promise.reject(new SyntaxError("Unexpected token '<'"))))) + .rejects.toMatchObject({ message: BAD_RESPONSE_MESSAGE, status: 200 }); + }); +}); diff --git a/frontend/src/__tests__/api.test.ts b/frontend/src/__tests__/api.test.ts index af7174aca..0f3716fc2 100644 --- a/frontend/src/__tests__/api.test.ts +++ b/frontend/src/__tests__/api.test.ts @@ -403,7 +403,12 @@ describe('API Requests', () => { status: 200, json: () => Promise.reject(new SyntaxError('Unexpected token in JSON at position 0')), }); - await expect(apiRequest('/test')).rejects.toThrow(SyntaxError); + // Issue #748: rejects with friendly text, not "Unexpected token ...". + await expect(apiRequest('/test')).rejects.toMatchObject({ + message: 'CUDly sent an unexpected response. Try again in a moment.', + status: 200, + detail: expect.stringContaining('Unexpected token in JSON'), + }); }); test('surfaces a timeout error when the backend hangs past timeoutMs (issue #20)', async () => { @@ -418,7 +423,56 @@ describe('API Requests', () => { }); })); - await expect(apiRequest('/test', { timeoutMs: 10 })).rejects.toThrow(/timed out after 10ms/); + // Issue #748: the user sees a sentence; the raw timing stays on detail. + await expect(apiRequest('/test', { timeoutMs: 10 })).rejects.toMatchObject({ + message: 'CUDly took too long to respond. Try again in a moment.', + status: 0, + detail: 'Request to /test timed out after 10ms', + }); + }); + + describe('friendly transport errors (issue #748)', () => { + test('a network failure becomes "Can\'t reach CUDly" with status 0', async () => { + fetchMock.mockRejectedValue(new TypeError('Failed to fetch')); + await expect(apiRequest('/test')).rejects.toMatchObject({ + message: "Can't reach CUDly. Check your connection and try again.", + status: 0, + detail: 'Request to /test failed: Failed to fetch', + }); + }); + + test('a caller abort is rethrown untouched', async () => { + const controller = new AbortController(); + fetchMock.mockImplementation(() => { + controller.abort(); + const err = new Error('The operation was aborted.'); + err.name = 'AbortError'; + return Promise.reject(err); + }); + await expect(apiRequest('/test', { signal: controller.signal })).rejects.toMatchObject({ name: 'AbortError' }); + }); + + test.each([ + [502, 'CUDly is temporarily unavailable. Try again in a moment.'], + [503, 'CUDly is temporarily unavailable. Try again in a moment.'], + [504, 'CUDly took too long to respond. Try again in a moment.'], + [500, 'CUDly hit an unexpected error. Try again in a moment.'], + [403, "You don't have permission to do that."], + [418, 'The request could not be completed (error 418).'], + ])('HTTP %i with an HTML body says what it means, not "HTTP %i"', async (status, message) => { + fetchMock.mockResolvedValue({ ok: false, status, json: () => Promise.reject(new SyntaxError("Unexpected token '<'")) }); + await expect(apiRequest('/test')).rejects.toMatchObject({ message, status, detail: `HTTP ${status} from /test` }); + }); + + test('the backend catch-all "Internal server error" gets the friendly 500 text', async () => { + fetchMock.mockResolvedValue({ ok: false, status: 500, json: () => Promise.resolve({ error: 'Internal server error' }) }); + await expect(apiRequest('/test')).rejects.toThrow('CUDly hit an unexpected error. Try again in a moment.'); + }); + + test('a specific server message still wins over the status text', async () => { + fetchMock.mockResolvedValue({ ok: false, status: 503, json: () => Promise.resolve({ error: 'maintenance until 10:00 UTC' }) }); + await expect(apiRequest('/test')).rejects.toThrow('maintenance until 10:00 UTC'); + }); }); test('passes the caller-provided signal through alongside the timeout controller', async () => { @@ -660,6 +714,25 @@ describe('API Requests', () => { // tab on the same origin inherits the session. expect(localStorageMock.setItem).toHaveBeenCalledWith('authToken', 'admin-token'); }); + + // Issue #748: a proxy 502 with an HTML body used to surface as + // "Unexpected token '<'" because the error body was parsed uncaught. + test('a 502 HTML error page fails with friendly text, not a JSON parse error', async () => { + fetchMock.mockResolvedValue({ ok: false, status: 502, json: () => Promise.reject(new SyntaxError("Unexpected token '<'")) }); + await expect(setupAdmin('api-key', 'admin@example.com', 'password')) + .rejects.toMatchObject({ message: 'CUDly is temporarily unavailable. Try again in a moment.', status: 502 }); + }); + + test('a 400 keeps the server reason', async () => { + fetchMock.mockResolvedValue({ ok: false, status: 400, json: () => Promise.resolve({ error: 'admin already exists' }) }); + await expect(setupAdmin('api-key', 'admin@example.com', 'password')).rejects.toThrow('admin already exists'); + }); + + test('a network failure says the server cannot be reached', async () => { + fetchMock.mockRejectedValue(new TypeError('Failed to fetch')); + await expect(setupAdmin('api-key', 'admin@example.com', 'password')) + .rejects.toThrow("Can't reach CUDly. Check your connection and try again."); + }); }); }); diff --git a/frontend/src/api/auth.ts b/frontend/src/api/auth.ts index db6781c15..d8e5e7679 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -14,6 +14,7 @@ import type { MFARecoveryCodesResponse, MFALoginErrorCode, } from './types'; +import { fetchOrNetworkError, readJson, responseError } from './errors'; /** * Login error raised when the server returns one of the MFA-related @@ -62,21 +63,21 @@ export async function login(email: string, password: string, mfaCode?: string): // Add content hash for CloudFront OAC await addContentHashHeader(headers, body); - const response = await fetch(`${API_BASE}/auth/login`, { + const response = await fetchOrNetworkError(`${API_BASE}/auth/login`, { method: 'POST', headers, body }); if (!response.ok) { - const data = await response.json().catch(() => ({})) as { error?: string }; - if (isMFALoginErrorCode(data.error)) { - throw new MFALoginError(data.error); + const err = await responseError(response, 'Login failed'); + if (isMFALoginErrorCode(err.message)) { + throw new MFALoginError(err.message); } - throw new Error(data.error || 'Login failed'); + throw err; } - const data = await response.json() as LoginResponse & { csrf_token?: string }; + const data = await readJson(response); setAuthToken(data.token); // Store CSRF token if provided by server if (data.csrf_token) { @@ -166,14 +167,13 @@ export async function requestPasswordReset(email: string): Promise { const headers: Record = { 'Content-Type': 'application/json' }; await addContentHashHeader(headers, body); - const response = await fetch(`${API_BASE}/auth/forgot-password`, { + const response = await fetchOrNetworkError(`${API_BASE}/auth/forgot-password`, { method: 'POST', headers, body }); if (!response.ok) { - const data = await response.json().catch(() => ({})) as { error?: string }; - throw new Error(data.error || 'Failed to send password reset email'); + throw await responseError(response, 'Failed to send password reset email'); } } @@ -186,7 +186,7 @@ export async function resetPassword(token: string, newPassword: string): Promise const headers: Record = { 'Content-Type': 'application/json' }; await addContentHashHeader(headers, body); - const response = await fetch(`${API_BASE}/auth/reset-password`, { + const response = await fetchOrNetworkError(`${API_BASE}/auth/reset-password`, { method: 'POST', headers, body @@ -199,9 +199,7 @@ export async function resetPassword(token: string, newPassword: string): Promise // different one") never reached the toast and users saw the opaque // fallback. Read `error` first; keep `message` as a defensive // fallback for any other shape. See issue #459. - const data = await response.json().catch(() => ({})) as { error?: string; message?: string }; - const msg = data.error || data.message; - throw new Error(msg || 'Failed to reset password'); + throw await responseError(response, 'Failed to reset password'); } } @@ -286,18 +284,19 @@ export async function setupAdmin(key: string, email: string, password: string): const headers: Record = { 'X-API-Key': key, 'Content-Type': 'application/json' }; await addContentHashHeader(headers, body); - const response = await fetch(`${API_BASE}/auth/setup-admin`, { + const response = await fetchOrNetworkError(`${API_BASE}/auth/setup-admin`, { method: 'POST', headers, body }); + // A proxy 502 used to reach the user as "Unexpected token '<'" here + // because the error body was parsed with no catch (issue #748). if (!response.ok) { - const data = await response.json() as { error?: string }; - throw new Error(data.error || 'Failed to create admin'); + throw await responseError(response, 'Failed to create admin'); } - const data = await response.json() as LoginResponse & { csrf_token?: string }; + const data = await readJson(response); setAuthToken(data.token); // Store CSRF token if provided by server if (data.csrf_token) { diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 2b1d0617f..50e88017f 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -4,6 +4,15 @@ import type { ApiError, RequestOptions } from './types'; import { reloadPage } from '../utils'; +import { + BAD_RESPONSE_MESSAGE, + NETWORK_ERROR_MESSAGE, + TIMEOUT_ERROR_MESSAGE, + httpStatusMessage, + isGenericServerMessage, + isNetworkFailure, + makeApiError, +} from './errors'; const API_BASE = '/api'; @@ -239,12 +248,14 @@ export async function apiRequest(endpoint: string, options: RequestOptions = signal, }); } catch (err) { - if (timeoutController.signal.aborted) { - const error: ApiError = new Error( - `Request to ${endpoint} timed out after ${timeoutMs}ms`, - ); - error.status = 0; - throw error; + // Transport failures carry status 0 and a sentence the user can act + // on; the raw text stays on `detail` for support (issue #748). A + // caller's own abort is rethrown untouched so it can tell it apart. + if (timeoutController.signal.aborted && !options.signal?.aborted) { + throw makeApiError(TIMEOUT_ERROR_MESSAGE, 0, `Request to ${endpoint} timed out after ${timeoutMs}ms`); + } + if (isNetworkFailure(err)) { + throw makeApiError(NETWORK_ERROR_MESSAGE, 0, `Request to ${endpoint} failed: ${(err as Error).message}`); } throw err; } finally { @@ -252,12 +263,15 @@ export async function apiRequest(endpoint: string, options: RequestOptions = } if (!response.ok) { - const error: ApiError = new Error(`HTTP ${response.status}`); - error.status = response.status; + // Without a server message, say what the status means instead of + // showing a bare "HTTP 502" (issue #748). A server message wins. + const error: ApiError = makeApiError( + httpStatusMessage(response.status), response.status, `HTTP ${response.status} from ${endpoint}`, + ); try { const data = await response.json() as Record; const message = typeof data['error'] === 'string' ? data['error'] : ''; - if (message) error.message = message; + if (message && !isGenericServerMessage(message)) error.message = message; // Strip the `error` key and surface the rest as structured // details so callers can branch on ops_hint / retry_attempt_n / // threshold / retry_execution_id without substring-matching the @@ -282,7 +296,14 @@ export async function apiRequest(endpoint: string, options: RequestOptions = if (response.status === 204 || response.headers?.get('content-length') === '0') { return null as T; } - return await response.json() as T; + try { + return await response.json() as T; + } catch (parseErr) { + // A proxy or CDN error page served with a 2xx would otherwise reach + // the user as "Unexpected token '<' ..." (issue #748). + throw makeApiError(BAD_RESPONSE_MESSAGE, response.status, + `Unparseable response from ${endpoint}: ${(parseErr as Error).message}`); + } } /** diff --git a/frontend/src/api/errors.ts b/frontend/src/api/errors.ts new file mode 100644 index 000000000..16b253334 --- /dev/null +++ b/frontend/src/api/errors.ts @@ -0,0 +1,119 @@ +/** + * User-facing wording for API and transport failures (issue #748). + * + * The API client used to surface developer text: "HTTP 502", "Request to + * /api/x timed out after 90000ms", the browser's "Failed to fetch", or a + * JSON parse error such as "Unexpected token '<'" when a proxy answered + * with an HTML error page. Those strings were interpolated into dozens of + * toasts and inline errors. The client now builds its errors from these + * helpers, so every caller that shows `err.message` gets a sentence a user + * can act on, while the raw text is kept on `ApiError.detail` (and in the + * console) for support. A message the server sent is always preferred + * over these generic ones. + */ + +import type { ApiError } from './types'; + +export const NETWORK_ERROR_MESSAGE = "Can't reach CUDly. Check your connection and try again."; +export const TIMEOUT_ERROR_MESSAGE = 'CUDly took too long to respond. Try again in a moment.'; +export const UNAVAILABLE_ERROR_MESSAGE = 'CUDly is temporarily unavailable. Try again in a moment.'; +export const BAD_RESPONSE_MESSAGE = 'CUDly sent an unexpected response. Try again in a moment.'; +export const SERVER_ERROR_MESSAGE = 'CUDly hit an unexpected error. Try again in a moment.'; +const UNKNOWN_ERROR_MESSAGE = 'Something went wrong. Try again in a moment.'; + +/** Generic sentence for an HTTP status when the server sent no message. */ +export function httpStatusMessage(status: number): string { + if (status === 401) return 'Your session has expired. Sign in again.'; + if (status === 403) return "You don't have permission to do that."; + if (status === 404) return 'The requested item was not found.'; + if (status === 408 || status === 504) return TIMEOUT_ERROR_MESSAGE; + if (status === 429) return 'Too many requests. Wait a moment and try again.'; + if (status === 500) return SERVER_ERROR_MESSAGE; + if (status > 500) return UNAVAILABLE_ERROR_MESSAGE; + return `The request could not be completed (error ${status}).`; +} + +/** + * The backend's catch-all 500 body ({"error": "internal server error"}) + * says nothing the status does not, so it gets the friendly status text; + * any other server message is specific and is shown as sent. + */ +export function isGenericServerMessage(message: string): boolean { + return /^internal server error\.?$/i.test(message.trim()); +} + +/** Build an ApiError with a user-facing message and the raw text for support. */ +export function makeApiError(message: string, status: number, detail?: string): ApiError { + const error: ApiError = new Error(message); + error.status = status; + if (detail) error.detail = detail; + return error; +} + +/** + * True for the TypeError fetch() rejects with when the request never got + * a response (offline, DNS, CORS, connection reset). Chrome says "Failed + * to fetch", Firefox "NetworkError when attempting to fetch resource.", + * Safari "Load failed". + */ +export function isNetworkFailure(err: unknown): boolean { + return err instanceof TypeError + && /failed to fetch|networkerror|load failed|network request failed/i.test(err.message); +} + +/** + * A message safe to show a user for any thrown value: the server's (or + * client's) message for an ApiError, friendly text for a raw network or + * parse failure, otherwise the error's own message. + */ +export function friendlyError(err: unknown): string { + if (isNetworkFailure(err)) return NETWORK_ERROR_MESSAGE; + if (err instanceof SyntaxError) return BAD_RESPONSE_MESSAGE; + if (err instanceof Error) { + if (err.message) return err.message; + const status = (err as ApiError).status; + return typeof status === 'number' && status > 0 ? httpStatusMessage(status) : UNKNOWN_ERROR_MESSAGE; + } + if (typeof err === 'string' && err) return err; + return UNKNOWN_ERROR_MESSAGE; +} + +/* + * Helpers for the few auth calls that use fetch() directly instead of + * apiRequest (login, password reset, first-admin setup), so they fail + * with the same wording. + */ + +/** fetch() that turns a transport failure into a friendly ApiError. */ +export async function fetchOrNetworkError(url: string, init?: RequestInit): Promise { + try { + return await fetch(url, init); + } catch (err) { + if (isNetworkFailure(err)) { + throw makeApiError(NETWORK_ERROR_MESSAGE, 0, `Request to ${url} failed: ${(err as Error).message}`); + } + throw err; + } +} + +/** + * The error to throw for a non-OK response: the server's `error` (or + * `message`) when it sent a specific one, the friendly status text for a + * 5xx or an unreadable body, otherwise the caller's fallback. + */ +export async function responseError(response: Response, fallback: string): Promise { + const data = await response.json().catch(() => ({})) as { error?: unknown; message?: unknown }; + const raw = typeof data.error === 'string' ? data.error : typeof data.message === 'string' ? data.message : ''; + const generic = !raw || isGenericServerMessage(raw); + const message = !generic ? raw : response.status >= 500 ? httpStatusMessage(response.status) : fallback; + return makeApiError(message, response.status, `HTTP ${response.status} from ${response.url || 'auth endpoint'}`); +} + +/** response.json() that fails with friendly text on an HTML or truncated body. */ +export async function readJson(response: Response): Promise { + try { + return await response.json() as T; + } catch (err) { + throw makeApiError(BAD_RESPONSE_MESSAGE, response.status, `Unparseable response: ${(err as Error).message}`); + } +} diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index f3ad8bc6f..aec34cc46 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -942,6 +942,10 @@ export interface ApiError extends Error { // See internal/api/handler.go for the flattening — keys are // promoted to the top level of the JSON body. details?: Record; + // Raw technical text behind a user-facing message (e.g. "HTTP 502 from + // /api/x", "timed out after 90000ms", the browser's fetch error), kept + // for the console and support while `message` stays readable (#748). + detail?: string; } export interface RequestOptions extends RequestInit { From 90290b8ef32562c5f0030836b746d1c3834a1ce4 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Fri, 23 Oct 2026 23:44:08 +0000 Subject: [PATCH 2/2] fix(frontend): show the server's reason when deletes and rejects fail Deleting a user, group, API key or registration, revoking an API key, rejecting a registration, and saving bulk overrides all replaced the server's error with a fixed string ("Failed to delete user", "Check your connection and try again"), so an admin never learned why the action was refused. These now append friendlyError(err): the server's reason when it sent one, otherwise the transport wording. Also adds a Playwright spec for the transport wording on the dashboard: a 502 HTML page, a dropped connection and an HTML page served with 200. Refs #748 Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/apikeys.test.ts | 6 +-- frontend/src/__tests__/groups.test.ts | 2 +- .../src/__tests__/settings-accounts.test.ts | 5 ++- frontend/src/__tests__/users.test.ts | 7 ++++ frontend/src/apikeys.ts | 5 ++- frontend/src/groups/groupActions.ts | 3 +- frontend/src/modules/registrations.ts | 9 +++-- frontend/src/settings.ts | 6 ++- frontend/src/users/userActions.ts | 3 +- .../tests-e2e/friendly-api-errors.spec.ts | 38 +++++++++++++++++++ 10 files changed, 69 insertions(+), 15 deletions(-) create mode 100644 frontend/tests-e2e/friendly-api-errors.spec.ts diff --git a/frontend/src/__tests__/apikeys.test.ts b/frontend/src/__tests__/apikeys.test.ts index ecf634f16..acdc5ce4a 100644 --- a/frontend/src/__tests__/apikeys.test.ts +++ b/frontend/src/__tests__/apikeys.test.ts @@ -950,7 +950,7 @@ describe('API Keys Module', () => { await revokeApiKey('key-1'); - expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key'); + expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key: Revoke failed'); expect(document.getElementById('create-apikey-error')?.classList.contains('hidden')).toBe(true); consoleError.mockRestore(); }); @@ -965,7 +965,7 @@ describe('API Keys Module', () => { await revokeApiKey('key-1'); expect(consoleError).toHaveBeenCalledWith('Failed to revoke API key:', expect.any(Error)); - expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key'); + expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to revoke API key: Revoke failed'); consoleError.mockRestore(); }); }); @@ -1019,7 +1019,7 @@ describe('API Keys Module', () => { await deleteApiKey('key-1'); expect(consoleError).toHaveBeenCalledWith('Failed to delete API key:', expect.any(Error)); - expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to delete API key'); + expect(document.querySelector('.toast-error')?.querySelector('.toast-message')?.textContent).toBe('Failed to delete API key: Delete failed'); consoleError.mockRestore(); }); }); diff --git a/frontend/src/__tests__/groups.test.ts b/frontend/src/__tests__/groups.test.ts index 211de0534..ed0afb226 100644 --- a/frontend/src/__tests__/groups.test.ts +++ b/frontend/src/__tests__/groups.test.ts @@ -907,7 +907,7 @@ describe('groups/groupActions', () => { await groupActions.deleteGroup('group-1'); - expect(document.querySelector('.toast-error')?.textContent).toContain('Failed to delete group'); + expect(document.querySelector('.toast-error')?.textContent).toContain('Failed to delete group: Server error'); }); }); diff --git a/frontend/src/__tests__/settings-accounts.test.ts b/frontend/src/__tests__/settings-accounts.test.ts index bd8db15fd..0cc0c816a 100644 --- a/frontend/src/__tests__/settings-accounts.test.ts +++ b/frontend/src/__tests__/settings-accounts.test.ts @@ -2165,7 +2165,7 @@ describe('Bulk override modal (issue #119)', () => { test('all saves fail: error shown in form, modal stays open', async () => { (api.listAccountServiceOverrides as jest.Mock).mockResolvedValue([]); - (api.saveAccountServiceOverride as jest.Mock).mockRejectedValue(new Error('timeout')); + (api.saveAccountServiceOverride as jest.Mock).mockRejectedValue(new Error('coverage must be between 0 and 100')); const { form, bulkList } = await openBulkModal(); @@ -2184,7 +2184,8 @@ describe('Bulk override modal (issue #119)', () => { expect(mockShowToast).not.toHaveBeenCalled(); const errEl = document.getElementById('override-form-error') as HTMLElement; - expect(errEl.textContent).toMatch(/failed/i); + // Issue #748: the server's reason, not a blanket "Check your connection". + expect(errEl.textContent).toBe('All 2 saves failed: coverage must be between 0 and 100'); // Modal stays open so user can retry. const modal = document.getElementById('override-modal') as HTMLElement; diff --git a/frontend/src/__tests__/users.test.ts b/frontend/src/__tests__/users.test.ts index 755426520..abd133ed2 100644 --- a/frontend/src/__tests__/users.test.ts +++ b/frontend/src/__tests__/users.test.ts @@ -1135,6 +1135,13 @@ describe('users/userActions', () => { ); }); + it('shows the server reason when the delete fails (issue #748)', async () => { + (api.deleteUser as jest.Mock).mockRejectedValueOnce(new Error('cannot delete the last administrator')); + await userActions.deleteUser('1'); + + expect(document.querySelector('.toast-error')?.textContent).toContain('Failed to delete user: cannot delete the last administrator'); + }); + it('should reload users after deletion', async () => { await userActions.deleteUser('1'); diff --git a/frontend/src/apikeys.ts b/frontend/src/apikeys.ts index 31cd71265..d23168943 100644 --- a/frontend/src/apikeys.ts +++ b/frontend/src/apikeys.ts @@ -7,6 +7,7 @@ import type { APIKeyInfo, CreateAPIKeyResponse } from './types'; import { loadApiKeysUsageStats } from './apikeys_usage'; import { escapeHtml, formatDateTime, formatRelativeTime } from './utils'; import { confirmDialog } from './confirmDialog'; +import { friendlyError } from './api/errors'; import { showToast } from './toast'; import { openModal, closeModal } from './modal'; import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; @@ -496,7 +497,7 @@ export async function revokeApiKey(keyId: string): Promise { await loadApiKeys(); } catch (error) { console.error('Failed to revoke API key:', error); - showError('Failed to revoke API key'); + showError(`Failed to revoke API key: ${friendlyError(error)}`); } } @@ -520,7 +521,7 @@ export async function deleteApiKey(keyId: string): Promise { await loadApiKeys(); } catch (error) { console.error('Failed to delete API key:', error); - showError('Failed to delete API key'); + showError(`Failed to delete API key: ${friendlyError(error)}`); } } diff --git a/frontend/src/groups/groupActions.ts b/frontend/src/groups/groupActions.ts index 9a0320f7d..920f6b650 100644 --- a/frontend/src/groups/groupActions.ts +++ b/frontend/src/groups/groupActions.ts @@ -7,6 +7,7 @@ import { availableGroups } from '../users/state'; import { showError, showSuccess } from '../users/utils'; import { loadUsers } from '../users/userActions'; import { confirmDialog } from '../confirmDialog'; +import { friendlyError } from '../api/errors'; /** * Delete group @@ -29,7 +30,7 @@ export async function deleteGroup(groupId: string): Promise { showSuccess('Group deleted successfully'); } catch (error) { console.error('Failed to delete group:', error); - showError('Failed to delete group'); + showError(`Failed to delete group: ${friendlyError(error)}`); } } diff --git a/frontend/src/modules/registrations.ts b/frontend/src/modules/registrations.ts index 81e5272c3..db30d2392 100644 --- a/frontend/src/modules/registrations.ts +++ b/frontend/src/modules/registrations.ts @@ -9,6 +9,7 @@ import { openAccountModal, loadAccountsForProvider } from '../settings'; import { formatDateTime } from '../utils'; import { showToast } from '../toast'; import { confirmDialog } from '../confirmDialog'; +import { friendlyError } from '../api/errors'; type AccountProvider = 'aws' | 'azure' | 'gcp'; @@ -232,8 +233,8 @@ async function handleDelete(reg: AccountRegistration): Promise { try { await api.deleteRegistration(reg.id); await loadRegistrations(); - } catch { - showToast({ message: 'Failed to delete registration.', kind: 'error' }); + } catch (err) { + showToast({ message: `Failed to delete registration: ${friendlyError(err)}`, kind: 'error' }); } } @@ -259,9 +260,9 @@ async function handleReject(reg: AccountRegistration): Promise { try { await api.rejectRegistration(reg.id, reason || undefined); await loadRegistrations(); - } catch { + } catch (err) { // Use showToast instead of blocking alert() (finding 11-L3). - showToast({ message: 'Failed to reject registration.', kind: 'error' }); + showToast({ message: `Failed to reject registration: ${friendlyError(err)}`, kind: 'error' }); } } diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index efbed45af..bcc2b8d64 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -7,6 +7,7 @@ import type { ConfigResponse } from './types'; import { fetchAndPopulateCommitmentOptions } from './commitmentOptions'; import { initFederationPanel } from './federation'; import { confirmDialog } from './confirmDialog'; +import { friendlyError } from './api/errors'; import { reflectDirtyState } from './settings-subnav'; import { showToast } from './toast'; import { isValidCombination, getValidPaymentOptions, getValidTermOptions, getPaymentLabel, getCommitmentConfig, normalizePaymentValue } from './commitmentOptions'; @@ -1995,7 +1996,10 @@ async function submitBulkOverrideForm( } else if (succeeded === 0) { if (submitBtn) submitBtn.disabled = false; if (errEl) { - errEl.textContent = `All ${failed.length} save${failed.length !== 1 ? 's' : ''} failed. Check your connection and try again.`; + // Show the server's reason (e.g. a validation error) instead of + // always blaming the connection (issue #748). + const firstFailure = results.find((r): r is PromiseRejectedResult => r.status === 'rejected'); + errEl.textContent = `All ${failed.length} save${failed.length !== 1 ? 's' : ''} failed: ${friendlyError(firstFailure?.reason)}`; } } else { // Partial success: close and reload, surface the failures in a toast. diff --git a/frontend/src/users/userActions.ts b/frontend/src/users/userActions.ts index cb1f85d02..3a9226cdd 100644 --- a/frontend/src/users/userActions.ts +++ b/frontend/src/users/userActions.ts @@ -15,6 +15,7 @@ import { } from './state'; import { showError, showSuccess, validateGroupCombination } from './utils'; import { confirmDialog } from '../confirmDialog'; +import { friendlyError } from '../api/errors'; import { applyFilters } from './filters'; import { renderUsers, renderUserStats, populateBulkGroupSelect } from './userList'; import { renderGroups } from '../groups/groupList'; @@ -148,7 +149,7 @@ export async function deleteUser(userId: string): Promise { showSuccess('User deleted successfully'); } catch (error) { console.error('Failed to delete user:', error); - showError('Failed to delete user'); + showError(`Failed to delete user: ${friendlyError(error)}`); } } diff --git a/frontend/tests-e2e/friendly-api-errors.spec.ts b/frontend/tests-e2e/friendly-api-errors.spec.ts new file mode 100644 index 000000000..96af2dd88 --- /dev/null +++ b/frontend/tests-e2e/friendly-api-errors.spec.ts @@ -0,0 +1,38 @@ +import { test, expect } from '@playwright/test'; +import { mockApi, seedAuth } from './fixtures/recs'; + +// Issue #748: transport failures reach the user as a sentence they can act +// on, not "HTTP 502", "Failed to fetch" or "Unexpected token '<'". + +const summary = (page: import('@playwright/test').Page) => page.locator('#summary .error'); + +test.beforeEach(async ({ page }) => { + await seedAuth(page); + await mockApi(page); +}); + +test('a proxy 502 HTML page on the dashboard says CUDly is unavailable', async ({ page }) => { + await page.route('**/api/dashboard/summary**', route => route.fulfill({ + status: 502, contentType: 'text/html', body: '

502 Bad Gateway

', + })); + await page.goto('/home'); + + await expect(summary(page)).toHaveText('Failed to load dashboard: CUDly is temporarily unavailable. Try again in a moment.'); + await expect(summary(page)).not.toContainText('HTTP 502'); +}); + +test('a dropped connection on the dashboard says CUDly cannot be reached', async ({ page }) => { + await page.route('**/api/dashboard/summary**', route => route.abort('internetdisconnected')); + await page.goto('/home'); + + await expect(summary(page)).toHaveText("Failed to load dashboard: Can't reach CUDly. Check your connection and try again."); +}); + +test('an HTML page served with 200 is reported as an unexpected response, not a JSON parse error', async ({ page }) => { + await page.route('**/api/dashboard/summary**', route => route.fulfill({ + status: 200, contentType: 'text/html', body: '', + })); + await page.goto('/home'); + + await expect(summary(page)).toHaveText('Failed to load dashboard: CUDly sent an unexpected response. Try again in a moment.'); +});