diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d883ea1..2b2fede1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -209,6 +209,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The precomputed `library-cluster.db` identity keys used for cross-library dedup are now pruned on rebuild when their track no longer exists (removed, or dropped when a server mints a fresh id on rename). Previously the rebuild only refreshed live tracks and never deleted stale rows, so the sidecar grew with library churn until it was recreated wholesale (server switch / restore / import). The rows were inert (reads only ever join live tracks), so dedup and browse results are unchanged — this just stops the sidecar from bloating. +### Sync — large play queues no longer revert after pausing + +**By [@norperz](https://github.com/norperz), PR [#1262](https://github.com/Psychotoxical/psysonic/pull/1262)** + +* Pausing a large queue behind a reverse proxy (e.g. Nginx) could snap the player back to an earlier track — the save was one long URL that hit the HTTP 414 limit, failed silently, and idle auto-pull restored the stale server queue. +* Servers advertising the OpenSubsonic `formPost` extension (Navidrome) now save via POST; others retry once as POST on 414. A failed save no longer lets auto-pull overwrite playback — it resumes only after a successful save. + ## [1.49.0] - 2026-06-29 diff --git a/src/config/settingsCredits.ts b/src/config/settingsCredits.ts index ecce3b61..b5f2bba1 100644 --- a/src/config/settingsCredits.ts +++ b/src/config/settingsCredits.ts @@ -460,6 +460,13 @@ const CONTRIBUTOR_ENTRIES = [ 'Italian (it) full UI translation (PR #1250)', ], }, + { + github: 'norperz', + since: '1.50.0', + contributions: [ + 'Sync: form POST for large play queues to avoid HTTP 414 behind reverse proxies (PR #1262)', + ], + }, ] as const; // PR number of a contributor's first listed contribution, used as the diff --git a/src/features/playback/store/playerStore.persistence.test.ts b/src/features/playback/store/playerStore.persistence.test.ts index b355f3e3..7f681643 100644 --- a/src/features/playback/store/playerStore.persistence.test.ts +++ b/src/features/playback/store/playerStore.persistence.test.ts @@ -183,7 +183,7 @@ describe('flushPlayQueuePosition', () => { seedQueue([track], { index: 0, currentTrack: track }); vi.mocked(savePlayQueue).mockRejectedValueOnce(new Error('offline')); - await expect(flushPlayQueuePosition()).resolves.toBeUndefined(); + await expect(flushPlayQueuePosition()).resolves.toBe(false); }); it('floors the position to whole milliseconds', async () => { diff --git a/src/features/playback/store/queueSync.test.ts b/src/features/playback/store/queueSync.test.ts index 7c6b67b0..dccd4849 100644 --- a/src/features/playback/store/queueSync.test.ts +++ b/src/features/playback/store/queueSync.test.ts @@ -141,6 +141,15 @@ describe('pushQueueOnPlaybackStart', () => { expect(isIdleQueuePullSuspended()).toBe(false); }); + it('keeps idle pull suspended when the immediate flush fails', async () => { + syncUserQueueMutationToServer(queue, track('a'), 30); + savePlayQueueMock.mockRejectedValueOnce(new Error('414')); + pushQueueOnPlaybackStart(queue, track('a'), 42); + await vi.runAllTimersAsync(); + expect(savePlayQueueMock).toHaveBeenCalled(); + expect(isIdleQueuePullSuspended()).toBe(true); + }); + it('debounces when idle pull is not suspended', () => { pushQueueOnPlaybackStart(queue, track('a'), 12); expect(hasPendingQueueSync()).toBe(true); @@ -148,6 +157,15 @@ describe('pushQueueOnPlaybackStart', () => { }); }); +describe('flushQueueSyncToServer failure', () => { + it('suspends idle pull when an immediate push fails', async () => { + savePlayQueueMock.mockRejectedValueOnce(new Error('offline')); + const ok = await flushQueueSyncToServer([ref('a')], track('a'), 12); + expect(ok).toBe(false); + expect(isIdleQueuePullSuspended()).toBe(true); + }); +}); + describe('flushPlayQueueForServer', () => { it('flushes only the target server slice', async () => { playerState.queueItems = [ref('a', 'srv-a'), ref('b', 'b.test')]; diff --git a/src/features/playback/store/queueSync.ts b/src/features/playback/store/queueSync.ts index cf13b93c..900f0272 100644 --- a/src/features/playback/store/queueSync.ts +++ b/src/features/playback/store/queueSync.ts @@ -8,7 +8,13 @@ import { } from '@/features/playback/utils/playback/playbackServer'; import { filterQueueRefsForServerProfile } from '@/features/playback/utils/playback/trackServerScope'; import { getPlaybackProgressSnapshot } from '@/features/playback/store/playbackProgress'; -import { touchQueueMutationClock, isIdleQueuePullSuspended, resumeIdleQueuePull, markQueueNaturallyEnded } from '@/features/playback/store/queuePlaybackIdle'; +import { + touchQueueMutationClock, + isIdleQueuePullSuspended, + resumeIdleQueuePull, + suspendIdleQueuePull, + markQueueNaturallyEnded, +} from '@/features/playback/store/queuePlaybackIdle'; import { usePlayerStore } from '@/features/playback/store/playerStore'; /** @@ -42,19 +48,26 @@ function isPlaybackServerReachable(): boolean { return serverId ? isSubsonicServerReachable(serverId) : false; } +/** @returns true when the server accepted the queue (or there was nothing to push). */ function pushRefsForServer( refs: QueueItemRef[], currentTrack: Track | null, currentTime: number, serverId: string, -): Promise { - if (!serverId || refs.length === 0 || !currentTrack) return Promise.resolve(); - if (playbackProfileIdForTrack(currentTrack) !== serverId) return Promise.resolve(); +): Promise { + if (!serverId || refs.length === 0 || !currentTrack) return Promise.resolve(true); + if (playbackProfileIdForTrack(currentTrack) !== serverId) return Promise.resolve(true); const ids = refs.slice(0, QUEUE_ID_LIMIT).map(r => r.trackId); const pos = Math.floor(currentTime * 1000); - return savePlayQueue(ids, currentTrack.id, pos, serverId).catch(() => { - // Expected when offline or the playback server is unreachable. - }); + return savePlayQueue(ids, currentTrack.id, pos, serverId).then( + () => true, + () => { + // Offline / unreachable / URI-too-long: keep local queue authoritative + // so idle auto-pull cannot rewind to the last successful server snapshot. + suspendIdleQueuePull(); + return false; + }, + ); } function scheduleQueueSyncToServer( @@ -88,13 +101,18 @@ export function syncUserQueueMutationToServer( scheduleQueueSyncToServer(queue, currentTrack, currentTime); } -export function flushQueueSyncToServer(queue: QueueItemRef[], currentTrack: Track | null, currentTime: number): Promise { +/** @returns true when the push succeeded (or was a no-op). */ +export function flushQueueSyncToServer( + queue: QueueItemRef[], + currentTrack: Track | null, + currentTime: number, +): Promise { if (syncTimeout) { clearTimeout(syncTimeout); syncTimeout = null; } - if (!isPlaybackServerReachable()) return Promise.resolve(); - if (!currentTrack || queue.length === 0) return Promise.resolve(); + if (!isPlaybackServerReachable()) return Promise.resolve(true); + if (!currentTrack || queue.length === 0) return Promise.resolve(true); lastQueueHeartbeatAt = Date.now(); const serverId = getPlaybackServerId(); const refs = filterQueueRefsForPlaybackServer(queue); @@ -105,16 +123,16 @@ export function flushQueueSyncToServer(queue: QueueItemRef[], currentTrack: Trac * Immediate flush of one server's queue slice (e.g. before browse switch). * Does not mutate local player state. */ -export function flushPlayQueueForServer(serverProfileId: string): Promise { +export function flushPlayQueueForServer(serverProfileId: string): Promise { if (syncTimeout) { clearTimeout(syncTimeout); syncTimeout = null; } - if (!serverProfileId || !isSubsonicServerReachable(serverProfileId)) return Promise.resolve(); + if (!serverProfileId || !isSubsonicServerReachable(serverProfileId)) return Promise.resolve(true); const s = usePlayerStore.getState(); - if (s.currentRadio) return Promise.resolve(); + if (s.currentRadio) return Promise.resolve(true); const refs = filterQueueRefsForServerProfile(s.queueItems, serverProfileId); - if (refs.length === 0 || !s.currentTrack) return Promise.resolve(); + if (refs.length === 0 || !s.currentTrack) return Promise.resolve(true); const currentTime = getPlaybackProgressSnapshot().currentTime; return pushRefsForServer(refs, s.currentTrack, currentTime, serverProfileId); } @@ -135,9 +153,9 @@ export function getLastQueueHeartbeatAt(): number { * live current-time via the playback-progress snapshot so the position * isn't stale by the debounced store commit. */ -export function flushPlayQueuePosition(): Promise { +export function flushPlayQueuePosition(): Promise { const s = usePlayerStore.getState(); - if (s.currentRadio) return Promise.resolve(); + if (s.currentRadio) return Promise.resolve(true); return flushQueueSyncToServer(s.queueItems, s.currentTrack, getPlaybackProgressSnapshot().currentTime); } @@ -148,7 +166,7 @@ export function flushPlayQueuePosition(): Promise { export function finalizePlayQueueAtTrackEnd( queue: QueueItemRef[], currentTrack: Track, -): Promise { +): Promise { if (syncTimeout) { clearTimeout(syncTimeout); syncTimeout = null; @@ -170,8 +188,8 @@ export function pushQueueOnPlaybackStart( ): void { if (!currentTrack || queue.length === 0) return; if (isIdleQueuePullSuspended()) { - void flushQueueSyncToServer(queue, currentTrack, currentTime).then(() => { - resumeIdleQueuePull(); + void flushQueueSyncToServer(queue, currentTrack, currentTime).then(ok => { + if (ok) resumeIdleQueuePull(); }); return; } @@ -188,8 +206,8 @@ export function flushLocalQueueWhenTakingPlayback(): Promise { s.queueItems, s.currentTrack, getPlaybackProgressSnapshot().currentTime, - ).then(() => { - resumeIdleQueuePull(); + ).then(ok => { + if (ok) resumeIdleQueuePull(); }); } diff --git a/src/lib/api/subsonicClient.formPost.test.ts b/src/lib/api/subsonicClient.formPost.test.ts new file mode 100644 index 00000000..cc6145b5 --- /dev/null +++ b/src/lib/api/subsonicClient.formPost.test.ts @@ -0,0 +1,67 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import axios, { AxiosError } from 'axios'; +import { + isHttp414, + serializeSubsonicParams, +} from '@/lib/api/subsonicClient'; + +vi.mock('axios'); + +describe('serializeSubsonicParams', () => { + it('encodes scalars and repeats array keys like axios indexes:null', () => { + expect( + serializeSubsonicParams({ + u: 'user', + id: ['a', 'b'], + current: 'a', + position: 1200, + }), + ).toBe('u=user&id=a&id=b¤t=a&position=1200'); + }); + + it('skips null and undefined values', () => { + expect(serializeSubsonicParams({ a: 1, b: null, c: undefined, d: ['x'] })).toBe('a=1&d=x'); + }); +}); + +describe('isHttp414', () => { + it('detects axios 414 responses', () => { + const err = new AxiosError('Request failed'); + err.response = { status: 414, data: '', statusText: 'URI Too Long', headers: {}, config: {} as never }; + expect(isHttp414(err)).toBe(true); + }); + + it('detects message-based URI-too-long errors', () => { + expect(isHttp414(new Error('Request-URI Too Large'))).toBe(true); + expect(isHttp414(new Error('offline'))).toBe(false); + }); +}); + +describe('apiPostFormWithCredentials', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('POSTs form-urlencoded body with path-only URL', async () => { + const { apiPostFormWithCredentials } = await import('@/lib/api/subsonicClient'); + vi.mocked(axios.post).mockResolvedValue({ + data: { 'subsonic-response': { status: 'ok' } }, + }); + + await apiPostFormWithCredentials('https://music.example:4533', 'user', 'pass', 'savePlayQueue.view', { + id: ['a', 'b'], + current: 'a', + position: 10, + }); + + expect(axios.post).toHaveBeenCalledTimes(1); + const [url, body, config] = vi.mocked(axios.post).mock.calls[0]!; + expect(url).toBe('https://music.example:4533/rest/savePlayQueue.view'); + expect(String(body)).toContain('id=a&id=b'); + expect(String(body)).toContain('current=a'); + expect(String(body)).toContain('u=user'); + expect((config as { headers?: Record }).headers?.['Content-Type']).toBe( + 'application/x-www-form-urlencoded', + ); + }); +}); diff --git a/src/lib/api/subsonicClient.ts b/src/lib/api/subsonicClient.ts index 807be086..877752c1 100644 --- a/src/lib/api/subsonicClient.ts +++ b/src/lib/api/subsonicClient.ts @@ -33,6 +33,48 @@ export function restBaseFromUrl(serverUrl: string): string { return `${base}/rest`; } +/** + * Encode Subsonic REST params the same way axios does with `paramsSerializer: { indexes: null }` + * (repeated keys for arrays: `id=a&id=b`). Used for OpenSubsonic form POST bodies. + */ +export function serializeSubsonicParams(params: Record): string { + const parts: string[] = []; + for (const [key, value] of Object.entries(params)) { + if (value === undefined || value === null) continue; + if (Array.isArray(value)) { + for (const item of value) { + if (item === undefined || item === null) continue; + parts.push(`${encodeURIComponent(key)}=${encodeURIComponent(String(item))}`); + } + continue; + } + parts.push(`${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`); + } + return parts.join('&'); +} + +function parseSubsonicResponse(respData: unknown): T { + const data = (respData as { ['subsonic-response']?: { status?: string; error?: { message?: string } } })?.[ + 'subsonic-response' + ]; + if (!data) throw new Error('Invalid response from server (possibly not a Subsonic server)'); + if (data.status !== 'ok') throw new Error(data.error?.message ?? 'Subsonic API error'); + return data as T; +} + +/** True when a reverse proxy / server rejected the request because the URI was too long. */ +export function isHttp414(err: unknown): boolean { + if (err && typeof err === 'object' && 'response' in err) { + const status = (err as { response?: { status?: number } }).response?.status; + if (status === 414) return true; + } + if (err instanceof Error) { + const msg = err.message.toLowerCase(); + return msg.includes('414') || msg.includes('uri too long') || msg.includes('request-uri too large'); + } + return false; +} + export async function apiWithCredentials( serverUrl: string, username: string, @@ -50,10 +92,33 @@ export async function apiWithCredentials( paramsSerializer: { indexes: null }, timeout, }); - const data = resp.data?.['subsonic-response']; - if (!data) throw new Error('Invalid response from server (possibly not a Subsonic server)'); - if (data.status !== 'ok') throw new Error(data.error?.message ?? 'Subsonic API error'); - return data as T; + return parseSubsonicResponse(resp.data); +} + +/** + * OpenSubsonic `formPost`: send all API args in an `application/x-www-form-urlencoded` body + * (path-only URL - no query string) so large multi-`id` calls avoid HTTP 414. + */ +export async function apiPostFormWithCredentials( + serverUrl: string, + username: string, + password: string, + endpoint: string, + extra: Record = {}, + timeout = 15000, + headerProfile?: ServerHttpHeaderProfile, +): Promise { + const params = { ...getAuthParams(username, password), ...extra }; + const headers = { + ...(headerProfile ? headersForServerRequest(headerProfile, serverUrl) : {}), + 'Content-Type': 'application/x-www-form-urlencoded', + }; + const resp = await axios.post( + `${restBaseFromUrl(serverUrl)}/${endpoint}`, + serializeSubsonicParams(params), + { headers, timeout }, + ); + return parseSubsonicResponse(resp.data); } export function getClient() { @@ -69,6 +134,12 @@ export function getServerById(serverId: string): ServerProfile | undefined { return findServerByIdOrIndexKey(serverId); } +/** True when the server advertises the OpenSubsonic `formPost` extension. */ +export function serverSupportsFormPost(serverId: string): boolean { + const exts = useAuthStore.getState().openSubsonicExtensionsByServer[serverId] ?? []; + return exts.includes('formPost'); +} + /** Subsonic REST call against an explicit saved server (not necessarily the active one). */ export async function apiForServer( serverId: string, @@ -93,6 +164,26 @@ export async function apiForServer( ); } +/** Form-POST variant of `apiForServer` (OpenSubsonic `formPost`). */ +export async function apiPostFormForServer( + serverId: string, + endpoint: string, + extra: Record = {}, + timeout = 15000, +): Promise { + const server = getServerById(serverId); + if (!server) throw new Error(`Unknown server: ${serverId}`); + return apiPostFormWithCredentials( + connectBaseUrlForServer(server), + server.username, + server.password, + endpoint, + extra, + timeout, + server, + ); +} + export async function api( endpoint: string, extra: Record = {}, @@ -111,10 +202,7 @@ export async function api( timeout, signal, }); - const data = resp.data?.['subsonic-response']; - if (!data) throw new Error('Invalid response from server (possibly not a Subsonic server)'); - if (data.status !== 'ok') throw new Error(data.error?.message ?? 'Subsonic API error'); - return data as T; + return parseSubsonicResponse(resp.data); } /** Optional `musicFolderId` when the user narrowed browsing to one Subsonic library (see `getMusicFolders`). */ diff --git a/src/lib/api/subsonicPlayQueue.test.ts b/src/lib/api/subsonicPlayQueue.test.ts new file mode 100644 index 00000000..3dc42db6 --- /dev/null +++ b/src/lib/api/subsonicPlayQueue.test.ts @@ -0,0 +1,81 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { AxiosError } from 'axios'; + +const { apiForServerMock, apiPostFormForServerMock, authState } = vi.hoisted(() => ({ + apiForServerMock: vi.fn(async () => ({ status: 'ok' })), + apiPostFormForServerMock: vi.fn(async () => ({ status: 'ok' })), + authState: { + openSubsonicExtensionsByServer: {} as Record, + }, +})); + +vi.mock('@/lib/api/subsonicClient', async () => { + const actual = await vi.importActual('@/lib/api/subsonicClient'); + return { + ...actual, + api: vi.fn(), + apiForServer: apiForServerMock, + apiPostFormForServer: apiPostFormForServerMock, + }; +}); + +vi.mock('@/store/authStore', () => ({ + useAuthStore: { + getState: () => authState, + }, +})); + +import { savePlayQueue } from '@/lib/api/subsonicPlayQueue'; + +beforeEach(() => { + apiForServerMock.mockReset(); + apiPostFormForServerMock.mockReset(); + apiForServerMock.mockResolvedValue({ status: 'ok' }); + apiPostFormForServerMock.mockResolvedValue({ status: 'ok' }); + authState.openSubsonicExtensionsByServer = {}; +}); + +describe('savePlayQueue transport', () => { + it('uses form POST when formPost is advertised', async () => { + authState.openSubsonicExtensionsByServer = { 'srv-a': ['formPost', 'playbackReport'] }; + await savePlayQueue(['a', 'b'], 'a', 1000, 'srv-a'); + expect(apiPostFormForServerMock).toHaveBeenCalledWith('srv-a', 'savePlayQueue.view', { + id: ['a', 'b'], + current: 'a', + position: 1000, + }); + expect(apiForServerMock).not.toHaveBeenCalled(); + }); + + it('uses GET when formPost is not advertised', async () => { + authState.openSubsonicExtensionsByServer = { 'srv-a': ['playbackReport'] }; + await savePlayQueue(['a'], 'a', 0, 'srv-a'); + expect(apiForServerMock).toHaveBeenCalledWith('srv-a', 'savePlayQueue.view', { + id: ['a'], + current: 'a', + position: 0, + }); + expect(apiPostFormForServerMock).not.toHaveBeenCalled(); + }); + + it('retries once as POST after HTTP 414 on GET', async () => { + const err = new AxiosError('Request failed'); + err.response = { status: 414, data: '', statusText: 'URI Too Long', headers: {}, config: {} as never }; + apiForServerMock.mockRejectedValueOnce(err); + + await savePlayQueue(['a', 'b'], 'a', 50, 'srv-a'); + + expect(apiForServerMock).toHaveBeenCalledTimes(1); + expect(apiPostFormForServerMock).toHaveBeenCalledWith('srv-a', 'savePlayQueue.view', { + id: ['a', 'b'], + current: 'a', + position: 50, + }); + }); + + it('does not retry POST on non-414 GET failures', async () => { + apiForServerMock.mockRejectedValueOnce(new Error('offline')); + await expect(savePlayQueue(['a'], 'a', 0, 'srv-a')).rejects.toThrow('offline'); + expect(apiPostFormForServerMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/api/subsonicPlayQueue.ts b/src/lib/api/subsonicPlayQueue.ts index 1bfaaa71..f4d1484e 100644 --- a/src/lib/api/subsonicPlayQueue.ts +++ b/src/lib/api/subsonicPlayQueue.ts @@ -1,4 +1,4 @@ -import { api, apiForServer } from '@/lib/api/subsonicClient'; +import { api, apiForServer, apiPostFormForServer, isHttp414, serverSupportsFormPost } from '@/lib/api/subsonicClient'; import type { SubsonicSong } from '@/lib/api/subsonicTypes'; export type PlayQueueResult = { current?: string; position?: number; songs: SubsonicSong[] }; @@ -32,6 +32,11 @@ export async function getPlayQueueForServer(serverId: string): Promise 0) params.id = songIds; if (current !== undefined) params.current = current; if (position !== undefined) params.position = position; - await apiForServer(serverId, 'savePlayQueue.view', params); + + if (serverSupportsFormPost(serverId)) { + await apiPostFormForServer(serverId, 'savePlayQueue.view', params); + return; + } + + try { + await apiForServer(serverId, 'savePlayQueue.view', params); + } catch (err) { + if (isHttp414(err)) { + await apiPostFormForServer(serverId, 'savePlayQueue.view', params); + return; + } + throw err; + } }