diff --git a/CHANGELOG.md b/CHANGELOG.md index ee17d2be..e3fb7c7d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Frontend date/time unit tests no longer fail outside UTC / en-US.** Three tests encoded the machine timezone or locale into their expectations (`dateTimeUtils.isSame`, `RecordingUtils.toDateString`, `SeriesModalUtils.getEpisodeAirdate`), so the suite failed on boxes east of UTC or non-US locales. Fixtures now use local calendar datetimes and locale-computed expectations so the suite passes in every environment. — Thanks [@nagelm](https://github.com/nagelm) +- **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 new file mode 100644 index 00000000..d1e92ef9 --- /dev/null +++ b/frontend/src/__tests__/formatApiError.test.js @@ -0,0 +1,196 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { formatApiError } from '../utils'; + +const djangoHtml500 = + ' Server Error (500)

Server Error (500)

'; + +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', () => { + 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', () => { + expect(formatApiError({ status: 500, body: djangoHtml500 })).toBe( + '500 - Internal Server Error' + ); + }); + + 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 - Bad Gateway'); + }); + + 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('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('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', () => { + 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..c14863cd 100644 --- a/frontend/src/utils.js +++ b/frontend/src/utils.js @@ -68,6 +68,115 @@ export function useDebounce(value, delay = 500, callback = null) { return debouncedValue; } +// 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'; + } + + const { status, body, response } = error; + + if (body && typeof body === 'object') { + const formatted = formatJsonErrorBody(body); + if (formatted) return formatted; + } + + const text = + typeof body === 'string' ? body.trim() : body == null ? '' : String(body); + + // Sniff leading '<' only when Content-Type is missing. + const contentType = ( + response?.headers?.get?.('content-type') || '' + ).toLowerCase(); + const isMarkup = + contentType.includes('html') || + contentType.includes('xml') || + (!contentType && text.startsWith('<')); + + if (!text || isMarkup) { + if (text) { + console.debug(`API error ${status} response body:`, text); + } + return formatStatusLine(status, response); + } + + 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); diff --git a/frontend/src/utils/__tests__/dateTimeUtils.test.js b/frontend/src/utils/__tests__/dateTimeUtils.test.js index 3bc642af..0e841074 100644 --- a/frontend/src/utils/__tests__/dateTimeUtils.test.js +++ b/frontend/src/utils/__tests__/dateTimeUtils.test.js @@ -132,14 +132,14 @@ describe('dateTimeUtils', () => { describe('isSame', () => { it('should return true when dates are same day', () => { - const date1 = '2024-01-15T10:00:00Z'; - const date2 = '2024-01-15T11:00:00Z'; + const date1 = '2024-01-15 10:00:00'; + const date2 = '2024-01-15 11:00:00'; expect(dateTimeUtils.isSame(date1, date2)).toBe(true); }); it('should return false when dates are different days', () => { - const date1 = '2024-01-15T10:00:00Z'; - const date2 = '2024-01-16T10:00:00Z'; + const date1 = '2024-01-15 10:00:00'; + const date2 = '2024-01-16 10:00:00'; expect(dateTimeUtils.isSame(date1, date2)).toBe(false); }); diff --git a/frontend/src/utils/components/__tests__/SeriesModalUtils.test.js b/frontend/src/utils/components/__tests__/SeriesModalUtils.test.js index d986cfcc..5f4f4490 100644 --- a/frontend/src/utils/components/__tests__/SeriesModalUtils.test.js +++ b/frontend/src/utils/components/__tests__/SeriesModalUtils.test.js @@ -353,7 +353,7 @@ describe('SeriesModalUtils', () => { it('should format valid air date', () => { const episode = { air_date: '2024-01-15' }; const formatted = getEpisodeAirdate(episode); - expect(formatted).toMatch(/1\/1[4|5]\/2024/); + expect(formatted).toBe(new Date('2024-01-15').toLocaleDateString()); }); it('should return N/A for missing air date', () => { diff --git a/frontend/src/utils/forms/__tests__/RecordingUtils.test.js b/frontend/src/utils/forms/__tests__/RecordingUtils.test.js index 6e3ae901..db74a1f9 100644 --- a/frontend/src/utils/forms/__tests__/RecordingUtils.test.js +++ b/frontend/src/utils/forms/__tests__/RecordingUtils.test.js @@ -111,7 +111,7 @@ describe('RecordingUtils', () => { describe('toDateString', () => { it('formats a Date to YYYY-MM-DD', () => { - const d = new Date('2024-06-15T12:00:00Z'); + const d = new Date(2024, 5, 15, 12, 0, 0); const result = toDateString(d); expect(result).toBe('2024-06-15'); });