From c9e1b3e0a004b6f4c655d84b3f92c10c721d4ad0 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:20:29 -0400 Subject: [PATCH] Fix: report Books, Now Playing and Backup failures in the widget's own words These three reported failures with a plain Error, which api-error.js replaces with "Something went wrong." on the way out. So "Audiobookshelf URL and API key required" and "Authentication failed, check password" never reached anyone. They use ctx.fail now, with a kind on each. The Backup helpers took a bare fetchJSON and had no way to reach ctx, so they take ctx itself. Backup swallowed two upstream failures with an empty catch. One unreachable instance still leaves its own slots null rather than failing the whole widget, but the reason is logged now instead of discarded. Navidrome no longer interpolates the upstream error text into the message it vouches for; a vouched message may carry only words this repo chose. --- CHANGELOG.md | 5 ++ api/test/backup-data.test.js | 11 ++++- api/test/widget-error.test.js | 39 +++++++++++++++- ui/widgets/backup/data.js | 87 +++++++++++++++++++---------------- ui/widgets/books/data.js | 20 ++++---- ui/widgets/nowplaying/data.js | 19 ++++---- 6 files changed, 119 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77f5512a..7198289f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- The Books, Now Playing and Backup widgets now show why they failed, such as a + missing API key or a rejected password, instead of a generic error. + ## [1.5.2] - 2026-08-09 ### Changed diff --git a/api/test/backup-data.test.js b/api/test/backup-data.test.js index 24370bd8..f1a83332 100644 --- a/api/test/backup-data.test.js +++ b/api/test/backup-data.test.js @@ -3,6 +3,8 @@ const assert = require('node:assert/strict'); const path = require('node:path'); const dataFn = require(path.join(__dirname, '..', '..', 'ui', 'widgets', 'backup', 'data.js')); +const { WidgetError, KIND } = require('../src/api-error'); +const log = require('../src/log'); /* Duplicati mints a fresh token per instance, so every fixture answers the login route as well as the data route it is asked about. */ @@ -12,6 +14,13 @@ function ctxFor(config, endpoint, routes, row = null) { calls, ctx: { config, endpoint, row, + /* The same surface widget-data.js hands a data function: a failure is + thrown as a WidgetError so its message reaches the browser intact. */ + fail: (message, opts) => { + throw new WidgetError(message, opts); + }, + KIND, + log, async fetchJSON(url, opts) { calls.push({ url, opts }); for (const [suffix, reply] of Object.entries(routes)) { @@ -44,7 +53,7 @@ test('duplicati-jobs rejects a row with no URL', async () => { test('an options fetch with no resolved row is refused', async () => { const { ctx } = ctxFor({}, 'kopia-sources', {}); - await assert.rejects(() => dataFn(ctx), /no slot selected/); + await assert.rejects(() => dataFn(ctx), /No slot selected/i); }); test('kopia-sources sends basic auth only when a username is present', async () => { diff --git a/api/test/widget-error.test.js b/api/test/widget-error.test.js index 41c85c66..54f8008d 100644 --- a/api/test/widget-error.test.js +++ b/api/test/widget-error.test.js @@ -121,11 +121,17 @@ test('no widget returns an error instead of throwing', () => { }); /* A caught exception re-reported as a returned field skipped sanitisation - entirely: the raw message went to the browser in a 200 body. There were 17. */ + entirely: the raw message went to the browser in a 200 body. There were 17. + + Log calls are stripped first: putting the caught message in the container log + is what a swallowed error should do, and it never reaches the browser. */ test('no widget forwards a raw caught message to the browser', () => { const offenders = []; for (const [name, p] of dataFiles) { - const src = fs.readFileSync(p, 'utf8').replace(/\/\*[\s\S]*?\*\//g, ''); + const src = fs + .readFileSync(p, 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/ctx\.log\.\w+\([^;]*?\);/g, ''); if (/error:\s*e\.message/.test(src)) offenders.push(name); } assert.deepEqual(offenders, [], `Raw caught message sent as data: ${offenders.join(', ')}`); @@ -153,3 +159,32 @@ test('every widget frontend shows the message the server vouched for', () => { assert.deepEqual(offenders, [], `Widget-data failure reported without the server's message. Use fetchData from widget-toolbox, or read .error off the response body:\n${offenders.join('\n')}`); }); + +/* The other half of the convention. A plain Error is sanitised to "Something + went wrong." on its way out, so `throw new Error('Komga URL and API key + required')` tells the user nothing. ctx.fail is what says the message + contains only words the author chose, and is therefore sent as written. + + Not a blanket ban on throw: a guard inside a pure helper with no ctx in scope + still throws, and the sanitiser is right to replace it. What is banned is a + sentence written for the user that never reaches them. + + connections is exempt and has to be. It reports several services in one + result, catches each service's throw itself, and writes the message into that + service's own `error` field, which is an error inside a successful response + rather than a failed one. Its sentences do reach the user, by the other route + the convention allows. */ +const THROWS_INTERNALLY = new Set(['connections']); + +test('no widget reports a user-facing failure with a plain Error', () => { + const offenders = []; + for (const [name, p] of dataFiles) { + if (THROWS_INTERNALLY.has(name)) continue; + const src = fs.readFileSync(p, 'utf8').replace(/\/\*[\s\S]*?\*\//g, ''); + for (const m of src.matchAll(/throw new Error\(([^)]*)\)/g)) { + offenders.push(`${name}: throw new Error(${m[1].slice(0, 50)})`); + } + } + assert.deepEqual(offenders, [], + `Use ctx.fail(message, { kind }) so the message reaches the browser:\n${offenders.join('\n')}`); +}); diff --git a/ui/widgets/backup/data.js b/ui/widgets/backup/data.js index 27ba3707..5960671c 100644 --- a/ui/widgets/backup/data.js +++ b/ui/widgets/backup/data.js @@ -22,44 +22,44 @@ const BACKUP_MS = 10000; /* backup providers respond more slowly than a normal d fresh login instead of reusing a token minted with the old one. */ const _dupTokens = new Map(); -async function dupLogin(base, password, fetchJSON) { - const r = await fetchJSON(base + '/api/v1/auth/login', { +async function dupLogin(base, password, ctx) { + const r = await ctx.fetchJSON(base + '/api/v1/auth/login', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ Password: password }), timeout: BACKUP_MS, }); - if (r.status !== 200) throw new Error(`Duplicati login failed: HTTP ${r.status}`); + if (r.status !== 200) ctx.fail(`Duplicati login failed: HTTP ${r.status}`, { kind: ctx.KIND.AUTH }); const { AccessToken, RefreshNonce } = r.data || {}; - if (!AccessToken) throw new Error('Duplicati login returned no token'); + if (!AccessToken) ctx.fail('Duplicati login returned no token'); return { accessToken: AccessToken, refreshNonce: RefreshNonce }; } -async function dupRefresh(base, refreshNonce, fetchJSON) { - const r = await fetchJSON(base + '/api/v1/auth/refresh', { +async function dupRefresh(base, refreshNonce, ctx) { + const r = await ctx.fetchJSON(base + '/api/v1/auth/refresh', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ RefreshNonce: refreshNonce }), timeout: BACKUP_MS, }); - if (r.status !== 200) throw new Error(`Duplicati refresh failed: HTTP ${r.status}`); + if (r.status !== 200) ctx.fail(`Duplicati refresh failed: HTTP ${r.status}`, { kind: ctx.KIND.AUTH }); const { AccessToken, RefreshNonce } = r.data || {}; - if (!AccessToken) throw new Error('Duplicati refresh returned no token'); + if (!AccessToken) ctx.fail('Duplicati refresh returned no token'); return { accessToken: AccessToken, refreshNonce: RefreshNonce }; } -async function dupGetToken(base, password, fetchJSON) { +async function dupGetToken(base, password, ctx) { const cached = _dupTokens.get(base); if (cached && cached.password === password && cached.expiresAt > Date.now() + 30000) return cached.accessToken; let tokens; if (cached && cached.password === password && cached.refreshNonce) { try { - tokens = await dupRefresh(base, cached.refreshNonce, fetchJSON); + tokens = await dupRefresh(base, cached.refreshNonce, ctx); } catch { - tokens = await dupLogin(base, password, fetchJSON); + tokens = await dupLogin(base, password, ctx); } } else { - tokens = await dupLogin(base, password, fetchJSON); + tokens = await dupLogin(base, password, ctx); } _dupTokens.set(base, { accessToken: tokens.accessToken, @@ -70,33 +70,34 @@ async function dupGetToken(base, password, fetchJSON) { return tokens.accessToken; } -async function dupFetch(base, password, path, fetchJSON) { - const token = await dupGetToken(base, password, fetchJSON); - const r = await fetchJSON(base + path, { +async function dupFetch(base, password, path, ctx) { + const token = await dupGetToken(base, password, ctx); + const r = await ctx.fetchJSON(base + path, { headers: { Authorization: `Bearer ${token}` }, timeout: BACKUP_MS, }); if (r.status !== 401) return r; _dupTokens.delete(base); - const retry = await dupGetToken(base, password, fetchJSON); - return fetchJSON(base + path, { + const retry = await dupGetToken(base, password, ctx); + return ctx.fetchJSON(base + path, { headers: { Authorization: `Bearer ${retry}` }, timeout: BACKUP_MS, }); } -function kopiaFetch(url, username, password, path, fetchJSON) { +function kopiaFetch(url, username, password, path, ctx) { const headers = {}; if (username && password) { headers['Authorization'] = 'Basic ' + Buffer.from(`${username}:${password}`).toString('base64'); } - return fetchJSON(url.replace(/\/$/, '') + path, { headers, timeout: BACKUP_MS }); + return ctx.fetchJSON(url.replace(/\/$/, '') + path, { headers, timeout: BACKUP_MS }); } -async function duplicatiJobs(row, fetchJSON) { - const base = dupNormalizeBase((row.dupUrl || '').trim()); - const r = await dupFetch(base, row.dupPass || '', '/api/v1/backups', fetchJSON); - if (r.status === 401) throw new Error('Authentication failed, check password'); +async function duplicatiJobs(row, ctx) { + const url = (row.dupUrl || '').trim(); + if (!url) ctx.fail('Duplicati URL not configured', { kind: ctx.KIND.INVALID }); + const r = await dupFetch(dupNormalizeBase(url), row.dupPass || '', '/api/v1/backups', ctx); + if (r.status === 401) ctx.fail('Authentication failed, check password', { kind: ctx.KIND.AUTH }); return { options: dupList(r.data) .map(j => ({ value: dupId(j), label: dupName(j) })) @@ -104,12 +105,12 @@ async function duplicatiJobs(row, fetchJSON) { }; } -async function kopiaSources(row, fetchJSON) { +async function kopiaSources(row, ctx) { const url = (row.kopiaUrl || '').trim(); - if (!url) throw new Error('Kopia URL required'); - const r = await kopiaFetch(url, (row.kopiaUser || '').trim(), row.kopiaPass || '', '/api/v1/sources', fetchJSON); - if (r.status === 401) throw new Error('Kopia authentication failed'); - if (r.status !== 200) throw new Error(`Kopia returned HTTP ${r.status}`); + if (!url) ctx.fail('Kopia URL required', { kind: ctx.KIND.INVALID }); + const r = await kopiaFetch(url, (row.kopiaUser || '').trim(), row.kopiaPass || '', '/api/v1/sources', ctx); + if (r.status === 401) ctx.fail('Kopia authentication failed', { kind: ctx.KIND.AUTH }); + if (r.status !== 200) ctx.fail(`Kopia returned HTTP ${r.status}`); return { options: (r.data?.sources || []).map(s => ({ value: kopiaSourceId(s.source), label: s.source.path })), }; @@ -119,7 +120,7 @@ async function kopiaSources(row, fetchJSON) { same container makes one round of upstream calls, then write each answer back into the slot's own index. Nulls are kept so the card maps result to slot by position. */ -async function slots(config, fetchJSON) { +async function slots(config, ctx) { const list = Array.isArray(config.slots) ? config.slots : []; const dupGroups = {}, kopiaGroups = {}; @@ -143,8 +144,8 @@ async function slots(config, fetchJSON) { Object.values(dupGroups).map(async ({ base, pass, slots: gs }) => { try { const [stateR, backupsR] = await Promise.all([ - dupFetch(base, pass, '/api/v1/serverstate', fetchJSON), - dupFetch(base, pass, '/api/v1/backups', fetchJSON), + dupFetch(base, pass, '/api/v1/serverstate', ctx), + dupFetch(base, pass, '/api/v1/backups', ctx), ]); if (stateR.status === 401 || backupsR.status === 401) return; const serverState = stateR.data || {}; @@ -166,14 +167,18 @@ async function slots(config, fetchJSON) { nextRun: proposed[id] || dupSchedule(j)?.Time || null, }; }); - } catch {} + } catch (e) { + /* One unreachable instance leaves its own slots null and the rest of the + widget intact, so the failure is logged rather than thrown. */ + ctx.log.warn('backup: duplicati group failed', { base, error: e.message }); + } }), ); await Promise.all( Object.values(kopiaGroups).map(async ({ url, user, pass, slots: gs }) => { try { - const r = await kopiaFetch(url, user, pass, '/api/v1/sources', fetchJSON); + const r = await kopiaFetch(url, user, pass, '/api/v1/sources', ctx); if (r.status !== 200) return; const allSources = r.data?.sources || []; gs.forEach(({ i, jobId, customName }) => { @@ -187,7 +192,9 @@ async function slots(config, fetchJSON) { nextRun: null, }; }); - } catch {} + } catch (e) { + ctx.log.warn('backup: kopia group failed', { url, error: e.message }); + } }), ); @@ -195,14 +202,14 @@ async function slots(config, fetchJSON) { } module.exports = async function backupData(ctx) { - const { endpoint, config, row, fetchJSON } = ctx; + const { endpoint, config, row } = ctx; if (endpoint === 'duplicati-jobs') { - if (!row) throw new Error('no slot selected'); - return duplicatiJobs(row, fetchJSON); + if (!row) ctx.fail('No slot selected', { kind: ctx.KIND.INVALID }); + return duplicatiJobs(row, ctx); } if (endpoint === 'kopia-sources') { - if (!row) throw new Error('no slot selected'); - return kopiaSources(row, fetchJSON); + if (!row) ctx.fail('No slot selected', { kind: ctx.KIND.INVALID }); + return kopiaSources(row, ctx); } - return slots(config, fetchJSON); + return slots(config, ctx); }; diff --git a/ui/widgets/books/data.js b/ui/widgets/books/data.js index d3f01914..0917abbd 100644 --- a/ui/widgets/books/data.js +++ b/ui/widgets/books/data.js @@ -33,10 +33,10 @@ function authErr(r) { /* ───────────────────────── Audiobookshelf ───────────────────────── */ async function absLibrary(ctx, base, hdr) { const r = await jget(ctx, base, '/api/libraries', hdr); - if (authErr(r)) throw new Error('Audiobookshelf auth failed (check API key)'); + if (authErr(r)) ctx.fail('Audiobookshelf auth failed (check API key)', { kind: ctx.KIND.AUTH }); const libs = (r.data && r.data.libraries) || []; const book = libs.find(l => l.mediaType === 'book') || libs[0]; - if (!book) throw new Error('No Audiobookshelf library found'); + if (!book) ctx.fail('No Audiobookshelf library found'); return book.id; } function absBook(li) { @@ -54,7 +54,7 @@ function absBook(li) { async function abs(ctx) { const base = ctx.normalizeBase(ctx.config.absUrl); const key = ctx.config.absKey; - if (!base || !key) throw new Error('Audiobookshelf URL and API key required'); + if (!base || !key) ctx.fail('Audiobookshelf URL and API key required', { kind: ctx.KIND.INVALID }); const hdr = { Authorization: 'Bearer ' + key }; const lib = await absLibrary(ctx, base, hdr); const source = ctx.config.source || 'recently'; @@ -92,7 +92,7 @@ async function abs(ctx) { async function absLists(ctx) { const base = ctx.normalizeBase(ctx.config.absUrl); const key = ctx.config.absKey; - if (!base || !key) throw new Error('Audiobookshelf URL and API key required'); + if (!base || !key) ctx.fail('Audiobookshelf URL and API key required', { kind: ctx.KIND.INVALID }); const hdr = { Authorization: 'Bearer ' + key }; const lib = await absLibrary(ctx, base, hdr); const out = []; @@ -126,7 +126,7 @@ function komgaBook(b) { async function komga(ctx) { const base = ctx.normalizeBase(ctx.config.komgaUrl); const key = ctx.config.komgaKey; - if (!base || !key) throw new Error('Komga URL and API key required'); + if (!base || !key) ctx.fail('Komga URL and API key required', { kind: ctx.KIND.INVALID }); const hdr = { 'X-API-Key': key }; const source = ctx.config.source || 'recently'; let path; @@ -134,7 +134,7 @@ async function komga(ctx) { else if (source === 'unread') path = `/api/v1/books/ondeck?size=${CAP}`; else path = `/api/v1/books/latest?size=${CAP}`; const r = await jget(ctx, base, path, hdr); - if (authErr(r)) throw new Error('Komga auth failed (check API key)'); + if (authErr(r)) ctx.fail('Komga auth failed (check API key)', { kind: ctx.KIND.AUTH }); const content = (r.data && r.data.content) || (Array.isArray(r.data) ? r.data : []); return { provider: 'komga', @@ -147,7 +147,7 @@ async function komga(ctx) { async function komgaLists(ctx) { const base = ctx.normalizeBase(ctx.config.komgaUrl); const key = ctx.config.komgaKey; - if (!base || !key) throw new Error('Komga URL and API key required'); + if (!base || !key) ctx.fail('Komga URL and API key required', { kind: ctx.KIND.INVALID }); const r = await jget(ctx, base, `/api/v1/readlists?size=100`, { 'X-API-Key': key }); const content = (r.data && r.data.content) || []; return { options: content.filter(l => l && l.id).map(l => ({ value: String(l.id), label: l.name || 'Read list' })) }; @@ -156,7 +156,7 @@ async function komgaLists(ctx) { /* ───────────────────────────── Kavita ───────────────────────────── */ async function kavitaToken(ctx, base, key) { const r = await jpost(ctx, base, `/api/Plugin/authenticate?apiKey=${enc(key)}&pluginName=Stackyard`, null, null); - if (authErr(r) || !(r.data && r.data.token)) throw new Error('Kavita auth failed (check API key)'); + if (authErr(r) || !(r.data && r.data.token)) ctx.fail('Kavita auth failed (check API key)', { kind: ctx.KIND.AUTH }); return r.data.token; } function kavitaSeries(s) { @@ -174,7 +174,7 @@ function kavitaSeries(s) { async function kavita(ctx) { const base = ctx.normalizeBase(ctx.config.kavitaUrl); const key = ctx.config.kavitaKey; - if (!base || !key) throw new Error('Kavita URL and API key required'); + if (!base || !key) ctx.fail('Kavita URL and API key required', { kind: ctx.KIND.INVALID }); const tok = await kavitaToken(ctx, base, key); const hdr = { Authorization: 'Bearer ' + tok }; const source = ctx.config.source || 'recently'; @@ -210,7 +210,7 @@ async function kavita(ctx) { async function kavitaLists(ctx) { const base = ctx.normalizeBase(ctx.config.kavitaUrl); const key = ctx.config.kavitaKey; - if (!base || !key) throw new Error('Kavita URL and API key required'); + if (!base || !key) ctx.fail('Kavita URL and API key required', { kind: ctx.KIND.INVALID }); const tok = await kavitaToken(ctx, base, key); const r = await jpost(ctx, base, `/api/ReadingList/lists?PageNumber=1&PageSize=100&includePromoted=true`, { Authorization: 'Bearer ' + tok, diff --git a/ui/widgets/nowplaying/data.js b/ui/widgets/nowplaying/data.js index 5692f772..20fc13cc 100644 --- a/ui/widgets/nowplaying/data.js +++ b/ui/widgets/nowplaying/data.js @@ -22,14 +22,14 @@ const MAX = 5; async function plex(ctx) { const base = ctx.normalizeBase(ctx.config.plexUrl); const token = ctx.config.plexToken; - if (!base || !token) throw new Error('Plex URL and token required'); + if (!base || !token) ctx.fail('Plex URL and token required', { kind: ctx.KIND.INVALID }); // Plex returns XML unless JSON is requested; token may be header or query const r = await ctx.fetchJSON(`${base}/status/sessions`, { headers: { Accept: 'application/json', 'X-Plex-Token': token }, timeout: 8000, }); - if (r.status === 401 || r.status === 403) throw new Error('Plex auth failed (check token)'); - if (r.status >= 400) throw new Error('Plex HTTP ' + r.status); + if (r.status === 401 || r.status === 403) ctx.fail('Plex auth failed (check token)', { kind: ctx.KIND.AUTH }); + if (r.status >= 400) ctx.fail('Plex HTTP ' + r.status); let list = (r.data && r.data.MediaContainer && r.data.MediaContainer.Metadata) || []; if (!Array.isArray(list)) list = [list]; return list.map(m => { @@ -51,14 +51,14 @@ async function jellyfinLike(ctx, provider) { const base = ctx.normalizeBase(provider === 'emby' ? ctx.config.embyUrl : ctx.config.jellyfinUrl); const key = provider === 'emby' ? ctx.config.embyKey : ctx.config.jellyfinKey; const name = provider === 'emby' ? 'Emby' : 'Jellyfin'; - if (!base || !key) throw new Error(name + ' URL and API key required'); + if (!base || !key) ctx.fail(name + ' URL and API key required', { kind: ctx.KIND.INVALID }); /* Header auth works on current and older servers. Jellyfin deprecated the api_key query param (removal targeted for 10.13); Emby keeps its own token header. */ const authHeaders = provider === 'emby' ? { 'X-Emby-Token': key } : { Authorization: `MediaBrowser Token="${key}"` }; const r = await ctx.fetchJSON(`${base}/Sessions`, { headers: authHeaders, timeout: 8000 }); - if (r.status === 401 || r.status === 403) throw new Error(name + ' auth failed'); - if (r.status >= 400) throw new Error(name + ' HTTP ' + r.status); + if (r.status === 401 || r.status === 403) ctx.fail(name + ' auth failed', { kind: ctx.KIND.AUTH }); + if (r.status >= 400) ctx.fail(name + ' HTTP ' + r.status); const list = Array.isArray(r.data) ? r.data : []; const out = []; for (const s of list) { @@ -93,7 +93,7 @@ async function navidrome(ctx) { const base = ctx.normalizeBase(ctx.config.navidromeUrl); const user = ctx.config.navidromeUser, pass = ctx.config.navidromePassword; - if (!base || !user || !pass) throw new Error('Navidrome URL, username and password required'); + if (!base || !user || !pass) ctx.fail('Navidrome URL, username and password required', { kind: ctx.KIND.INVALID }); /* md5(password + salt) is the Subsonic API's authentication token, which Navidrome implements; anything stronger simply fails to authenticate, and the only alternative the protocol offers is sending the password in clear @@ -107,9 +107,10 @@ async function navidrome(ctx) { .digest('hex'); const url = `${base}/rest/getNowPlaying?u=${encodeURIComponent(user)}&t=${token}&s=${salt}&v=1.16.1&c=stackyard&f=json`; const r = await ctx.fetchJSON(url, { timeout: 8000 }); - if (r.status >= 400) throw new Error('Navidrome HTTP ' + r.status); + if (r.status >= 400) ctx.fail('Navidrome HTTP ' + r.status); const sr = r.data && r.data['subsonic-response']; - if (sr && sr.status === 'failed') throw new Error('Navidrome: ' + ((sr.error && sr.error.message) || 'auth failed')); + if (sr && sr.status === 'failed') + ctx.fail('Navidrome rejected the request, check username and password', { kind: ctx.KIND.AUTH }); let entries = (sr && sr.nowPlaying && sr.nowPlaying.entry) || []; if (!Array.isArray(entries)) entries = [entries]; return entries.map(e => {