From 083e74b095352a710d39d9055fb9757fee617dc2 Mon Sep 17 00:00:00 2001 From: SandObserver <260779319+SandObserver@users.noreply.github.com> Date: Tue, 11 Aug 2026 22:34:43 -0400 Subject: [PATCH] Fix: let the config export report a failure The export set an anchor's href to the endpoint and clicked it, which hands the request to the browser. The page never learned the outcome, so the try/catch around the click could not observe one: a refusal, a server error or an expired session was written to disk as an error body under the backup's own filename. The failure surfaced when the backup was needed. Fetch it through ag instead, check the response, and save a blob built from it. An expired session now raises the sign-in box and the export finishes afterwards, since that is what every other admin request already does. The catalog already carried toast.exportFailed, which nothing could reach. --- CHANGELOG.md | 3 ++ ui/js/admin.js | 17 +++++++-- ui/test/config-export.test.mjs | 65 ++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 ui/test/config-export.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 4430892a..940e0575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Exporting the config now reports a failure instead of saving an error response + as `stackyard-config.json`. + - A session that expires while you are working in Admin now raises the sign-in box over the page, and the interrupted save completes once you are back in, instead of failing as "Unauthorised" until the page is reloaded. diff --git a/ui/js/admin.js b/ui/js/admin.js index 338e622a..5ed6ecd9 100644 --- a/ui/js/admin.js +++ b/ui/js/admin.js @@ -10,7 +10,7 @@ import { snapshotItems, saveWithRevert, } from '/js/admin-save-logic.js?v=77cac1d1'; -import { API, toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3'; +import { toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3'; import { checkAuth, requireLogin, wirePasswordStrength } from '/js/admin-auth.js?v=dd849d4c'; import { state } from '/js/admin-state.js?v=3f9ad806'; import { buildWidgetForm } from '/js/admin-widget-form.js?v=653071f0'; @@ -1232,16 +1232,29 @@ function initLanguage() { const dashSaveEl = el('dash-save'); if (dashSaveEl) dashSaveEl.onclick = () => save(); +/* Fetched rather than reached by navigating a link. A link hands the request to + the browser, so the page never learns the outcome: a refusal or an expired + session saved an error body under the backup's own filename, and the catch + here could not see it. Going through `ag` also means an expired session + raises the sign-in box and the export finishes afterwards. */ el('btn-exp').onclick = async () => { + let url; try { + const config = await ag('/api/config/export'); + const blob = new Blob([JSON.stringify(config, null, 2)], { type: 'application/json' }); + url = URL.createObjectURL(blob); const a = document.createElement('a'); - a.href = API + '/api/config/export'; + a.href = url; a.download = 'stackyard-config.json'; document.body.appendChild(a); a.click(); document.body.removeChild(a); } catch (e) { toast(t('toast.exportFailed', { err: e.message }), 'err'); + } finally { + /* Revoked on the next frame: revoking it in this one races the download the + click just started, and the browser keeps nothing to save. */ + if (url) requestAnimationFrame(() => URL.revokeObjectURL(url)); } }; el('imp').onchange = async e => { diff --git a/ui/test/config-export.test.mjs b/ui/test/config-export.test.mjs new file mode 100644 index 00000000..de7c9106 --- /dev/null +++ b/ui/test/config-export.test.mjs @@ -0,0 +1,65 @@ +/* Regression test for the config export, which could not report a failure. + + The export set an anchor's href to /api/config/export and clicked it, which + hands the request to the browser. The page then never learns the outcome: a + refusal, a server error or an expired session was written to disk as an error + body under the backup's own filename, and the try/catch around the click + could not observe any of it. The failure surfaces when the backup is needed, + which is the worst moment to find out. + + Fetching it instead means the response is checked before anything is saved, + and going through `ag` means an expired session raises the sign-in box and + the export finishes afterwards. + + Scanned from the source rather than driven, because the download itself is + browser machinery: what matters is which of the two routes the code takes. */ + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const admin = fs.readFileSync(path.join(root, 'js/admin.js'), 'utf8'); + +/* The handler only, so an unrelated mention of the endpoint elsewhere in the + file cannot pass or fail this. */ +function exportHandler() { + const start = admin.indexOf("el('btn-exp').onclick"); + assert.notEqual(start, -1, 'the export handler moved; this test needs updating'); + const end = admin.indexOf("el('imp').onchange", start); + assert.notEqual(end, -1, 'the export handler moved; this test needs updating'); + return admin.slice(start, end); +} + +test('the export never reaches the endpoint by navigating to it', () => { + const body = exportHandler(); + assert.ok( + !/\.href\s*=\s*[^;]*\/api\/config\/export/.test(body), + 'assigning the endpoint to an href hands the request to the browser, which cannot report a failure', + ); +}); + +test('the export goes through the shared helper, so a failure is visible', () => { + const body = exportHandler(); + assert.match( + body, + /await ag\(\s*'\/api\/config\/export'\s*\)/, + 'the response has to be checked before anything is saved', + ); +}); + +test('the export reports a failure it can now see', () => { + assert.match(exportHandler(), /toast\(\s*t\('toast\.exportFailed'/); +}); + +test('the object URL is released rather than leaked', () => { + const body = exportHandler(); + assert.match(body, /createObjectURL/); + assert.match(body, /revokeObjectURL/, 'a blob URL held for the page lifetime pins the whole config in memory'); +}); + +test('the download keeps the filename users already know', () => { + assert.match(exportHandler(), /download\s*=\s*'stackyard-config\.json'/); +});