diff --git a/frontend/src/__tests__/api-errors.test.ts b/frontend/src/__tests__/api-errors.test.ts new file mode 100644 index 00000000..1936e9ff --- /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 a59af4f9..35aae214 100644 --- a/frontend/src/__tests__/api.test.ts +++ b/frontend/src/__tests__/api.test.ts @@ -404,7 +404,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 () => { @@ -419,7 +424,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 () => { @@ -662,15 +716,23 @@ describe('API Requests', () => { expect(localStorageMock.setItem).toHaveBeenCalledWith('authToken', 'admin-token'); }); - test('a non-JSON error body (proxy 502) yields a readable message, not a SyntaxError', async () => { - fetchMock.mockResolvedValue({ - ok: false, - status: 502, - json: () => Promise.reject(new SyntaxError('Unexpected token < in JSON at position 0')), - }); + // 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('Failed to create admin (HTTP 502)'); + .rejects.toThrow("Can't reach CUDly. Check your connection and try again."); }); test('a JSON error body still surfaces the server message', async () => { diff --git a/frontend/src/__tests__/apikeys.test.ts b/frontend/src/__tests__/apikeys.test.ts index b1a42548..a26e1b78 100644 --- a/frontend/src/__tests__/apikeys.test.ts +++ b/frontend/src/__tests__/apikeys.test.ts @@ -962,7 +962,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(); }); @@ -977,7 +977,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(); }); }); @@ -1031,7 +1031,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 6cbd6e30..f0d34d12 100644 --- a/frontend/src/__tests__/groups.test.ts +++ b/frontend/src/__tests__/groups.test.ts @@ -910,7 +910,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 ca89b2b9..e3ec189d 100644 --- a/frontend/src/__tests__/settings-accounts.test.ts +++ b/frontend/src/__tests__/settings-accounts.test.ts @@ -2303,7 +2303,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(); @@ -2322,7 +2322,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 96eb5855..f43139ef 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/api/auth.ts b/frontend/src/api/auth.ts index ebe37119..02b0d3b7 100644 --- a/frontend/src/api/auth.ts +++ b/frontend/src/api/auth.ts @@ -15,6 +15,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 @@ -63,21 +64,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) { @@ -167,14 +168,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'); } } @@ -187,7 +187,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 @@ -200,9 +200,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'); } } @@ -287,19 +285,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) { - // A proxy 502 carries an HTML body; don't surface the JSON SyntaxError. - const data = await response.json().catch(() => ({})) as { error?: string }; - throw new Error(data.error || `Failed to create admin (HTTP ${response.status})`); + 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 33b85827..01fd1dc4 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'; @@ -254,12 +263,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 { @@ -267,12 +278,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 @@ -297,7 +311,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 00000000..16b25333 --- /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 b6014d3b..7116a110 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -957,6 +957,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 { diff --git a/frontend/src/apikeys.ts b/frontend/src/apikeys.ts index 7bd53c4b..979c99f0 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'; @@ -516,7 +517,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)}`); } } @@ -540,7 +541,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 ef74b109..91dc0af1 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'; import { isSystemManagedGroup, SYSTEM_GROUP_NOTE } from './systemGroups'; /** @@ -37,9 +38,9 @@ export async function deleteGroup(groupId: string): Promise { } catch (error) { console.error('Failed to delete group:', error); // Surface the server's reason (e.g. the system-managed refusal), as the - // Edit path already does (#919 review comment). - const err = error as Error; - showError(err?.message ? `Failed to delete group: ${err.message}` : 'Failed to delete group'); + // Edit path already does (#919 review comment). friendlyError keeps a + // server message and turns transport failures into plain text (#748). + showError(`Failed to delete group: ${friendlyError(error)}`); } } diff --git a/frontend/src/modules/registrations.ts b/frontend/src/modules/registrations.ts index 7bbaa587..06836ba0 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 4ef97a90..f14bc7c3 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 { deleteAccount } from './settings-account-delete'; import { reflectDirtyState } from './settings-subnav'; import { showToast } from './toast'; @@ -1857,7 +1858,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 de15a92b..c978997f 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'; @@ -175,7 +176,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 00000000..96af2dd8 --- /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.'); +});