From ed1c4492347702a64e37bfab085b6a2c5f0a7d8b Mon Sep 17 00:00:00 2001 From: nagelm <10207686+nagelm@users.noreply.github.com> Date: Tue, 14 Jul 2026 20:37:58 +1200 Subject: [PATCH 1/2] fix(ui): stop rendering raw HTML error pages in toast notifications (#1261) Failed API responses whose body is not JSON - Django's HTML "Server Error (500)" page, nginx's HTML 502/504 pages when the backend is down or timing out - were interpolated verbatim into the error toast, showing users a wall of markup. request() deliberately keeps the raw text when JSON.parse fails, and errorNotification() rendered `${status} - ${body}` unconditionally. Route the body through a new formatApiError() helper (in utils.js so it is unit-testable without importing the store-heavy api module): - JSON object bodies keep their existing pretty-printed formatting - markup and empty bodies collapse to the response's own status line: the declared Content-Type decides what counts as markup (body sniffing only as fallback when the header is missing), and the label comes from the fetch Response's statusText - the protocol's reason phrase, defined for every status code, rather than a hardcoded status map. HTTP/2+ transmits no reason phrase, so an empty statusText falls back to a generic label. request() already attaches the Response to the error, so no call-site changes are needed. - the full suppressed body goes to console.debug so the markup stays available for troubleshooting (deliberate, not a leftover debug statement) - plain-text bodies are truncated at 200 chars as a backstop - errors without a status keep the error.message fallback --- frontend/src/__tests__/formatApiError.test.js | 138 ++++++++++++++++++ frontend/src/api.js | 19 +-- frontend/src/utils.js | 52 +++++++ 3 files changed, 192 insertions(+), 17 deletions(-) create mode 100644 frontend/src/__tests__/formatApiError.test.js diff --git a/frontend/src/__tests__/formatApiError.test.js b/frontend/src/__tests__/formatApiError.test.js new file mode 100644 index 00000000..2814dac5 --- /dev/null +++ b/frontend/src/__tests__/formatApiError.test.js @@ -0,0 +1,138 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { formatApiError } from '../utils'; + +const djangoHtml500 = + ' Server Error (500)

Server Error (500)

'; + +// request() attaches the fetch Response to the error; formatApiError reads +// the reason phrase and declared content type from it. +const fakeResponse = (statusText, contentType) => ({ + statusText, + headers: { get: (name) => (name === 'content-type' ? contentType : null) }, +}); + +describe('formatApiError', () => { + it('replaces a Django HTML 500 body with the response status line', () => { + const msg = formatApiError({ + status: 500, + body: djangoHtml500, + response: fakeResponse('Internal Server Error', 'text/html'), + }); + expect(msg).toBe('500 - Internal Server Error'); + expect(msg).not.toContain('<'); + }); + + it('uses the reason phrase for nginx gateway HTML pages', () => { + expect( + formatApiError({ + status: 504, + body: '

504 Gateway Time-out

', + response: fakeResponse('Gateway Time-out', 'text/html'), + }) + ).toBe('504 - Gateway Time-out'); + expect( + formatApiError({ + status: 502, + body: '', + response: fakeResponse('Bad Gateway', 'text/html'), + }) + ).toBe('502 - Bad Gateway'); + }); + + it('trusts the declared content type over body sniffing', () => { + // Some proxies prepend text before the markup; the header still says html. + const msg = formatApiError({ + status: 503, + body: 'upstream connect error ...', + response: fakeResponse('Service Unavailable', 'text/html; charset=utf-8'), + }); + expect(msg).toBe('503 - Service Unavailable'); + }); + + it('sniffs markup when no content type is available', () => { + // No response attached (older call sites); the leading '<' is the tell. + expect(formatApiError({ status: 500, body: djangoHtml500 })).toBe( + '500 - Request failed' + ); + }); + + it('falls back to a generic label when the reason phrase is absent', () => { + // HTTP/2+ responses carry no reason phrase (statusText === ''). + expect( + formatApiError({ + status: 502, + body: '', + response: fakeResponse('', 'text/html'), + }) + ).toBe('502 - Request failed'); + }); + + it('preserves the suppressed HTML body via console.debug', () => { + const spy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + formatApiError({ + status: 500, + body: djangoHtml500, + response: fakeResponse('Internal Server Error', 'text/html'), + }); + expect(spy).toHaveBeenCalledWith( + 'API error 500 response body:', + djangoHtml500 + ); + spy.mockRestore(); + }); + + it('keeps JSON object bodies pretty-printed (existing behaviour)', () => { + expect( + formatApiError({ status: 500, body: { error: 'Redis not available' } }) + ).toBe(JSON.stringify({ error: 'Redis not available' }, null, 2)); + }); + + it('passes through short plain-text bodies with the status', () => { + expect( + formatApiError({ + status: 403, + body: 'Forbidden by policy', + response: fakeResponse('Forbidden', 'text/plain'), + }) + ).toBe('403 - Forbidden by policy'); + }); + + it('truncates long plain-text bodies', () => { + const long = 'x'.repeat(500); + const msg = formatApiError({ + status: 500, + body: long, + response: fakeResponse('Internal Server Error', 'text/plain'), + }); + expect(msg.length).toBeLessThanOrEqual(210); + expect(msg.endsWith('...')).toBe(true); + }); + + it('uses the status line when the body is empty', () => { + expect( + formatApiError({ + status: 503, + body: '', + response: fakeResponse('Service Unavailable', ''), + }) + ).toBe('503 - Service Unavailable'); + }); + + it('handles non-standard statuses via the server-provided reason phrase', () => { + expect( + formatApiError({ + status: 599, + body: '', + response: fakeResponse('Network Connect Timeout Error', 'text/html'), + }) + ).toBe('599 - Network Connect Timeout Error'); + }); + + it('uses error.message for non-HTTP errors (fetch/network failures)', () => { + expect(formatApiError(new TypeError('Failed to fetch'))).toBe( + 'Failed to fetch' + ); + expect(formatApiError({})).toBe('Unknown error'); + }); +}); diff --git a/frontend/src/api.js b/frontend/src/api.js index 50db0165..3a745896 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -15,7 +15,7 @@ import useChannelsTableStore from './store/channelsTable'; import useStreamsTableStore from './store/streamsTable'; import useUsersStore from './store/users'; import useConnectStore from './store/connect'; -import Limiter from './utils'; +import Limiter, { formatApiError } from './utils'; // If needed, you can set a base host or keep it empty if relative requests const host = import.meta.env.DEV @@ -23,22 +23,7 @@ const host = import.meta.env.DEV : ''; const errorNotification = (message, error) => { - let errorMessage = ''; - - if (error.status) { - try { - // Try to format the error body if it's an object - if (typeof error.body === 'object') { - errorMessage = JSON.stringify(error.body, null, 2); - } else { - errorMessage = `${error.status} - ${error.body}`; - } - } catch (e) { - errorMessage = `${error.status} - ${String(error.body)}`; - } - } else { - errorMessage = error.message || 'Unknown error'; - } + const errorMessage = formatApiError(error); notifications.show({ title: 'Error', diff --git a/frontend/src/utils.js b/frontend/src/utils.js index 27abf86c..9276ccd1 100644 --- a/frontend/src/utils.js +++ b/frontend/src/utils.js @@ -68,6 +68,58 @@ export function useDebounce(value, delay = 500, callback = null) { return debouncedValue; } +// Human-readable message for a failed API response (#1261). Backends that +// are down or timing out answer with whole HTML error pages (Django's +// "Server Error (500)" page, nginx's 502/504 pages) - rendering those +// verbatim in a toast is unreadable. JSON error bodies keep their existing +// formatting; markup and empty bodies collapse to the response's own +// status line (the full body goes to console.debug for troubleshooting); +// long plain-text bodies are truncated. +const ERROR_BODY_MAX_CHARS = 200; + +export const formatApiError = (error) => { + if (!error || !error.status) { + return (error && error.message) || 'Unknown error'; + } + + // request() attaches the fetch Response as error.response; it carries the + // canonical reason phrase and the declared content type. + const { status, body, response } = error; + + if (body && typeof body === 'object') { + try { + return JSON.stringify(body, null, 2); + } catch { + // Unserializable object; fall through to the string handling below. + } + } + + const text = + typeof body === 'string' ? body.trim() : body == null ? '' : String(body); + + // Trust the declared content type first; sniff only when it is absent + // (some proxies omit or mislabel it on error pages). + const contentType = response?.headers?.get?.('content-type') || ''; + const isMarkup = + contentType.includes('html') || + contentType.includes('xml') || + text.startsWith('<'); + + if (!text || isMarkup) { + if (text) { + console.debug(`API error ${status} response body:`, text); + } + // statusText is the protocol's reason phrase ("Bad Gateway"), defined + // for every status code. HTTP/2+ does not transmit one, so fall back + // to a generic label rather than re-enumerating the HTTP spec here. + return `${status} - ${response?.statusText || 'Request failed'}`; + } + + return text.length > ERROR_BODY_MAX_CHARS + ? `${status} - ${text.slice(0, ERROR_BODY_MAX_CHARS)}...` + : `${status} - ${text}`; +}; + export function sleep(ms) { return new Promise((resolve) => { setTimeout(resolve, ms); From 5263e18ed982aea6e2703b0629a7d8d5b5b18903 Mon Sep 17 00:00:00 2001 From: SergeantPanda Date: Fri, 17 Jul 2026 15:31:56 +0000 Subject: [PATCH 2/2] fix(api): improve error handling for API responses - Enhanced the `formatApiError` function to better format failed API responses, preventing raw HTML error pages from being displayed in toasts. JSON error bodies are now prioritized for user-friendly messages, while HTML and empty bodies are collapsed to a concise status line. Long plain-text bodies are truncated for readability. - Updated tests to cover new error formatting behavior, ensuring accurate extraction of error messages from various JSON structures. --- CHANGELOG.md | 1 + frontend/src/__tests__/formatApiError.test.js | 78 ++++++++++++-- frontend/src/utils.js | 101 ++++++++++++++---- 3 files changed, 148 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 119e3f14..814f54b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **API error toasts no longer dump raw HTML error pages.** Failed responses that return Django or nginx HTML (for example 500/502/504 when the backend is down) were interpolated into the toast body as markup. Errors are now formatted for display: JSON bodies prefer `detail` / `error` / field messages, HTML and empty bodies collapse to a short status line, and long plain-text bodies are truncated. — Thanks [@nagelm](https://github.com/nagelm) - **Swagger/OpenAPI schema generation no longer fails under concurrent gevent requests.** Concurrent hits to `/api/schema/` (for example Swagger UI loading) could race on DRF's shared `AutoSchema` state and raise `AssertionError: Schema generation REQUIRES a view instance`, leaving Swagger empty. Cold builds could also hang the gevent uWSGI worker. Schema introspection now runs in a real OS thread, builds are single-flight per process, and the result is cached in Django's cache (Redis) so all workers share it. - **`/proxy/ts/change_stream` now persists `stream_id` and waits for the real switch outcome in multi-worker deployments.** When the request landed on a worker that didn't own the channel, the owning worker's `STREAM_SWITCH` pubsub handler performed the switch but wrote only `url`/`user_agent` back to the channel's Redis metadata, so `GET /proxy/ts/status` kept reporting the old `stream_id` indefinitely; the handler now persists the full switch metadata (`stream_id`, `m3u_profile`, stream name) via the same `_update_channel_metadata()` helper the direct owner path uses. `stream_name` from the initial `get_stream_info_for_switch()` lookup is now forwarded through the pubsub payload (and from `change_stream` / `next_stream` on the direct owner path) so the owner no longer re-queries the database when writing metadata. The endpoint also no longer returns an optimistic 200 on the non-owner path: the requesting worker waits (up to 15s) for the owner to confirm via the `switch_status` Redis key and answers 502 if the owner reports failure or 504 if no confirmation arrives (e.g. owner worker gone); `next_stream` gets the same treatment. Requesting a switch to the URL the channel is already playing is now treated as success (with a metadata refresh) instead of a silent no-op, and the `switch_status` key now expires (60s TTL) instead of persisting forever. (Fixes #1412) - **XC live streams, `/panel_api.php`, and `/xmltv.php` no longer crash when a visible channel has no channel number.** Since `channel_number` became nullable, auto-sync and compact numbering can leave a channel with `effective_channel_number=None` while it remains visible in output (`hidden_from_output=False`). The XC live-stream number map skipped those rows but still tried to emit them, causing `KeyError` on `get_live_streams` and a 500 on `/panel_api.php` (`xc_get_info`). Authenticated XMLTV export (`/xmltv.php`, profile EPG with a logged-in user) had the same gap and could fail chunk-cache rebuilds with `KeyError` on `channel_num_map[channel.id]`. Null-number channels are now deferred into the existing collision-free integer assignment pass (same second loop as fractional numbers) and receive the next available integer; `_xc_channel_entry` also falls back to `channel.id` if a row is still unmapped. HDHR lineup behaviour is unchanged (null-number channels remain omitted there). diff --git a/frontend/src/__tests__/formatApiError.test.js b/frontend/src/__tests__/formatApiError.test.js index 2814dac5..d1e92ef9 100644 --- a/frontend/src/__tests__/formatApiError.test.js +++ b/frontend/src/__tests__/formatApiError.test.js @@ -5,8 +5,6 @@ import { formatApiError } from '../utils'; const djangoHtml500 = ' Server Error (500)

Server Error (500)

'; -// request() attaches the fetch Response to the error; formatApiError reads -// the reason phrase and declared content type from it. const fakeResponse = (statusText, contentType) => ({ statusText, headers: { get: (name) => (name === 'content-type' ? contentType : null) }, @@ -41,7 +39,6 @@ describe('formatApiError', () => { }); it('trusts the declared content type over body sniffing', () => { - // Some proxies prepend text before the markup; the header still says html. const msg = formatApiError({ status: 503, body: 'upstream connect error ...', @@ -51,21 +48,29 @@ describe('formatApiError', () => { }); it('sniffs markup when no content type is available', () => { - // No response attached (older call sites); the leading '<' is the tell. expect(formatApiError({ status: 500, body: djangoHtml500 })).toBe( - '500 - Request failed' + '500 - Internal Server Error' ); }); - it('falls back to a generic label when the reason phrase is absent', () => { - // HTTP/2+ responses carry no reason phrase (statusText === ''). + it('does not sniff markup when content type is explicitly non-html', () => { + expect( + formatApiError({ + status: 400, + body: ' is invalid', + response: fakeResponse('Bad Request', 'text/plain'), + }) + ).toBe('400 - is invalid'); + }); + + it('uses a status label when the reason phrase is absent (HTTP/2)', () => { expect( formatApiError({ status: 502, body: '', response: fakeResponse('', 'text/html'), }) - ).toBe('502 - Request failed'); + ).toBe('502 - Bad Gateway'); }); it('preserves the suppressed HTML body via console.debug', () => { @@ -82,10 +87,63 @@ describe('formatApiError', () => { spy.mockRestore(); }); - it('keeps JSON object bodies pretty-printed (existing behaviour)', () => { + it('extracts detail from DRF-style JSON bodies', () => { + expect( + formatApiError({ + status: 401, + body: { + detail: 'No active account found with the given credentials', + }, + }) + ).toBe('No active account found with the given credentials'); + }); + + it('extracts error from custom API JSON bodies', () => { expect( formatApiError({ status: 500, body: { error: 'Redis not available' } }) - ).toBe(JSON.stringify({ error: 'Redis not available' }, null, 2)); + ).toBe('Redis not available'); + }); + + it('extracts non_field_errors when detail/error are absent', () => { + expect( + formatApiError({ + status: 400, + body: { + non_field_errors: ['Unable to log in with provided credentials.'], + }, + }) + ).toBe('Unable to log in with provided credentials.'); + }); + + it('prefers detail over other JSON keys', () => { + expect( + formatApiError({ + status: 400, + body: { + detail: 'Primary message', + error: 'Secondary', + name: ['ignored when detail is present'], + }, + }) + ).toBe('Primary message'); + }); + + it('joins DRF field validation errors', () => { + expect( + formatApiError({ + status: 400, + body: { name: ['This field is required.'] }, + }) + ).toBe('name: This field is required.'); + expect( + formatApiError({ + status: 400, + body: { + name: ['This field is required.'], + url: ['Enter a valid URL.'], + }, + }) + ).toBe('name: This field is required.; url: Enter a valid URL.'); }); it('passes through short plain-text bodies with the status', () => { diff --git a/frontend/src/utils.js b/frontend/src/utils.js index 9276ccd1..c14863cd 100644 --- a/frontend/src/utils.js +++ b/frontend/src/utils.js @@ -68,51 +68,108 @@ export function useDebounce(value, delay = 500, callback = null) { return debouncedValue; } -// Human-readable message for a failed API response (#1261). Backends that -// are down or timing out answer with whole HTML error pages (Django's -// "Server Error (500)" page, nginx's 502/504 pages) - rendering those -// verbatim in a toast is unreadable. JSON error bodies keep their existing -// formatting; markup and empty bodies collapse to the response's own -// status line (the full body goes to console.debug for troubleshooting); -// long plain-text bodies are truncated. +// Format a failed API response for toast display. const ERROR_BODY_MAX_CHARS = 200; +const STATUS_LABELS = { + 400: 'Bad Request', + 401: 'Unauthorized', + 403: 'Forbidden', + 404: 'Not Found', + 405: 'Method Not Allowed', + 408: 'Request Timeout', + 409: 'Conflict', + 413: 'Payload Too Large', + 429: 'Too Many Requests', + 500: 'Internal Server Error', + 502: 'Bad Gateway', + 503: 'Service Unavailable', + 504: 'Gateway Timeout', +}; + +const formatErrorValue = (value) => { + if (value == null) return ''; + if (Array.isArray(value)) { + return value.map(formatErrorValue).filter(Boolean).join('; '); + } + if (typeof value === 'object') { + try { + return JSON.stringify(value); + } catch { + return ''; + } + } + return String(value); +}; + +const formatJsonErrorBody = (body) => { + if (Array.isArray(body)) { + return formatErrorValue(body) || null; + } + if (!body || typeof body !== 'object') { + return null; + } + + for (const key of ['detail', 'error', 'non_field_errors']) { + if (body[key] != null && body[key] !== '') { + const msg = formatErrorValue(body[key]); + if (msg) return msg; + } + } + + const fieldParts = Object.entries(body) + .map(([key, value]) => { + const msg = formatErrorValue(value); + return msg ? `${key}: ${msg}` : null; + }) + .filter(Boolean); + + if (fieldParts.length) { + return fieldParts.join('; '); + } + + try { + return JSON.stringify(body, null, 2); + } catch { + return null; + } +}; + +const formatStatusLine = (status, response) => { + const reason = response?.statusText?.trim(); + const label = reason || STATUS_LABELS[status] || 'Request failed'; + return `${status} - ${label}`; +}; + export const formatApiError = (error) => { if (!error || !error.status) { return (error && error.message) || 'Unknown error'; } - // request() attaches the fetch Response as error.response; it carries the - // canonical reason phrase and the declared content type. const { status, body, response } = error; if (body && typeof body === 'object') { - try { - return JSON.stringify(body, null, 2); - } catch { - // Unserializable object; fall through to the string handling below. - } + const formatted = formatJsonErrorBody(body); + if (formatted) return formatted; } const text = typeof body === 'string' ? body.trim() : body == null ? '' : String(body); - // Trust the declared content type first; sniff only when it is absent - // (some proxies omit or mislabel it on error pages). - const contentType = response?.headers?.get?.('content-type') || ''; + // Sniff leading '<' only when Content-Type is missing. + const contentType = ( + response?.headers?.get?.('content-type') || '' + ).toLowerCase(); const isMarkup = contentType.includes('html') || contentType.includes('xml') || - text.startsWith('<'); + (!contentType && text.startsWith('<')); if (!text || isMarkup) { if (text) { console.debug(`API error ${status} response body:`, text); } - // statusText is the protocol's reason phrase ("Bad Gateway"), defined - // for every status code. HTTP/2+ does not transmit one, so fall back - // to a generic label rather than re-enumerating the HTTP spec here. - return `${status} - ${response?.statusText || 'Request failed'}`; + return formatStatusLine(status, response); } return text.length > ERROR_BODY_MAX_CHARS