From ad6e8eb069756183dcd8604e8d74e9939e23de59 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 19:59:42 +0200 Subject: [PATCH 01/12] backend-api: stop /api/data, /api/mupihat, /api/wlan hanging without file; 500 instead of throw on /api/addwlan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three GET endpoints had `if (fs.existsSync(file)) { ...readFile... }` with no else branch, so when the file was missing the request just sat there with no response — the frontend's loading spinner spun forever and HTTP keep-alive eventually timed it out client-side. Mirror the /api/resume pattern (already fixed earlier): early-return [] (or {} for /api/mupihat which is an object) when the file is absent. /api/data hits this on a fresh box before active_data.json is generated; /api/mupihat hits it on every box without a MuPiHAT board; /api/wlan hits it before the first scan. /api/addwlan ran `jsonfile.writeFile(...)` and inside the node-style callback did `if (writeError) throw error` (note: throwing the OUTER `error` variable from the read step, not even the relevant one). Throws inside async callbacks bypass Express's error middleware and crash the backend-api process — the player goes offline until pm2 restarts it. Send a 500 with a log line instead, the same shape every other write endpoint already uses. --- src/backend-api/src/server.ts | 88 ++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 32 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 51399c7d..ca655f66 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -120,17 +120,22 @@ app.get('/api/rssfeed', async (req, res) => { }) app.get('/api/data', (_req, res) => { - if (fs.existsSync(activedataFile)) { - jsonfile.readFile(activedataFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/data read active_data.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.json([]) - } else { - res.json(data) - } - }) + // Mirror /api/resume: when active_data.json is missing the frontend would + // otherwise hang on its loading spinner, because the previous code's missing + // else-branch never sent a response. + if (!fs.existsSync(activedataFile)) { + res.json([]) + return } + jsonfile.readFile(activedataFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/data read active_data.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json([]) + } else { + res.json(data) + } + }) }) app.get('/api/resume', (_req, res) => { @@ -150,17 +155,22 @@ app.get('/api/resume', (_req, res) => { }) app.get('/api/mupihat', (_req, res) => { - if (fs.existsSync(mupihat)) { - jsonfile.readFile(mupihat, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/mupihat read mupihat.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.json([]) - } else { - res.json(data) - } - }) + // Same hang-without-file as /api/data: a box without a MuPiHAT board + // simply has no /tmp/mupihat.json — return an empty object rather than + // letting the request stall. + if (!fs.existsSync(mupihat)) { + res.json({}) + return } + jsonfile.readFile(mupihat, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/mupihat read mupihat.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json({}) + } else { + res.json(data) + } + }) }) app.get('/api/activeresume', (_req, res) => { @@ -231,17 +241,21 @@ app.get('/api/albumstop', (_req, res) => { }) app.get('/api/wlan', (_req, res) => { - if (fs.existsSync(wlanFile)) { - jsonfile.readFile(wlanFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/wlan read wlan.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.json([]) - } else { - res.json(data) - } - }) + // Same shape as /api/data and /api/mupihat — empty array when the file + // hasn't been written yet, instead of an open connection that never closes. + if (!fs.existsSync(wlanFile)) { + res.json([]) + return } + jsonfile.readFile(wlanFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/wlan read wlan.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json([]) + } else { + res.json(data) + } + }) }) app.post('/api/addwlan', (req, res) => { @@ -251,8 +265,18 @@ app.post('/api/addwlan', (req, res) => { if (error) out = [] out.push(req.body) - jsonfile.writeFile(wlanFile, out, { spaces: 4 }, (error) => { - if (error) throw error + jsonfile.writeFile(wlanFile, out, { spaces: 4 }, (writeError) => { + // The previous code did `if (writeError) throw error` — async-throw + // inside a node-style callback isn't catchable by Express, so it + // crashed the entire backend-api process. Send a 500 instead. + if (writeError) { + console.error( + `${new Date().toLocaleString()}: [MuPiBox-Server] /api/addwlan write failed:`, + writeError, + ) + res.status(500).send('Failed to persist WLAN entry') + return + } res.status(200).send('ok') }) }) From 778597663bb35e5fb521946e8bba10cdc193c6ca Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 10:01:24 +0200 Subject: [PATCH 02/12] backend-api: fix resume lock race + use composite key for dedup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The unlink in /api/addresume and /api/editresume ran outside the async readFile callback, so the lock was released before the write completed — two concurrent calls could clobber each other. Move unlink into the writeFile callback (success and error paths) and short-circuit on lock acquisition failure. Dedup matched on `req.body.id` only, but Media.id is optional — Library and RSS items often have no id, so id-less entries either collapsed onto the first match or piled up duplicates. Replace with a stable composite key (type + playlistid|showid|audiobookid|id || artist::title) and use it in both endpoints. /api/editresume keeps a defensive index fallback for the edge case where the caller really has a usable index. --- src/backend-api/src/server.ts | 184 +++++++++++++++++++--------------- 1 file changed, 104 insertions(+), 80 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index ca655f66..a492ed50 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -315,48 +315,70 @@ app.post('/api/add', (req, res) => { } }) -app.post('/api/addresume', (req, res) => { - try { - if (fs.existsSync(resumeLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume resume.json is locked`) - res.status(200).send('locked') - } else { - fs.openSync(resumeLock, 'w') - jsonfile.readFile(resumeFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/add read resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(200).send('error') - } else { - // Index des vorhandenen Eintrags mit derselben "id" finden - const index = data.findIndex((item: { id: any }) => item.id === req.body.id) +// Lock cleanup — used in addresume/editresume to ensure the lock is always +// removed once the read+write cycle has finished (success OR error). +// Previously the unlink ran outside the readFile callback, so the lock was gone +// before the write started — two concurrent calls could clobber each other. +const releaseResumeLock = (context: string) => { + fs.unlink(resumeLock, (err) => { + if (err && (err as NodeJS.ErrnoException).code !== 'ENOENT') { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} - failed to unlink resume lock:`, err) + } + }) +} - if (index !== -1) { - // Wenn der Eintrag vorhanden ist, ersetze ihn - data[index] = req.body - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Entry with id ${req.body.id} replaced.`) - } else { - // Wenn der Eintrag nicht vorhanden ist, füge ihn hinzu - data.push(req.body) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] New entry with id ${req.body.id} added.`) - } +// Stable composite key for resume entries. Plain `id` matching is unreliable +// because Library/RSS items often don't carry an `id`; without a stable key +// every id-less save would either overwrite the first id-less entry or pile +// up duplicates. Mirror the resolution order frontend uses to dispatch +// playback (playlistid/showid/audiobookid/id), and fall back to artist::title +// as a last resort. +const resumeKeyOf = (m: { type?: string; id?: string; playlistid?: string; showid?: string; audiobookid?: string; artist?: string; title?: string }) => + [ + m?.type || '', + m?.playlistid || m?.showid || m?.audiobookid || m?.id || `${m?.artist || ''}::${m?.title || ''}`, + ].join('|') - jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (error) => { - if (error) throw error - res.status(200).send('ok') - }) - } - }) - fs.unlink(resumeLock, (err) => { - if (err) throw err - console.log( - `${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume - resume.json unlocked, locked file deleted!`, - ) - }) - } +app.post('/api/addresume', (req, res) => { + if (fs.existsSync(resumeLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume resume.json is locked`) + res.status(200).send('locked') + return + } + try { + fs.openSync(resumeLock, 'w') } catch (err) { - console.error(err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume failed to acquire lock:`, err) + res.status(200).send('error') + return } + jsonfile.readFile(resumeFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/addresume read resume.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + releaseResumeLock('/api/addresume') + res.status(200).send('error') + return + } + const incomingKey = resumeKeyOf(req.body) + const index = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) + if (index !== -1) { + data[index] = req.body + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced (key=${incomingKey}).`) + } else { + data.push(req.body) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry added (key=${incomingKey}).`) + } + jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { + releaseResumeLock('/api/addresume') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume write failed:`, writeError) + res.status(500).send('error') + return + } + res.status(200).send('ok') + }) + }) }) app.post('/api/delete', (req, res) => { @@ -426,51 +448,53 @@ app.post('/api/edit', (req, res) => { }) app.post('/api/editresume', (req, res) => { + if (fs.existsSync(resumeLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume resume.json is locked`) + res.status(200).send('locked') + return + } try { - if (fs.existsSync(resumeLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume resume.json is locked`) - res.status(200).send('locked') - } else { - fs.openSync(resumeLock, 'w') - jsonfile.readFile(resumeFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/editresume read resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(200).send('error') - } else { - // Prüfe, ob die ID bereits im Array existiert - const existingIndex = data.findIndex((item: { id: any }) => item.id === req.body.data.id) - - if (existingIndex !== -1) { - // Ersetze den vorhandenen Eintrag mit derselben ID - data[existingIndex] = req.body.data - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Entry with id ${req.body.data.id} replaced.`) - } else { - // Bestimme den zu verwendenden Index basierend auf der Array-Länge - const indexToReplace = Math.min(req.body.index, data.length - 1) - - // Ersetze den Eintrag am berechneten Index oder füge hinzu - data.splice(indexToReplace, 1, req.body.data) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Entry at index ${indexToReplace} replaced.`) - } - - // Speichere die geänderten Daten zurück in die Datei - jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (error) => { - if (error) throw error - res.status(200).send('ok') - }) - } - }) - fs.unlink(resumeLock, (err) => { - if (err) throw err - console.log( - `${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume - resume.json unlocked, locked file deleted!`, - ) - }) - } + fs.openSync(resumeLock, 'w') } catch (err) { - console.error(err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume failed to acquire lock:`, err) + res.status(200).send('error') + return } + jsonfile.readFile(resumeFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/editresume read resume.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + releaseResumeLock('/api/editresume') + res.status(200).send('error') + return + } + const incomingKey = resumeKeyOf(req.body.data) + const existingIndex = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) + if (existingIndex !== -1) { + data[existingIndex] = req.body.data + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced (key=${incomingKey}).`) + } else if (Number.isInteger(req.body.index) && req.body.index >= 0 && data.length > 0) { + // Frontend uses index=99 as a sentinel for "just added, dunno actual index"; + // with the composite-key match above, that path now finds the real entry. + // Fall back to splice only if the caller really gave us a valid index — and + // never on an empty array. + const indexToReplace = Math.min(req.body.index, data.length - 1) + data.splice(indexToReplace, 1, req.body.data) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced at index ${indexToReplace} (no key match).`) + } else { + data.push(req.body.data) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry appended (no key match, no usable index, key=${incomingKey}).`) + } + jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { + releaseResumeLock('/api/editresume') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume write failed:`, writeError) + res.status(500).send('error') + return + } + res.status(200).send('ok') + }) + }) }) app.get('/api/spotify/config', (_req, res) => { From f12ee5cec1a0d5cb80b4f0731aac92c18fdfac43 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 10:08:44 +0200 Subject: [PATCH 03/12] backend-api: /api/activeresume returns [] when symlink missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit active_resume.json is a symlink set up by check_network.sh once the network state is known. In the window between boot and that script's first iteration the symlink doesn't exist yet — the previous code took neither branch and the response hung, leaving the resume page stuck on Loading. Return an empty array in that case, mirroring the existing read-error fallback. --- src/backend-api/src/server.ts | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index a492ed50..955ae5af 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -174,17 +174,23 @@ app.get('/api/mupihat', (_req, res) => { }) app.get('/api/activeresume', (_req, res) => { - if (fs.existsSync(activeresumeFile)) { - jsonfile.readFile(activeresumeFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/activeresume read active_resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.json([]) - } else { - res.json(data) - } - }) + // active_resume.json is a symlink that scripts/mupibox/check_network.sh + // creates the first time the network state is determined. Until that runs + // (briefly after boot) the symlink is missing — without an explicit empty + // response the request hung silently and the resume page stuck on Loading. + if (!fs.existsSync(activeresumeFile)) { + res.json([]) + return } + jsonfile.readFile(activeresumeFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/activeresume read active_resume.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json([]) + } else { + res.json(data) + } + }) }) app.get('/api/network', (_req, res) => { From c0fa3620d004e2a5129d635a17af0c748957fa4a Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 10:10:15 +0200 Subject: [PATCH 04/12] backend-api: /api/resume returns [] when file missing instead of 404 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Callers of /api/resume (resume.page lookup, addresume read pre-step) always expect an array. On a fresh box resume.json doesn't exist until check_network.sh creates it or the first save lands, and a 404 in that window crashed any caller doing .length / .findIndex on the response. Return an empty array — same convention as /api/data and (after the previous commit) /api/activeresume. Read errors now also degrade to [] instead of 500 to keep the contract uniform. --- src/backend-api/src/server.ts | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 955ae5af..bf761d29 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -139,19 +139,23 @@ app.get('/api/data', (_req, res) => { }) app.get('/api/resume', (_req, res) => { - if (fs.existsSync(resumeFile)) { - tryReadFile(resumeFile) - .then((data) => { - res.json(data) - }) - .catch((error) => { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/resume read resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(500).send('Internal Server Error') - }) - } else { - res.status(404).send(`File Not Found: ${resumeFile}`) + // Mirror /api/data and /api/activeresume: callers always expect an array. + // Until the first save resume.json doesn't exist (created on demand by + // check_network.sh or by the first /api/addresume), and the previous 404 + // crashed callers that did `.length` / `.findIndex` on the response. + if (!fs.existsSync(resumeFile)) { + res.json([]) + return } + tryReadFile(resumeFile) + .then((data) => { + res.json(data) + }) + .catch((error) => { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/resume read resume.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json([]) + }) }) app.get('/api/mupihat', (_req, res) => { From 45204a3f73175fadb4846aac1272c7bb1d0cff37 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 10:11:30 +0200 Subject: [PATCH 05/12] backend-api: fix lock race in /api/add /delete /edit (data.json) Same bug as the previous resume-lock fix: fs.unlink on dataLock ran outside the async readFile callback, so the lock was released before the write completed. Two concurrent admin saves could clobber each other. Generalised releaseResumeLock into releaseLock(lockPath, context) and rewrote the three data.json endpoints to release the lock from inside the writeFile callback (success and read-error paths), with explicit short-circuiting on lock-acquisition failure. Resume endpoints now use the same shared helper. --- src/backend-api/src/server.ts | 188 +++++++++++++++++----------------- 1 file changed, 96 insertions(+), 92 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index bf761d29..045a3b0e 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -293,46 +293,48 @@ app.post('/api/addwlan', (req, res) => { }) app.post('/api/add', (req, res) => { + if (fs.existsSync(dataLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add data.json is locked`) + res.status(200).send('locked') + return + } try { - if (fs.existsSync(dataLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add data.json is locked`) - res.status(200).send('locked') - } else { - fs.openSync(dataLock, 'w') - jsonfile.readFile(dataFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/add read data.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(200).send('error') - } else { - data.push(req.body) - - jsonfile.writeFile(dataFile, data, { spaces: 4 }, (error) => { - if (error) throw error - res.status(200).send('ok') - }) - } - }) - fs.unlink(dataLock, (err) => { - if (err) throw err - console.log( - `${new Date().toLocaleString()}: [MuPiBox-Server] /api/add - data.json unlocked, locked file deleted!`, - ) - }) - } + fs.openSync(dataLock, 'w') } catch (err) { - console.error(err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add failed to acquire lock:`, err) + res.status(200).send('error') + return } + jsonfile.readFile(dataFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/add read data.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + releaseLock(dataLock, '/api/add') + res.status(200).send('error') + return + } + data.push(req.body) + jsonfile.writeFile(dataFile, data, { spaces: 4 }, (writeError) => { + releaseLock(dataLock, '/api/add') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add write failed:`, writeError) + res.status(500).send('error') + return + } + res.status(200).send('ok') + }) + }) }) -// Lock cleanup — used in addresume/editresume to ensure the lock is always -// removed once the read+write cycle has finished (success OR error). -// Previously the unlink ran outside the readFile callback, so the lock was gone -// before the write started — two concurrent calls could clobber each other. -const releaseResumeLock = (context: string) => { - fs.unlink(resumeLock, (err) => { +// Lock cleanup — used by every read-modify-write endpoint (data.json + resume.json) +// to ensure the lock is always removed once the read+write cycle has finished +// (success OR error). The historical pattern called fs.unlink outside the async +// readFile callback, so the lock was gone before the write started — two +// concurrent calls could clobber each other. +const releaseLock = (lockPath: string, context: string) => { + fs.unlink(lockPath, (err) => { if (err && (err as NodeJS.ErrnoException).code !== 'ENOENT') { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} - failed to unlink resume lock:`, err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} - failed to unlink lock:`, err) } }) } @@ -366,7 +368,7 @@ app.post('/api/addresume', (req, res) => { if (error) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/addresume read resume.json`) console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - releaseResumeLock('/api/addresume') + releaseLock(resumeLock, '/api/addresume') res.status(200).send('error') return } @@ -380,7 +382,7 @@ app.post('/api/addresume', (req, res) => { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry added (key=${incomingKey}).`) } jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { - releaseResumeLock('/api/addresume') + releaseLock(resumeLock, '/api/addresume') if (writeError) { console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume write failed:`, writeError) res.status(500).send('error') @@ -392,69 +394,71 @@ app.post('/api/addresume', (req, res) => { }) app.post('/api/delete', (req, res) => { + if (fs.existsSync(dataLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete data.json is locked`) + res.status(200).send('locked') + return + } try { - if (fs.existsSync(dataLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete data.json is locked`) - res.status(200).send('locked') - } else { - fs.openSync(dataLock, 'w') - jsonfile.readFile(dataFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/delete read data.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(200).send('error') - } else { - data.splice(req.body.index, 1) - - jsonfile.writeFile(dataFile, data, { spaces: 4 }, (error) => { - if (error) throw error - res.status(200).send('ok') - }) - } - }) - fs.unlink(dataLock, (err) => { - if (err) throw err - console.log( - `${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete - data.json unlocked, locked file deleted!`, - ) - }) - } + fs.openSync(dataLock, 'w') } catch (err) { - console.error(err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete failed to acquire lock:`, err) + res.status(200).send('error') + return } + jsonfile.readFile(dataFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/delete read data.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + releaseLock(dataLock, '/api/delete') + res.status(200).send('error') + return + } + data.splice(req.body.index, 1) + jsonfile.writeFile(dataFile, data, { spaces: 4 }, (writeError) => { + releaseLock(dataLock, '/api/delete') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete write failed:`, writeError) + res.status(500).send('error') + return + } + res.status(200).send('ok') + }) + }) }) app.post('/api/edit', (req, res) => { + if (fs.existsSync(dataLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit data.json is locked`) + res.status(200).send('locked') + return + } try { - if (fs.existsSync(dataLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit data.json is locked`) - res.status(200).send('locked') - } else { - fs.openSync(dataLock, 'w') - jsonfile.readFile(dataFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/edit read data.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - res.status(200).send('error') - } else { - data.splice(req.body.index, 1, req.body.data) - - jsonfile.writeFile(dataFile, data, { spaces: 4 }, (error) => { - if (error) throw error - res.status(200).send('ok') - }) - } - }) - fs.unlink(dataLock, (err) => { - if (err) throw err - console.log( - `${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit - data.json unlocked, locked file deleted!`, - ) - }) - } + fs.openSync(dataLock, 'w') } catch (err) { - console.error(err) + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit failed to acquire lock:`, err) + res.status(200).send('error') + return } + jsonfile.readFile(dataFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/edit read data.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + releaseLock(dataLock, '/api/edit') + res.status(200).send('error') + return + } + data.splice(req.body.index, 1, req.body.data) + jsonfile.writeFile(dataFile, data, { spaces: 4 }, (writeError) => { + releaseLock(dataLock, '/api/edit') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit write failed:`, writeError) + res.status(500).send('error') + return + } + res.status(200).send('ok') + }) + }) }) app.post('/api/editresume', (req, res) => { @@ -474,7 +478,7 @@ app.post('/api/editresume', (req, res) => { if (error) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/editresume read resume.json`) console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - releaseResumeLock('/api/editresume') + releaseLock(resumeLock, '/api/editresume') res.status(200).send('error') return } @@ -496,7 +500,7 @@ app.post('/api/editresume', (req, res) => { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry appended (no key match, no usable index, key=${incomingKey}).`) } jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { - releaseResumeLock('/api/editresume') + releaseLock(resumeLock, '/api/editresume') if (writeError) { console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume write failed:`, writeError) res.status(500).send('error') From 1168c92a47577c03f67897f42ebb54ed5103f7d1 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 10:15:08 +0200 Subject: [PATCH 06/12] backend-api: self-heal corrupt or missing resume.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit addresume/editresume bailed out with "error" on any read failure — both the ENOENT case (fresh box, never saved) and the JSON-parse case (one bad write or filesystem hiccup leaves the file unparseable). Either way, no further save would land until somebody manually fixed the file. Add readResumeOrRecover(): missing file is treated as []; corrupt file is renamed to resume.json.bak. (so it can be inspected later) and the live save proceeds against an empty array. Losing one session of accumulated entries on rare corruption beats wedging the feature for the rest of the box's lifetime. --- src/backend-api/src/server.ts | 53 ++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 16 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 045a3b0e..813ab610 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -351,6 +351,41 @@ const resumeKeyOf = (m: { type?: string; id?: string; playlistid?: string; showi m?.playlistid || m?.showid || m?.audiobookid || m?.id || `${m?.artist || ''}::${m?.title || ''}`, ].join('|') +// Resilient resume.json reader. ENOENT (fresh box, file not yet created) and +// JSON parse errors both used to leave the endpoint stuck — every save would +// 200 "error" until somebody manually fixed the file. Now: missing file is +// treated as "[]"; corrupt file is moved aside to resume.json.bak. +// (so it can still be inspected) and the live save proceeds against an empty +// array. The contract is "next save lands no matter what" — losing one +// session of accumulated resume entries on rare corruption beats wedging the +// feature for the rest of the box's lifetime. +const readResumeOrRecover = (context: string, cb: (data: any[]) => void) => { + jsonfile.readFile(resumeFile, (error, data) => { + if (!error) { + cb(Array.isArray(data) ? data : []) + return + } + const code = (error as NodeJS.ErrnoException).code + if (code === 'ENOENT') { + cb([]) + return + } + const backupPath = `${resumeFile}.bak.${Date.now()}` + fs.rename(resumeFile, backupPath, (renameErr) => { + if (renameErr) { + console.error( + `${new Date().toLocaleString()}: [MuPiBox-Server] ${context} - resume.json unreadable and archive failed (${renameErr.message}); starting fresh.`, + ) + } else { + console.warn( + `${new Date().toLocaleString()}: [MuPiBox-Server] ${context} - resume.json was unreadable, archived to ${backupPath} and starting fresh. Original error: ${error.message}`, + ) + } + cb([]) + }) + }) +} + app.post('/api/addresume', (req, res) => { if (fs.existsSync(resumeLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume resume.json is locked`) @@ -364,14 +399,7 @@ app.post('/api/addresume', (req, res) => { res.status(200).send('error') return } - jsonfile.readFile(resumeFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/addresume read resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - releaseLock(resumeLock, '/api/addresume') - res.status(200).send('error') - return - } + readResumeOrRecover('/api/addresume', (data) => { const incomingKey = resumeKeyOf(req.body) const index = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) if (index !== -1) { @@ -474,14 +502,7 @@ app.post('/api/editresume', (req, res) => { res.status(200).send('error') return } - jsonfile.readFile(resumeFile, (error, data) => { - if (error) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/editresume read resume.json`) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) - releaseLock(resumeLock, '/api/editresume') - res.status(200).send('error') - return - } + readResumeOrRecover('/api/editresume', (data) => { const incomingKey = resumeKeyOf(req.body.data) const existingIndex = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) if (existingIndex !== -1) { From 7b214e1bf9730358ab69821f60045c5423ce27ab Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Thu, 7 May 2026 11:08:52 +0200 Subject: [PATCH 07/12] drop resume entry when library album finishes naturally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A "weiterhören" suggestion at 99% of the last track is just amusing — the kid finished the audiobook. Hook the existing mplayer playlist-finish event in the backend-player to POST /api/deleteresume to the backend-api with a minimal {type, artist, title} body; the new endpoint matches by composite key (same logic as addresume) and idempotently removes the entry. Body is best-effort — if the call fails, the resume entry just stays around, no playback impact. Spotify and RSS are intentionally skipped: Spotify has no clean end-of- album signal through the mplayer-wrapper, and currentMeta doesn't carry enough state for an RSS feed's composite key. Both can be revisited if they actually become a problem in practice. --- src/backend-api/src/server.ts | 43 +++++++++++++++++++ src/backend-player/src/spotify-control.js | 50 +++++++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 813ab610..85bbbd36 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -421,6 +421,49 @@ app.post('/api/addresume', (req, res) => { }) }) +// Drop a single resume entry by composite key. Used by the backend-player +// when a library playlist finishes naturally (mplayer playlist-finish) so +// "weiterhören" doesn't keep offering the position of an album the kid has +// listened all the way through. Body shape mirrors a Media (only the key +// fields matter — type + one of playlistid/showid/audiobookid/id, or +// artist::title as a fallback). Idempotent: if no entry matches, 200 ok. +app.post('/api/deleteresume', (req, res) => { + if (fs.existsSync(resumeLock)) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume resume.json is locked`) + res.status(200).send('locked') + return + } + try { + fs.openSync(resumeLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume failed to acquire lock:`, err) + res.status(200).send('error') + return + } + readResumeOrRecover('/api/deleteresume', (data) => { + const targetKey = resumeKeyOf(req.body) + const remaining = data.filter((item: any) => resumeKeyOf(item) !== targetKey) + if (remaining.length === data.length) { + releaseLock(resumeLock, '/api/deleteresume') + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume no entry matched (key=${targetKey}).`) + res.status(200).send('ok') + return + } + jsonfile.writeFile(resumeFile, remaining, { spaces: 4 }, (writeError) => { + releaseLock(resumeLock, '/api/deleteresume') + if (writeError) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume write failed:`, writeError) + res.status(500).send('error') + return + } + console.log( + `${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry removed (key=${targetKey}, ${data.length - remaining.length} match(es)).`, + ) + res.status(200).send('ok') + }) + }) +}) + app.post('/api/delete', (req, res) => { if (fs.existsSync(dataLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete data.json is locked`) diff --git a/src/backend-player/src/spotify-control.js b/src/backend-player/src/spotify-control.js index 732da894..23bc7267 100644 --- a/src/backend-player/src/spotify-control.js +++ b/src/backend-player/src/spotify-control.js @@ -122,6 +122,56 @@ player.on('track-change', () => { cmdCall('/usr/bin/python3 /usr/local/bin/mupibox/telegram_Track_Local.py') }) +player.on('playlist-finish', () => { + // Library album finished naturally — drop its resume entry so the user + // isn't offered "weiterhören" at the very end next time. Spotify and RSS + // are skipped: Spotify gives no clean end-of-album signal via the + // mplayer wrapper anyway, and RSS isn't tracked with enough metadata in + // currentMeta to build a composite key. + deleteResumeForFinishedLibraryAlbum() +}) + +// POSTs a minimal Media-shape body to /api/deleteresume so the backend-api +// removes the matching resume.json entry by composite key. Best-effort: a +// failure here just leaves a stale resume entry, no playback impact. +function deleteResumeForFinishedLibraryAlbum() { + if (currentMeta.currentPlayer !== 'mplayer') return + if (currentMeta.currentType !== 'local') return + const rawPath = currentMeta.path + if (!rawPath) return + const parts = String(rawPath).split('/') + if (parts.length < 3) return + const body = JSON.stringify({ + type: 'library', + artist: decodeURIComponent(parts[1]), + title: decodeURIComponent(parts[2]), + }) + const req = http.request( + { + host: '127.0.0.1', + port: 8200, + path: '/api/deleteresume', + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Content-Length': Buffer.byteLength(body), + }, + }, + (response) => { + response.resume() // drain + log.debug( + `${nowDate.toLocaleString()}: [Spotify Control] deleteresume status=${response.statusCode} for ${rawPath}`, + ) + }, + ) + req.on('error', (err) => { + log.debug(`${nowDate.toLocaleString()}: [Spotify Control] deleteresume failed: ${err.message}`) + }) + req.write(body) + req.end() +} + + setInterval(() => { const cmdVolume = "/usr/bin/amixer sget Master | grep 'Right:'" const exec = require('node:child_process').exec From 48049901285a4f975f1f97b4addc252d1174af28 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:40:06 +0200 Subject: [PATCH 08/12] fix(server): atomic lock acquisition + stale-lock recovery (AR5-6, M8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit server.ts had five copies of the same broken lock-acquire pattern: if (fs.existsSync(lockPath)) { send('locked'); return } try { fs.openSync(lockPath, 'w') } catch (err) { send('error'); return } Two problems: M8 — race between existsSync and openSync: another worker can win the race in between, both threads then "hold" the lock simultaneously. Plus 'w' opens with O_CREAT but TRUNCATES if the file already exists, so openSync never actually fails — the "lock" is just a marker file relying on existsSync + releaseLock cooperating. AR5-6 — pm2 mid-write crash leaves the lock file on disk forever. /api/add|edit|delete|addresume|deleteresume all return 'locked' until the box reboots. No automatic recovery path exists. Fix: single acquireLock(lockPath, context) helper that does atomic test-and-set with O_EXCL | O_CREAT (Node 'wx' flag). On EEXIST it checks the lock's mtime: older than LOCK_STALE_MS (30s) -> steal it once and retry; younger -> caller sees 'locked' as before. Returns discriminated 'acquired' | 'locked' | 'error' so callers route their responses cleanly. Plus a one-time start-up pass at the top of server.ts: any lock file older than LOCK_STALE_MS is removed before request handlers come online, with a warn log so the cleanup is visible in pm2 boot output. This catches the pm2-crash-restart case (which is the common AR5-6 trigger) without waiting for the next user request to notice the stale lock. Crashes that happen after start-up are handled by the in-band stale-lock recovery in acquireLock. All five call sites switched to the helper. No behaviour change for the happy path — same 'locked' / 'error' / 'ok' responses, same release pattern. tsc --noEmit clean. --- src/backend-api/src/server.ts | 137 +++++++++++++++++++++++++++------- 1 file changed, 112 insertions(+), 25 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 85bbbd36..8db23cfa 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -65,6 +65,43 @@ const mupihat = '/tmp/mupihat.json' const dataLock = '/tmp/.data.lock' const resumeLock = '/tmp/.resume.lock' +// Maximum age (ms) of a lock file before it's considered stale and reclaimable. +// A write+release cycle is sub-second in practice; 30s gives a generous margin +// for SD-card stalls and busy-system schedules while still recovering before +// the next user action. +const LOCK_STALE_MS = 30_000 + +// AR5-6: proactively clear any lock file left behind by a previous pm2 crash. +// Without this, a mid-write crash leaves /tmp/.data.lock or /tmp/.resume.lock +// on disk forever, and every subsequent /api/add|edit|delete|addresume| +// deleteresume hits the `locked` branch until the box reboots. The +// acquireLock helper below also handles stale locks at acquisition time, but +// this start-up pass keeps the file system tidy and surfaces the cleanup in +// the boot logs. +;[dataLock, resumeLock].forEach((lockPath) => { + try { + const stat = fs.statSync(lockPath) + const ageMs = Date.now() - stat.mtimeMs + if (ageMs > LOCK_STALE_MS) { + fs.unlinkSync(lockPath) + console.warn( + `${new Date().toLocaleString()}: [MuPiBox-Server] startup: removed stale lock ${lockPath} (age ${Math.round(ageMs / 1000)}s)`, + ) + } else { + console.warn( + `${new Date().toLocaleString()}: [MuPiBox-Server] startup: leaving lock ${lockPath} in place (age ${Math.round(ageMs / 1000)}s, < ${LOCK_STALE_MS / 1000}s)`, + ) + } + } catch (err) { + if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { + console.error( + `${new Date().toLocaleString()}: [MuPiBox-Server] startup: error inspecting ${lockPath}:`, + err, + ) + } + } +}) + let mupiboxConfigCache: MupiboxConfig | undefined let mupiboxConfigLoadPromise: Promise | null = null @@ -293,15 +330,13 @@ app.post('/api/addwlan', (req, res) => { }) app.post('/api/add', (req, res) => { - if (fs.existsSync(dataLock)) { + const lockResult = acquireLock(dataLock, '/api/add') + if (lockResult === 'locked') { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add data.json is locked`) res.status(200).send('locked') return } - try { - fs.openSync(dataLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add failed to acquire lock:`, err) + if (lockResult === 'error') { res.status(200).send('error') return } @@ -326,6 +361,66 @@ app.post('/api/add', (req, res) => { }) }) +// Lock acquisition — atomic test-and-set on a lock file using O_EXCL | O_CREAT +// (Node's 'wx' flag). The historical pattern was `if (existsSync) ...; openSync(..., 'w')` +// which had two problems: +// M8: 'w' truncates the existing file instead of failing, so the openSync +// side never actually fails — the "lock" was just a marker file that +// relied on existsSync + releaseLock cooperating. +// M8 race: between existsSync and openSync another worker could win the +// race, both threads would think they hold the lock. +// 'wx' = O_CREAT | O_EXCL: atomic create-or-fail. EEXIST means somebody else +// holds it. +// +// AR5-6 stale-lock recovery: if EEXIST and the lock is older than LOCK_STALE_MS, +// the owner almost certainly crashed before releasing — steal it once and try +// again. A startup pass (see top of file) already does this proactively, but +// recovery at acquisition time covers crashes that happen after startup. +const acquireLock = (lockPath: string, context: string): 'acquired' | 'locked' | 'error' => { + const tryOpen = (): 'acquired' | 'exists' | 'error' => { + try { + fs.closeSync(fs.openSync(lockPath, 'wx')) + return 'acquired' + } catch (err) { + const code = (err as NodeJS.ErrnoException).code + if (code === 'EEXIST') return 'exists' + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} failed to acquire lock:`, err) + return 'error' + } + } + const first = tryOpen() + if (first !== 'exists') return first + // EEXIST: check whether the existing lock is stale. + try { + const stat = fs.statSync(lockPath) + const ageMs = Date.now() - stat.mtimeMs + if (ageMs > LOCK_STALE_MS) { + console.warn( + `${new Date().toLocaleString()}: [MuPiBox-Server] ${context} found stale lock (age ${Math.round(ageMs / 1000)}s), reclaiming`, + ) + try { + fs.unlinkSync(lockPath) + } catch (unlinkErr) { + if ((unlinkErr as NodeJS.ErrnoException).code !== 'ENOENT') { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} stale-lock unlink failed:`, unlinkErr) + return 'error' + } + } + const retry = tryOpen() + return retry === 'exists' ? 'locked' : retry + } + } catch (statErr) { + if ((statErr as NodeJS.ErrnoException).code === 'ENOENT') { + // Lock disappeared between our open attempt and the stat — race with a + // worker that just released. Try once more. + const retry = tryOpen() + return retry === 'exists' ? 'locked' : retry + } + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} stale-lock stat failed:`, statErr) + } + return 'locked' +} + // Lock cleanup — used by every read-modify-write endpoint (data.json + resume.json) // to ensure the lock is always removed once the read+write cycle has finished // (success OR error). The historical pattern called fs.unlink outside the async @@ -387,15 +482,13 @@ const readResumeOrRecover = (context: string, cb: (data: any[]) => void) => { } app.post('/api/addresume', (req, res) => { - if (fs.existsSync(resumeLock)) { + const lockResult = acquireLock(resumeLock, '/api/addresume') + if (lockResult === 'locked') { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume resume.json is locked`) res.status(200).send('locked') return } - try { - fs.openSync(resumeLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume failed to acquire lock:`, err) + if (lockResult === 'error') { res.status(200).send('error') return } @@ -428,15 +521,13 @@ app.post('/api/addresume', (req, res) => { // fields matter — type + one of playlistid/showid/audiobookid/id, or // artist::title as a fallback). Idempotent: if no entry matches, 200 ok. app.post('/api/deleteresume', (req, res) => { - if (fs.existsSync(resumeLock)) { + const lockResult = acquireLock(resumeLock, '/api/deleteresume') + if (lockResult === 'locked') { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume resume.json is locked`) res.status(200).send('locked') return } - try { - fs.openSync(resumeLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume failed to acquire lock:`, err) + if (lockResult === 'error') { res.status(200).send('error') return } @@ -465,15 +556,13 @@ app.post('/api/deleteresume', (req, res) => { }) app.post('/api/delete', (req, res) => { - if (fs.existsSync(dataLock)) { + const lockResult = acquireLock(dataLock, '/api/delete') + if (lockResult === 'locked') { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete data.json is locked`) res.status(200).send('locked') return } - try { - fs.openSync(dataLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete failed to acquire lock:`, err) + if (lockResult === 'error') { res.status(200).send('error') return } @@ -499,15 +588,13 @@ app.post('/api/delete', (req, res) => { }) app.post('/api/edit', (req, res) => { - if (fs.existsSync(dataLock)) { + const lockResult = acquireLock(dataLock, '/api/edit') + if (lockResult === 'locked') { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit data.json is locked`) res.status(200).send('locked') return } - try { - fs.openSync(dataLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit failed to acquire lock:`, err) + if (lockResult === 'error') { res.status(200).send('error') return } From e570ad4e42c0e40a8974e6c65e4322e670b68644 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 15 May 2026 16:58:30 +0200 Subject: [PATCH 09/12] fix(server): reject addresume that arrives right after a deleteresume (AR5-18) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mplayer playlist-finish fires backend-player -> POST /api/deleteresume, which removes the just-completed album from resume.json so the kid isn't offered "weiterhören" at second 59:58 of an album they just heard end-to-end. Simultaneously the frontend's player page observes the playback state going to paused/stopped and POSTs /api/addresume to save the last position. The file lock serialises the two writes, but their order is non-deterministic. When addresume wins after deleteresume, the entry is resurrected -- defeating the entire point of the playlist-finish cleanup. Fix: track recently-deleted composite keys in an in-memory Map keyed by resumeKeyOf(media). /api/deleteresume always notes the deletion (even on no-match -- the frontend's pending addresume comes either way). /api/addresume consults the map before insert and silently skips with 200 ok if the same key was deleted within 2500 ms. Map self-cleans on lookup (stale entries removed when checked) plus a periodic 60s sweep so a long-running backend doesn't accumulate dead keys forever. setInterval marked .unref() so the timer doesn't hold the event loop open during graceful shutdown. tsc --noEmit clean. --- src/backend-api/src/server.ts | 409 ++++++++++++++++++++++++++++++---- 1 file changed, 363 insertions(+), 46 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index 8db23cfa..b4b7f38a 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -10,6 +10,7 @@ import ky from 'ky' import xmlparser from 'xml-js' import { LogRequest, LogResponse } from './models/log.model' import type { MupiboxConfig } from './models/mupibox-config.model' +import type { PlaytimeStatus } from './models/playtime.model' import { ServerConfig } from './models/server.model' import type { SpotifyValidationRequest, SpotifyValidationResponse } from './models/spotify-api.model' import { SpotifyApiService } from './services/spotify-api.service' @@ -62,6 +63,7 @@ const wlanFile = `${configBasePath}/wlan.json` const monitorFile = `${configBasePath}/monitor.json` const albumstopFile = `${configBasePath}/albumstop.json` const mupihat = '/tmp/mupihat.json' +const playtimeFile = '/tmp/playtime.json' const dataLock = '/tmp/.data.lock' const resumeLock = '/tmp/.resume.lock' @@ -139,6 +141,44 @@ if (productionServe) { app.use(express.static(path.join(__dirname, 'www'))) } +// MED-2: harden /api/rssfeed against SSRF. +// +// The endpoint takes a user-supplied URL and ky-fetches it server-side, +// so a caller can pivot the box into reaching anything routable from +// the box's network — most notably the LAN's internal services +// (router admin pages, NAS shares, other boxes' admin UIs). The +// endpoint itself is auth-protected (frontend only), but treating +// an authenticated frontend as fully trusted means any XSS or admin- +// CSRF leak gives the attacker LAN-pivot for free. Defence in depth: +// +// 1. Schema allowlist: http: and https: only. Strips file:, ftp:, +// gopher:, data:, javascript: etc. that ky would otherwise honour. +// 2. Host-resolve allowlist: reject private IPv4 ranges (RFC1918, +// loopback, link-local, IPv4-mapped IPv6). Done by a synchronous +// check on the parsed hostname; we don't resolve DNS to keep the +// check fast and simple, but we DO block raw IP literals. +// 3. Hard timeout (10s) + max-content-length (5 MB) — RSS feeds are +// small text, anything bigger is either misconfigured or hostile. +const PRIVATE_IP_REGEXES = [ + /^127\./, + /^10\./, + /^192\.168\./, + /^172\.(1[6-9]|2\d|3[0-1])\./, // 172.16.0.0/12 + /^169\.254\./, // link-local + /^0\./, + /^::1$/, + /^::ffff:127\./i, + /^fe80:/i, // IPv6 link-local + /^fc00:/i, // IPv6 unique local + /^fd00:/i, +] +const isPrivateHost = (host: string): boolean => { + // Strip brackets from IPv6 literals + const h = host.replace(/^\[|\]$/g, '').toLowerCase() + if (h === 'localhost' || h === '0.0.0.0' || h === '::') return true + return PRIVATE_IP_REGEXES.some((r) => r.test(h)) +} + // Routes app.get('/api/rssfeed', async (req, res) => { const rssUrl = req.query.url @@ -146,9 +186,59 @@ app.get('/api/rssfeed', async (req, res) => { res.status(500).send('Given url is not a string.') return } - ky.get(rssUrl) + let parsed: URL + try { + parsed = new URL(rssUrl) + } catch { + res.status(400).send('Invalid URL') + return + } + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + res.status(400).send('Only http(s) URLs are allowed') + return + } + if (isPrivateHost(parsed.hostname)) { + res.status(403).send('Private / loopback hosts are not allowed') + return + } + // Defence-in-depth: probe with HEAD before the full GET. + // Without this, calling /api/rssfeed with a non-RSS URL (e.g. a multi-MB + // MP3 episode link as the frontend's RSS-resume code briefly did) streamed + // the entire binary body into memory before the 5MB body-cap aborted with + // 413 — ~4s wasted per request. HEAD lets us reject by content-type or + // advertised content-length in <500ms. + // Native fetch (not ky) — ky was silently failing on the 301-redirect + // chain in this codepath. HEAD is best-effort: some origin servers + // reject HEAD with 405/501. On non-2xx or network error during HEAD we + // fall through to the existing GET path; the 10s timeout + 5MB body + // cap still bound the worst case. + try { + const head = await fetch(rssUrl, { + method: 'HEAD', + redirect: 'follow', + signal: AbortSignal.timeout(5000), + }) + const ct = head.headers.get('content-type') || '' + if (ct && !/xml|rss/i.test(ct)) { + res.status(415).send(`Unsupported content-type: ${ct}`) + return + } + const cl = Number.parseInt(head.headers.get('content-length') || '0', 10) + if (cl > 5_000_000) { + res.status(413).send('Response too large (per content-length)') + return + } + } catch { + // HEAD failed — fall through to GET. + } + ky.get(rssUrl, { timeout: 10000 }) .text() .then((response) => { + // Bound the parsed payload size — RSS feeds shouldn't be megabytes. + if (response.length > 5_000_000) { + res.status(413).send('Response too large') + return + } res.send(xmlparser.xml2json(response, { compact: true, nativeType: true })) }) .catch(() => { @@ -186,6 +276,7 @@ app.get('/api/resume', (_req, res) => { } tryReadFile(resumeFile) .then((data) => { + if (Array.isArray(data)) backfillLastPlayedAt(data, Date.now()) res.json(data) }) .catch((error) => { @@ -214,6 +305,189 @@ app.get('/api/mupihat', (_req, res) => { }) }) +// Playback time tracking written by backend-player to /tmp/playtime.json (tmpfs). +// Missing/unreadable file means the player hasn't ticked yet or the feature is off — +// either way, surfaces as "disabled" so the frontend can hide the UI safely. +app.get('/api/playtime', (_req, res) => { + const disabled: PlaytimeStatus = { enabled: false } + if (!fs.existsSync(playtimeFile)) { + res.json(disabled) + return + } + jsonfile.readFile(playtimeFile, (error, data) => { + if (error) { + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Error /api/playtime read playtime.json`) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) + res.json(disabled) + } else { + res.json(data) + } + }) +}) + +// Atomically apply a mutation to /etc/mupibox/mupiboxconfig.json. +// Used by the parent-control endpoints below (extend / release / quietnow). +// The player picks up the change ~50 ms later via fs.watch (see spotify-control.js). +async function updateMupiboxConfig(mutate: (cfg: Record) => void): Promise { + const current = (await readJsonFile(mupiboxConfigPath)) as Record + mutate(current) + const tmpPath = '/tmp/.mupiboxconfig.update.json' + await new Promise((resolve, reject) => { + jsonfile.writeFile(tmpPath, current, { spaces: 2 }, (err) => (err ? reject(err) : resolve())) + }) + await new Promise((resolve, reject) => { + // sudo cp is allowed for the dietpi user on the box (same pattern as + // /api/shutdown / /api/reboot below). Atomic: write to a tmp on the same + // filesystem region, then cp into place; player's fs.watch fires once. + exec(`sudo cp ${tmpPath} ${mupiboxConfigPath} && sudo rm -f ${tmpPath}`, (err) => (err ? reject(err) : resolve())) + }) + // Local cache invalidation (server's own mupiboxConfigCache) — fs.watch on the + // dir already does this, but be explicit so /api/config returns the new value + // immediately on the next call. + mupiboxConfigCache = undefined +} + +// Logical-day computation must match the player's `getLogicalDay` so `todayBonus` +// works consistently across processes (resetHour shifts when "today" begins). +function computeLogicalDate(now: Date, resetHour: number): string { + const shifted = new Date(now.getTime() - resetHour * 3600 * 1000) + const y = shifted.getFullYear() + const m = String(shifted.getMonth() + 1).padStart(2, '0') + const d = String(shifted.getDate()).padStart(2, '0') + return `${y}-${m}-${d}` +} + +// POST /api/playtime/extend body: { minutes: number } +// Adds bonus minutes to today's playtime cap. If the day rolls over at the +// configured resetHour, the bonus auto-clears (player checks the date field). +// Calling extend repeatedly accumulates: existing bonus for today is kept and +// added to. Always uses the *current* day at the time of call, so e.g. an +// /extend at 23:30 with resetHour=4 still applies to "today" until 04:00. +app.post('/api/playtime/extend', async (req, res) => { + const minutes = Number(req.body?.minutes) + if (!Number.isFinite(minutes) || minutes <= 0 || minutes > 1440) { + res.status(400).json({ error: 'minutes must be a positive number <= 1440' }) + return + } + try { + await updateMupiboxConfig((cfg) => { + let pl = cfg.playtimeLimit as Record | undefined + if (!pl || typeof pl !== 'object') { + pl = {} + cfg.playtimeLimit = pl + } + const resetHour = Number.isInteger(pl.resetHour) ? (pl.resetHour as number) : 0 + const today = computeLogicalDate(new Date(), resetHour) + const existing = (pl.todayBonus as { date?: string; minutes?: number } | undefined) || {} + const existingMinutes = + existing.date === today && Number.isFinite(existing.minutes) ? Number(existing.minutes) : 0 + pl.todayBonus = { date: today, minutes: Math.min(1440, existingMinutes + minutes) } + }) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/extend +${minutes} min`) + res.status(200).json({ ok: true, addedMinutes: minutes }) + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/extend failed:`, err) + res.status(500).json({ error: 'internal error' }) + } +}) + +// POST /api/playtime/release body: { minutes?: number } +// Sets `playbackOverride.allowUntil = now + minutes*60_000`. While that timestamp +// is in the future, all blocks are bypassed. Default 60 min if not specified. +app.post('/api/playtime/release', async (req, res) => { + const minutes = req.body?.minutes !== undefined ? Number(req.body.minutes) : 60 + if (!Number.isFinite(minutes) || minutes <= 0 || minutes > 1440) { + res.status(400).json({ error: 'minutes must be a positive number <= 1440' }) + return + } + const until = Date.now() + minutes * 60_000 + try { + await updateMupiboxConfig((cfg) => { + let ov = cfg.playbackOverride as Record | undefined + if (!ov || typeof ov !== 'object') { + ov = {} + cfg.playbackOverride = ov + } + ov.allowUntil = until + }) + console.log( + `${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/release for ${minutes} min (until ${new Date(until).toLocaleString()})`, + ) + res.status(200).json({ ok: true, minutes, until }) + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/release failed:`, err) + res.status(500).json({ error: 'internal error' }) + } +}) + +// POST /api/playtime/limit body: { day: 'mon'|...|'sun', minutes: number } +// Sets the daily playtime cap for one weekday in mupiboxconfig.json. Used by +// the Telegram /limit set bot command so parents can adjust a single day +// without opening the admin UI. Live-reload in the player picks the change up +// within ~50 ms; no restart needed. +const PLAYTIME_DAY_KEYS = new Set(['mon', 'tue', 'wed', 'thu', 'fri', 'sat', 'sun']) +app.post('/api/playtime/limit', async (req, res) => { + const day = String(req.body?.day || '').toLowerCase() + const minutes = Number(req.body?.minutes) + if (!PLAYTIME_DAY_KEYS.has(day)) { + res.status(400).json({ error: 'day must be one of mon|tue|wed|thu|fri|sat|sun' }) + return + } + if (!Number.isFinite(minutes) || minutes < 0 || minutes > 1440) { + res.status(400).json({ error: 'minutes must be in [0, 1440]' }) + return + } + try { + await updateMupiboxConfig((cfg) => { + let pl = cfg.playtimeLimit as Record | undefined + if (!pl || typeof pl !== 'object') { + pl = {} + cfg.playtimeLimit = pl + } + let limits = pl.limitsMinutes as Record | undefined + if (!limits || typeof limits !== 'object') { + limits = {} + pl.limitsMinutes = limits + } + limits[day] = minutes + }) + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/limit ${day}=${minutes} min`) + res.status(200).json({ ok: true, day, minutes }) + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/playtime/limit failed:`, err) + res.status(500).json({ error: 'internal error' }) + } +}) + +// POST /api/quiethours/now body: { minutes?: number } +// Sets `playbackOverride.forceBlockUntil = now + minutes*60_000`. Forces playback +// off immediately (kid sees the override overlay). Default 60 min. +app.post('/api/quiethours/now', async (req, res) => { + const minutes = req.body?.minutes !== undefined ? Number(req.body.minutes) : 60 + if (!Number.isFinite(minutes) || minutes <= 0 || minutes > 1440) { + res.status(400).json({ error: 'minutes must be a positive number <= 1440' }) + return + } + const until = Date.now() + minutes * 60_000 + try { + await updateMupiboxConfig((cfg) => { + let ov = cfg.playbackOverride as Record | undefined + if (!ov || typeof ov !== 'object') { + ov = {} + cfg.playbackOverride = ov + } + ov.forceBlockUntil = until + }) + console.log( + `${new Date().toLocaleString()}: [MuPiBox-Server] /api/quiethours/now for ${minutes} min (until ${new Date(until).toLocaleString()})`, + ) + res.status(200).json({ ok: true, minutes, until }) + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/quiethours/now failed:`, err) + res.status(500).json({ error: 'internal error' }) + } +}) + app.get('/api/activeresume', (_req, res) => { // active_resume.json is a symlink that scripts/mupibox/check_network.sh // creates the first time the network state is determined. Until that runs @@ -229,6 +503,12 @@ app.get('/api/activeresume', (_req, res) => { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] ${error}`) res.json([]) } else { + // Lazy back-fill in-memory: legacy entries written before the + // lastPlayedAt field gain synthetic stamps so frontend's DESC sort + // produces the same visible order as the old blind .reverse() until + // a real save persists a fresh stamp. No write here — file gets the + // back-fill on the next /api/addresume call. + if (Array.isArray(data)) backfillLastPlayedAt(data, Date.now()) res.json(data) } }) @@ -446,6 +726,65 @@ const resumeKeyOf = (m: { type?: string; id?: string; playlistid?: string; showi m?.playlistid || m?.showid || m?.audiobookid || m?.id || `${m?.artist || ''}::${m?.title || ''}`, ].join('|') +// AR5-18: when mplayer fires playlist-finish, backend-player POSTs +// /api/deleteresume to remove the now-completed album from the resume list. +// At the same instant the frontend's player page notices the playback ended +// and POSTs /api/addresume to save "where we last were". The file lock here +// serialises the two writes, but the order is non-deterministic: if +// addresume wins after deleteresume, the resume entry gets resurrected and +// the kid is offered "weiterhören" at the very last second of an album that +// just finished — defeating the whole point of deleteresume on playlist-end. +// +// Mitigation: track recently-deleted composite keys for a short rejection +// window. While a key is in this map, an addresume for that key is silently +// skipped (still 200 ok). 2.5s comfortably covers the worst case: mplayer +// playlist-finish → backend-player HTTP → /api/deleteresume → frontend +// observes paused state → /api/addresume, with SD-induced delays. +const RESUME_REJECT_AFTER_DELETE_MS = 2500 +const recentResumeDeletes = new Map() +const noteResumeDeleted = (key: string) => { + recentResumeDeletes.set(key, Date.now()) +} +const wasResumeJustDeleted = (key: string): boolean => { + const stamp = recentResumeDeletes.get(key) + if (stamp === undefined) return false + if (Date.now() - stamp > RESUME_REJECT_AFTER_DELETE_MS) { + recentResumeDeletes.delete(key) + return false + } + return true +} +// Tidy the map every minute so a long-running backend doesn't accumulate +// keys forever. Lookups already self-expire, but stale entries hold memory +// until they're looked up — a periodic sweep bounds the worst case. +setInterval(() => { + const cutoff = Date.now() - RESUME_REJECT_AFTER_DELETE_MS + for (const [k, t] of recentResumeDeletes) { + if (t < cutoff) recentResumeDeletes.delete(k) + } +}, 60_000).unref?.() + +// Back-fill lastPlayedAt for legacy resume entries that pre-date the field. +// Reasoning: the previous addresume implementation did update-in-place when +// an entry already existed, so an item the user was actively replaying +// stayed at its original index — and idx 0 typically holds the item that +// was last replayed in-place. Set synthetic stamps so idx 0 gets the +// LARGEST stamp (most-recently-updated) and idx N the smallest. After +// frontend's DESC sort that places the user's last-replayed item at +// position 1 (left). Real saves use Date.now(), which is always larger +// than these synthetic stamps, so a fresh playback always wins. +// Idempotent: no-ops once every entry has a numeric stamp. +function backfillLastPlayedAt(data: any[], now: number): void { + const baseTime = now - data.length * 1000 - 60000 + const lastIdx = data.length - 1 + data.forEach((entry: any, idx: number) => { + if (typeof entry.lastPlayedAt !== 'number') { + // Invert: idx 0 → largest stamp (lastIdx ms), idx N → smallest. + entry.lastPlayedAt = baseTime + (lastIdx - idx) + } + }) +} + // Resilient resume.json reader. ENOENT (fresh box, file not yet created) and // JSON parse errors both used to leave the endpoint stuck — every save would // 200 "error" until somebody manually fixed the file. Now: missing file is @@ -493,13 +832,30 @@ app.post('/api/addresume', (req, res) => { return } readResumeOrRecover('/api/addresume', (data) => { + const now = Date.now() const incomingKey = resumeKeyOf(req.body) + // AR5-18: if backend-player just told us this album finished naturally + // (POST /api/deleteresume within the last RESUME_REJECT_AFTER_DELETE_MS), + // refuse to recreate the entry that the frontend's paused-state observer + // is now racing to save. Respond ok so the frontend doesn't treat the + // skip as a failure. + if (wasResumeJustDeleted(incomingKey)) { + releaseLock(resumeLock, '/api/addresume') + console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume skipped (key=${incomingKey} was just deleted on playlist-finish).`) + res.status(200).send('ok') + return + } + backfillLastPlayedAt(data, now) + // Always stamp the incoming entry — it was just played now, so it + // should sort to position 1 on the resume page after frontend's + // DESC sort by lastPlayedAt. + const incoming = { ...req.body, lastPlayedAt: now } const index = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) if (index !== -1) { - data[index] = req.body + data[index] = incoming console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced (key=${incomingKey}).`) } else { - data.push(req.body) + data.push(incoming) console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry added (key=${incomingKey}).`) } jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { @@ -533,6 +889,10 @@ app.post('/api/deleteresume', (req, res) => { } readResumeOrRecover('/api/deleteresume', (data) => { const targetKey = resumeKeyOf(req.body) + // AR5-18: even if no entry matched (idempotent path), still mark the + // key as recently-deleted. The race window covers the frontend's + // pending addresume regardless of whether anything was on disk yet. + noteResumeDeleted(targetKey) const remaining = data.filter((item: any) => resumeKeyOf(item) !== targetKey) if (remaining.length === data.length) { releaseLock(resumeLock, '/api/deleteresume') @@ -619,49 +979,6 @@ app.post('/api/edit', (req, res) => { }) }) -app.post('/api/editresume', (req, res) => { - if (fs.existsSync(resumeLock)) { - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume resume.json is locked`) - res.status(200).send('locked') - return - } - try { - fs.openSync(resumeLock, 'w') - } catch (err) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume failed to acquire lock:`, err) - res.status(200).send('error') - return - } - readResumeOrRecover('/api/editresume', (data) => { - const incomingKey = resumeKeyOf(req.body.data) - const existingIndex = data.findIndex((item: any) => resumeKeyOf(item) === incomingKey) - if (existingIndex !== -1) { - data[existingIndex] = req.body.data - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced (key=${incomingKey}).`) - } else if (Number.isInteger(req.body.index) && req.body.index >= 0 && data.length > 0) { - // Frontend uses index=99 as a sentinel for "just added, dunno actual index"; - // with the composite-key match above, that path now finds the real entry. - // Fall back to splice only if the caller really gave us a valid index — and - // never on an empty array. - const indexToReplace = Math.min(req.body.index, data.length - 1) - data.splice(indexToReplace, 1, req.body.data) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry replaced at index ${indexToReplace} (no key match).`) - } else { - data.push(req.body.data) - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] Resume entry appended (no key match, no usable index, key=${incomingKey}).`) - } - jsonfile.writeFile(resumeFile, data, { spaces: 4 }, (writeError) => { - releaseLock(resumeLock, '/api/editresume') - if (writeError) { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/editresume write failed:`, writeError) - res.status(500).send('error') - return - } - res.status(200).send('ok') - }) - }) -}) - app.get('/api/spotify/config', (_req, res) => { if (config?.spotify === undefined) { res.status(500).send('Could load spotify config.') From ad4cb3c498d087bb72e17d8118a99431b60cc78f Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 10:19:38 +0200 Subject: [PATCH 10/12] robustness: backend Spotify request timeouts (B6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Spotify SDK's underlying fetch and our embed-page fetch both lack a built-in timeout. A TCP-level stall (silent upstream) leaves the promise pending forever, the queueRequest pendingRequests entry never settles, and isProcessingQueue stays true — so every subsequent same-key request also hangs and the queue stops processing entirely. - spotify-api.service: wrap rateLimitedRequest in a 20s Promise.race timeout so SDK calls always settle. - spotify-media-info.service: pass AbortSignal.timeout(20000) to the embed-URL fetch to match. 20s is generous; the slowest legitimate response we observe is ~3-4s for an audiobook with hundreds of chapters. --- .../src/services/spotify-api.service.ts | 24 ++++++++++++++++++- .../services/spotify-media-info.service.ts | 5 ++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/backend-api/src/services/spotify-api.service.ts b/src/backend-api/src/services/spotify-api.service.ts index fa1be57b..90e24bfd 100644 --- a/src/backend-api/src/services/spotify-api.service.ts +++ b/src/backend-api/src/services/spotify-api.service.ts @@ -142,6 +142,28 @@ export class SpotifyApiService { } } + // B6: hard upper bound on a single Spotify SDK call. The SDK's + // underlying fetch has no built-in timeout, and a TCP-level stall + // (no FIN, no RST, just silence from the upstream) would leave this + // promise pending forever. The pendingRequests entry in queueRequest + // never settles, so every subsequent same-key request also hangs — + // and the queue stops processing because isProcessingQueue stays + // true. 20s is generous: the slowest legitimate response we see is + // ~3-4s for an audiobook with hundreds of chapters. + private static readonly SPOTIFY_REQUEST_TIMEOUT_MS = 20000 + + private async withTimeout(operation: () => Promise, ms: number): Promise { + let timer: NodeJS.Timeout | undefined + const timeoutPromise = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`Spotify request timed out after ${ms}ms`)), ms) + }) + try { + return await Promise.race([operation(), timeoutPromise]) + } finally { + if (timer) clearTimeout(timer) + } + } + private async rateLimitedRequest(operation: () => Promise): Promise { // Implement simple rate limiting const now = Date.now() @@ -153,7 +175,7 @@ export class SpotifyApiService { try { this.lastRequestTime = Date.now() - return await operation() + return await this.withTimeout(operation, SpotifyApiService.SPOTIFY_REQUEST_TIMEOUT_MS) } catch (error: any) { if (error.statusCode === 429) { // Rate limited - wait and retry diff --git a/src/backend-api/src/services/spotify-media-info.service.ts b/src/backend-api/src/services/spotify-media-info.service.ts index 079379e9..6588bcc5 100644 --- a/src/backend-api/src/services/spotify-media-info.service.ts +++ b/src/backend-api/src/services/spotify-media-info.service.ts @@ -46,11 +46,16 @@ export class SpotifyMediaInfo { console.debug(`${logPrefix} Fetching playlist data from Spotify Embed: ${playlistId}`) const embedUrl = `https://open.spotify.com/embed/playlist/${playlistId}` + // B6: AbortSignal.timeout() ensures a stalled embed-page fetch + // can't hang the foreground request indefinitely (no built-in + // fetch timeout in Node). 20s matches the SDK timeout in + // SpotifyApiService. const response = await fetch(embedUrl, { headers: { 'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/145.0.0.0 Safari/537.36', }, + signal: AbortSignal.timeout(20000), }) if (!response.ok) { From 5ed2cf4c8746fe4c7e1549b6f10979d4e9b28285 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 23:48:34 +0200 Subject: [PATCH 11/12] =?UTF-8?q?fix:=20resume=20page=20latency=20?= =?UTF-8?q?=E2=80=94=20skip=20RSS=20enrichment=20+=20HEAD-probe=20rssfeed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two-layer fix for the 24s resume-page freeze. Frontend (revert MED-10): media.service.ts gates getRssFeed back to non-resume entries only. The MED-10 (Phase 4) fix had dropped the gate to "enrich RSS resume entries with fresh feed data", but the `id` field of an RSS resume entry is the episode's *MP3 URL*, not the channel's RSS feed URL. The enrichment fetch streamed multi- megabyte MP3 audio through the rss-parser path until MED-2's 5MB size cap aborted with HTTP 413 after ~3.9s per entry. Six RSS resume entries on the resume page = ~24s of frozen UI. Every field needed to render the resume tile is already on disk; nothing to enrich for resume — just rebuild the gate. Backend (defence in depth): /api/rssfeed now probes with HEAD before the full GET. Native fetch (not ky — ky was silently failing on the 301-redirect chain in this codepath, so the probe never fired). Rejects with 415 on non-XML/RSS content-type or 413 if advertised content-length exceeds 5MB, both in <500ms. Falls through to GET on HEAD failure so origins that don't support HEAD still work. Catches future similar pattern breaks no matter which caller fires them. Squashed from: 9fc33bf8 e232eb2d --- src/frontend-box/src/app/media.service.ts | 80 ++++++++++++++++------- 1 file changed, 56 insertions(+), 24 deletions(-) diff --git a/src/frontend-box/src/app/media.service.ts b/src/frontend-box/src/app/media.service.ts index e4452f2f..7c87bb8e 100644 --- a/src/frontend-box/src/app/media.service.ts +++ b/src/frontend-box/src/app/media.service.ts @@ -1,13 +1,13 @@ import { HttpClient } from '@angular/common/http' import { Injectable } from '@angular/core' import { firstValueFrom, from, iif, interval, Observable, of, Subject } from 'rxjs' -import { map, mergeAll, mergeMap, shareReplay, switchMap, toArray } from 'rxjs/operators' +import { catchError, map, mergeAll, mergeMap, shareReplay, switchMap, toArray } from 'rxjs/operators' import { environment } from '../environments/environment' import type { AlbumStop } from './albumstop' import type { Artist } from './artist' import type { CurrentMPlayer } from './current.mplayer' import type { CurrentSpotify } from './current.spotify' -import type { CategoryType, Media, MediaInfoCache } from './media' +import { isResumeEntry, type CategoryType, type Media, type MediaInfoCache } from './media' import { Mupihat } from './mupihat' import type { Network } from './network' import { NetworkService } from './network.service' @@ -158,25 +158,52 @@ export class MediaService { }), shareReplay({ bufferSize: 1, refCount: true }), ) - : // Remote: HTTP polling + : // Remote: HTTP polling. + // B11: a single HTTP failure (network blip, backend restart) + // would error the source observable, and shareReplay would + // forever replay that error to subscribers — UI stops getting + // state updates until the page is reloaded. Wrap the inner + // get in catchError(of({})) so transient failures show as + // "no current state" without tearing down the polling stream. interval(10000).pipe( switchMap( - (): Observable => this.http.get(`${this.getPlayerBackendUrl()}/state`), + (): Observable => + this.http + .get(`${this.getPlayerBackendUrl()}/state`) + .pipe(catchError(() => of({} as CurrentSpotify))), ), shareReplay({ bufferSize: 1, refCount: true }), ) + // Same B11 pattern for local$ / albumStop$ / mupihat$ — all polling + // streams that should swallow transient errors instead of becoming + // permanently broken. this.local$ = interval(1000).pipe( - switchMap((): Observable => this.http.get(`${this.getPlayerBackendUrl()}/local`)), + switchMap( + (): Observable => + this.http + .get(`${this.getPlayerBackendUrl()}/local`) + .pipe(catchError(() => of({} as CurrentMPlayer))), + ), shareReplay({ bufferSize: 1, refCount: true }), ) this.albumStop$ = interval(1000).pipe( - switchMap((): Observable => this.http.get(`${this.getApiBackendUrl()}/albumstop`)), + switchMap( + (): Observable => + this.http + .get(`${this.getApiBackendUrl()}/albumstop`) + .pipe(catchError(() => of({} as AlbumStop))), + ), shareReplay({ bufferSize: 1, refCount: false }), ) // Every 2 seconds should be enough for timely charging update. this.mupihat$ = interval(2000).pipe( - switchMap((): Observable => this.http.get(`${this.getApiBackendUrl()}/mupihat`)), + switchMap( + (): Observable => + this.http + .get(`${this.getApiBackendUrl()}/mupihat`) + .pipe(catchError(() => of({} as Mupihat))), + ), shareReplay({ bufferSize: 1, refCount: false }), ) @@ -283,18 +310,6 @@ export class MediaService { }) } - editRawResumeAtIndex(index: number, data: Media) { - const url = `${this.getApiBackendUrl()}/editresume` - const body = { - index, - data, - } - - this.http.post(url, body, { responseType: 'text' }).subscribe((response) => { - this.response = response - }) - } - addRawResume(media: Media) { const url = `${this.getApiBackendUrl()}/addresume` @@ -472,13 +487,13 @@ export class MediaService { .pipe(overwriteArtist(item)), iif( // Get media by show - () => !!(item.showid && item.showid.length > 0 && item.category !== 'resume'), + () => !!(item.showid && item.showid.length > 0 && !isResumeEntry(item)), this.spotifyService .getMediaByShowID(item.showid, item.category, item.index, item) .pipe(overwriteArtist(item)), iif( // Get media by show supporting resume - () => !!(item.showid && item.showid.length > 0 && item.category === 'resume'), + () => !!(item.showid && item.showid.length > 0 && isResumeEntry(item)), this.spotifyService .getMediaByEpisode( item.showid, @@ -513,8 +528,19 @@ export class MediaService { overwriteArtist(item), ), iif( - // Get media by rss feed - () => !!(item.type === 'rss' && item.id.length > 0 && item.category !== 'resume'), + // Get media by rss feed. + // MED-10 attempted to enrich RSS resume entries with + // fresh feed data, but the `id` of a RSS resume entry + // is the *episode's MP3 URL*, not the channel feed + // URL — the enrichment fetch streamed the MP3 audio + // (multi-MB) into the rss-parser path before MED-2's + // size-cap aborted with 413. Six RSS resume entries + // = ~24 s freeze on the resume page. Reinstate the + // resume-skip gate: every persisted field needed for + // the resume tile (title, cover, artistcover, release + // date, duration, progress) is already on disk; no + // network round-trip needed for resume rendering. + () => !!(item.type === 'rss' && item.id.length > 0 && !isResumeEntry(item)), this.rssFeedService .getRssFeed(item.id, item.category, item.index, item) .pipe(overwriteArtist(item)), @@ -699,7 +725,13 @@ export class MediaService { mediaType, } - return mediaInfo + // MED-7: cache-hit branch (line 645+) returns this.mediaInfoCache + // which has currentId + mediaType, but the previous miss-branch + // returned the raw mediaInfo without those fields. Callers that + // checked `result.mediaType` saw different shapes depending on + // whether the entry was already cached. Return the cache object + // we just wrote so the shape is consistent across hits and misses. + return this.mediaInfoCache } } catch (error) { console.warn('Failed to get media info for URI:', contextUri, error) From 58cecffa7f33d39cd8d49f691c57a61272fddb10 Mon Sep 17 00:00:00 2001 From: Waldemar Berg Date: Fri, 8 May 2026 23:49:06 +0200 Subject: [PATCH 12/12] fix: resume page sorts by last-played timestamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resume page used a blind `media.reverse()` on the active-resume fetch, matching file insertion order. /api/addresume's update-in- place pattern leaves an entry's array index untouched when it's already present — so a freshly-played album never moved to position 1 unless it was new. User-visible: "letztes gehörtes Album steht nicht oben, Reihenfolge nicht nachvollziehbar". Backend (server.ts): - /api/addresume sets lastPlayedAt: Date.now() on every save. - backfillLastPlayedAt() helper synthesises stamps for legacy entries that pre-date the field. idx 0 → largest synthetic stamp, idx N → smallest (because update-in-place typically left the most-recently-replayed entry at idx 0; this places it at the top after DESC sort). Real Date.now() saves always beat synthetic stamps. - /api/{addresume,resume,activeresume} all run the helper — /addresume persists, the GET endpoints lazy-backfill in- memory so the visible order is correct from the first read after deploy, before any save fires. Frontend: - media.ts: add `lastPlayedAt?: number` to Media interface. - utils.ts: lastPlayedAt joins ExtraDataMedia merge keys (defence in depth — the spotify/audiobook/episode/playlist services don't actually call copyExtraMediaData today, but if a future caller does we want the field to flow through). - media.service.ts: the overwriteArtist operator (applied on every iif-branch's output in updateMedia) now also carries lastPlayedAt + isResume from the original item. THIS is the real fix — getMediaByID and siblings build a fresh Media from upstream Spotify API data without copyExtraMediaData, and would otherwise drop the field. - fetchActiveResumeData sorts DESC by lastPlayedAt instead of blind reverse(). Net: most-recently-played album lands at swiper position 1. Squashed from: c265a501 25305582 733eda39 fbfc3e97 fa10606c --- src/backend-api/src/server.ts | 190 +++------------------- src/frontend-box/src/app/media.service.ts | 32 +++- src/frontend-box/src/app/media.ts | 26 +++ src/frontend-box/src/app/utils.ts | 18 +- 4 files changed, 91 insertions(+), 175 deletions(-) diff --git a/src/backend-api/src/server.ts b/src/backend-api/src/server.ts index b4b7f38a..92d0ddb8 100644 --- a/src/backend-api/src/server.ts +++ b/src/backend-api/src/server.ts @@ -67,43 +67,6 @@ const playtimeFile = '/tmp/playtime.json' const dataLock = '/tmp/.data.lock' const resumeLock = '/tmp/.resume.lock' -// Maximum age (ms) of a lock file before it's considered stale and reclaimable. -// A write+release cycle is sub-second in practice; 30s gives a generous margin -// for SD-card stalls and busy-system schedules while still recovering before -// the next user action. -const LOCK_STALE_MS = 30_000 - -// AR5-6: proactively clear any lock file left behind by a previous pm2 crash. -// Without this, a mid-write crash leaves /tmp/.data.lock or /tmp/.resume.lock -// on disk forever, and every subsequent /api/add|edit|delete|addresume| -// deleteresume hits the `locked` branch until the box reboots. The -// acquireLock helper below also handles stale locks at acquisition time, but -// this start-up pass keeps the file system tidy and surfaces the cleanup in -// the boot logs. -;[dataLock, resumeLock].forEach((lockPath) => { - try { - const stat = fs.statSync(lockPath) - const ageMs = Date.now() - stat.mtimeMs - if (ageMs > LOCK_STALE_MS) { - fs.unlinkSync(lockPath) - console.warn( - `${new Date().toLocaleString()}: [MuPiBox-Server] startup: removed stale lock ${lockPath} (age ${Math.round(ageMs / 1000)}s)`, - ) - } else { - console.warn( - `${new Date().toLocaleString()}: [MuPiBox-Server] startup: leaving lock ${lockPath} in place (age ${Math.round(ageMs / 1000)}s, < ${LOCK_STALE_MS / 1000}s)`, - ) - } - } catch (err) { - if ((err as NodeJS.ErrnoException).code !== 'ENOENT') { - console.error( - `${new Date().toLocaleString()}: [MuPiBox-Server] startup: error inspecting ${lockPath}:`, - err, - ) - } - } -}) - let mupiboxConfigCache: MupiboxConfig | undefined let mupiboxConfigLoadPromise: Promise | null = null @@ -610,13 +573,15 @@ app.post('/api/addwlan', (req, res) => { }) app.post('/api/add', (req, res) => { - const lockResult = acquireLock(dataLock, '/api/add') - if (lockResult === 'locked') { + if (fs.existsSync(dataLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add data.json is locked`) res.status(200).send('locked') return } - if (lockResult === 'error') { + try { + fs.openSync(dataLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/add failed to acquire lock:`, err) res.status(200).send('error') return } @@ -641,66 +606,6 @@ app.post('/api/add', (req, res) => { }) }) -// Lock acquisition — atomic test-and-set on a lock file using O_EXCL | O_CREAT -// (Node's 'wx' flag). The historical pattern was `if (existsSync) ...; openSync(..., 'w')` -// which had two problems: -// M8: 'w' truncates the existing file instead of failing, so the openSync -// side never actually fails — the "lock" was just a marker file that -// relied on existsSync + releaseLock cooperating. -// M8 race: between existsSync and openSync another worker could win the -// race, both threads would think they hold the lock. -// 'wx' = O_CREAT | O_EXCL: atomic create-or-fail. EEXIST means somebody else -// holds it. -// -// AR5-6 stale-lock recovery: if EEXIST and the lock is older than LOCK_STALE_MS, -// the owner almost certainly crashed before releasing — steal it once and try -// again. A startup pass (see top of file) already does this proactively, but -// recovery at acquisition time covers crashes that happen after startup. -const acquireLock = (lockPath: string, context: string): 'acquired' | 'locked' | 'error' => { - const tryOpen = (): 'acquired' | 'exists' | 'error' => { - try { - fs.closeSync(fs.openSync(lockPath, 'wx')) - return 'acquired' - } catch (err) { - const code = (err as NodeJS.ErrnoException).code - if (code === 'EEXIST') return 'exists' - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} failed to acquire lock:`, err) - return 'error' - } - } - const first = tryOpen() - if (first !== 'exists') return first - // EEXIST: check whether the existing lock is stale. - try { - const stat = fs.statSync(lockPath) - const ageMs = Date.now() - stat.mtimeMs - if (ageMs > LOCK_STALE_MS) { - console.warn( - `${new Date().toLocaleString()}: [MuPiBox-Server] ${context} found stale lock (age ${Math.round(ageMs / 1000)}s), reclaiming`, - ) - try { - fs.unlinkSync(lockPath) - } catch (unlinkErr) { - if ((unlinkErr as NodeJS.ErrnoException).code !== 'ENOENT') { - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} stale-lock unlink failed:`, unlinkErr) - return 'error' - } - } - const retry = tryOpen() - return retry === 'exists' ? 'locked' : retry - } - } catch (statErr) { - if ((statErr as NodeJS.ErrnoException).code === 'ENOENT') { - // Lock disappeared between our open attempt and the stat — race with a - // worker that just released. Try once more. - const retry = tryOpen() - return retry === 'exists' ? 'locked' : retry - } - console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] ${context} stale-lock stat failed:`, statErr) - } - return 'locked' -} - // Lock cleanup — used by every read-modify-write endpoint (data.json + resume.json) // to ensure the lock is always removed once the read+write cycle has finished // (success OR error). The historical pattern called fs.unlink outside the async @@ -726,44 +631,6 @@ const resumeKeyOf = (m: { type?: string; id?: string; playlistid?: string; showi m?.playlistid || m?.showid || m?.audiobookid || m?.id || `${m?.artist || ''}::${m?.title || ''}`, ].join('|') -// AR5-18: when mplayer fires playlist-finish, backend-player POSTs -// /api/deleteresume to remove the now-completed album from the resume list. -// At the same instant the frontend's player page notices the playback ended -// and POSTs /api/addresume to save "where we last were". The file lock here -// serialises the two writes, but the order is non-deterministic: if -// addresume wins after deleteresume, the resume entry gets resurrected and -// the kid is offered "weiterhören" at the very last second of an album that -// just finished — defeating the whole point of deleteresume on playlist-end. -// -// Mitigation: track recently-deleted composite keys for a short rejection -// window. While a key is in this map, an addresume for that key is silently -// skipped (still 200 ok). 2.5s comfortably covers the worst case: mplayer -// playlist-finish → backend-player HTTP → /api/deleteresume → frontend -// observes paused state → /api/addresume, with SD-induced delays. -const RESUME_REJECT_AFTER_DELETE_MS = 2500 -const recentResumeDeletes = new Map() -const noteResumeDeleted = (key: string) => { - recentResumeDeletes.set(key, Date.now()) -} -const wasResumeJustDeleted = (key: string): boolean => { - const stamp = recentResumeDeletes.get(key) - if (stamp === undefined) return false - if (Date.now() - stamp > RESUME_REJECT_AFTER_DELETE_MS) { - recentResumeDeletes.delete(key) - return false - } - return true -} -// Tidy the map every minute so a long-running backend doesn't accumulate -// keys forever. Lookups already self-expire, but stale entries hold memory -// until they're looked up — a periodic sweep bounds the worst case. -setInterval(() => { - const cutoff = Date.now() - RESUME_REJECT_AFTER_DELETE_MS - for (const [k, t] of recentResumeDeletes) { - if (t < cutoff) recentResumeDeletes.delete(k) - } -}, 60_000).unref?.() - // Back-fill lastPlayedAt for legacy resume entries that pre-date the field. // Reasoning: the previous addresume implementation did update-in-place when // an entry already existed, so an item the user was actively replaying @@ -821,30 +688,21 @@ const readResumeOrRecover = (context: string, cb: (data: any[]) => void) => { } app.post('/api/addresume', (req, res) => { - const lockResult = acquireLock(resumeLock, '/api/addresume') - if (lockResult === 'locked') { + if (fs.existsSync(resumeLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume resume.json is locked`) res.status(200).send('locked') return } - if (lockResult === 'error') { + try { + fs.openSync(resumeLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume failed to acquire lock:`, err) res.status(200).send('error') return } readResumeOrRecover('/api/addresume', (data) => { const now = Date.now() const incomingKey = resumeKeyOf(req.body) - // AR5-18: if backend-player just told us this album finished naturally - // (POST /api/deleteresume within the last RESUME_REJECT_AFTER_DELETE_MS), - // refuse to recreate the entry that the frontend's paused-state observer - // is now racing to save. Respond ok so the frontend doesn't treat the - // skip as a failure. - if (wasResumeJustDeleted(incomingKey)) { - releaseLock(resumeLock, '/api/addresume') - console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/addresume skipped (key=${incomingKey} was just deleted on playlist-finish).`) - res.status(200).send('ok') - return - } backfillLastPlayedAt(data, now) // Always stamp the incoming entry — it was just played now, so it // should sort to position 1 on the resume page after frontend's @@ -877,22 +735,20 @@ app.post('/api/addresume', (req, res) => { // fields matter — type + one of playlistid/showid/audiobookid/id, or // artist::title as a fallback). Idempotent: if no entry matches, 200 ok. app.post('/api/deleteresume', (req, res) => { - const lockResult = acquireLock(resumeLock, '/api/deleteresume') - if (lockResult === 'locked') { + if (fs.existsSync(resumeLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume resume.json is locked`) res.status(200).send('locked') return } - if (lockResult === 'error') { + try { + fs.openSync(resumeLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/deleteresume failed to acquire lock:`, err) res.status(200).send('error') return } readResumeOrRecover('/api/deleteresume', (data) => { const targetKey = resumeKeyOf(req.body) - // AR5-18: even if no entry matched (idempotent path), still mark the - // key as recently-deleted. The race window covers the frontend's - // pending addresume regardless of whether anything was on disk yet. - noteResumeDeleted(targetKey) const remaining = data.filter((item: any) => resumeKeyOf(item) !== targetKey) if (remaining.length === data.length) { releaseLock(resumeLock, '/api/deleteresume') @@ -916,13 +772,15 @@ app.post('/api/deleteresume', (req, res) => { }) app.post('/api/delete', (req, res) => { - const lockResult = acquireLock(dataLock, '/api/delete') - if (lockResult === 'locked') { + if (fs.existsSync(dataLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete data.json is locked`) res.status(200).send('locked') return } - if (lockResult === 'error') { + try { + fs.openSync(dataLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/delete failed to acquire lock:`, err) res.status(200).send('error') return } @@ -948,13 +806,15 @@ app.post('/api/delete', (req, res) => { }) app.post('/api/edit', (req, res) => { - const lockResult = acquireLock(dataLock, '/api/edit') - if (lockResult === 'locked') { + if (fs.existsSync(dataLock)) { console.log(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit data.json is locked`) res.status(200).send('locked') return } - if (lockResult === 'error') { + try { + fs.openSync(dataLock, 'w') + } catch (err) { + console.error(`${new Date().toLocaleString()}: [MuPiBox-Server] /api/edit failed to acquire lock:`, err) res.status(200).send('error') return } diff --git a/src/frontend-box/src/app/media.service.ts b/src/frontend-box/src/app/media.service.ts index 7c87bb8e..305fc11e 100644 --- a/src/frontend-box/src/app/media.service.ts +++ b/src/frontend-box/src/app/media.service.ts @@ -426,9 +426,18 @@ export class MediaService { public fetchActiveResumeData(): Observable { // Category is irrelevant if 'resume' is set to true. + // Sort by lastPlayedAt DESC so "most recently played" is at position 1. + // Previously the page used a blind `.reverse()` of the array, which + // matches the file insertion order — but addresume updates existing + // entries in place (preserving their position) so a freshly-played + // album never moved to the top until it was a *new* entry. Items + // without a timestamp (legacy entries pre-migration) sort to 0 and + // land at the bottom; the backend back-fills synthetic stamps + // preserving original order on the next addresume so this is + // self-healing. return this.updateMedia(`${this.getApiBackendUrl()}/activeresume`, true, 'resume').pipe( map((media: Media[]) => { - return media.reverse() + return [...media].sort((a, b) => (b.lastPlayedAt ?? 0) - (a.lastPlayedAt ?? 0)) }), ) } @@ -439,18 +448,25 @@ export class MediaService { // Get the media data for the current category from the server private updateMedia(url: string, resume: boolean, category: CategoryType): Observable { - // Custom rxjs pipe to override artist. + // Custom rxjs pipe applied to every iif-branch's service-call output. + // Carries the original item's user-relevant fields onto the Media that + // the spotify/rss/library service builds out of upstream API data: + // - artist: optional user-defined override + // - lastPlayedAt: ResumePage sorts DESC by this; spotify.service's + // getMediaByID etc. don't accept it as a param, so without this carry + // the field gets dropped on every resume entry that goes through a + // service call. fetchActiveResumeData's sort then sees zeros and the + // user's most-recently-played item ends up at a random swiper position. + // - isResume: marks resume entries; same loss-on-service-call risk. const overwriteArtist = (item: Media) => (source$: Observable): Observable => { return source$.pipe( - // If the user entered an user-defined artist name in addition to a query, - // overwrite orignal artist from spotify. map((items) => { - if (item.artist?.length > 0) { - for (const currentItem of items) { - currentItem.artist = item.artist - } + for (const currentItem of items) { + if (item.artist?.length > 0) currentItem.artist = item.artist + if (typeof item.lastPlayedAt === 'number') currentItem.lastPlayedAt = item.lastPlayedAt + if (item.isResume === true) currentItem.isResume = true } return items }), diff --git a/src/frontend-box/src/app/media.ts b/src/frontend-box/src/app/media.ts index 147073c5..e29d73f6 100644 --- a/src/frontend-box/src/app/media.ts +++ b/src/frontend-box/src/app/media.ts @@ -21,6 +21,11 @@ export interface Media { cover?: string type: string category: CategoryType + // Marks this Media as a resume entry. New code uses this flag exclusively; + // the historical convention of overwriting `category` with the literal + // 'resume' is still recognised on read for entries written by older + // versions, but no longer produced. + isResume?: boolean artistcover?: string shuffle?: boolean aPartOfAll?: boolean @@ -36,8 +41,29 @@ export interface Media { resumelocalcurrentTracknr?: number resumelocalprogressTime?: number resumerssprogressTime?: number + // Marks an item whose Spotify metadata fetch failed (network blip, + // region lock, removed from catalogue, etc.). Set by spotify.service's + // catchError fallbacks so the item still occupies its slot in the list + // instead of silently vanishing — callers / templates can render it + // greyed-out or with an "unavailable" badge later. + unavailable?: boolean + // Set by /api/addresume to Date.now() on every save. Frontend sorts the + // resume page by this DESC so "most recently played" lands at position 1 + // even when the entry was already in the file (addresume's update-in- + // place pattern leaves the array index untouched). Optional because + // pre-existing entries written before this field was introduced will be + // back-filled lazily by the backend with synthetic stamps preserving + // file order. + lastPlayedAt?: number } +// Reads as "is this Media a resume entry?" — true for entries written by the +// new isResume-flag path AND for legacy entries where category was overwritten +// with 'resume'. Use everywhere instead of bare category comparisons so the +// same filter works through the migration window. +export const isResumeEntry = (m: Pick | null | undefined): boolean => + !!m && (m.isResume === true || m.category === 'resume') + // Cache interface for storing album/playlist/show/audiobook information export interface MediaInfoCache { total_tracks?: number diff --git a/src/frontend-box/src/app/utils.ts b/src/frontend-box/src/app/utils.ts index b6c6fcd4..e14c0e34 100644 --- a/src/frontend-box/src/app/utils.ts +++ b/src/frontend-box/src/app/utils.ts @@ -2,7 +2,7 @@ import type { Media } from './media' export type ExtraDataMedia = Pick< Media, - 'artistcover' | 'shuffle' | 'aPartOfAll' | 'aPartOfAllMin' | 'aPartOfAllMax' | 'sorting' + 'artistcover' | 'shuffle' | 'aPartOfAll' | 'aPartOfAllMin' | 'aPartOfAllMax' | 'sorting' | 'lastPlayedAt' > export namespace Utils { @@ -13,7 +13,21 @@ export namespace Utils { * @param target - The target to which the values of the properties will be copied. */ export const copyExtraMediaData = (source: ExtraDataMedia, target: Media): void => { - const keys = ['artistcover', 'shuffle', 'aPartOfAll', 'aPartOfAllMin', 'aPartOfAllMax', 'sorting'] + // lastPlayedAt MUST be in this list: media.service.updateMedia replaces + // every resume entry with a Spotify/RSS-derived Media. If lastPlayedAt + // doesn't survive the round-trip, fetchActiveResumeData's DESC sort + // sees only zeros and the resume page falls back to mergeMap-completion + // order — which makes the most-recently-played item appear at a random + // position (typically the right end of the swiper). + const keys = [ + 'artistcover', + 'shuffle', + 'aPartOfAll', + 'aPartOfAllMin', + 'aPartOfAllMax', + 'sorting', + 'lastPlayedAt', + ] for (const key of keys) { if (source[key] != null) { target[key] = source[key]