fix(admin): legacy pages show why a request was refused
A job running in the workspace refuses the legacy record-changing requests,
and a corrupt config.json refuses a settings save. The legacy pages now
name the reason instead of hiding it:
- LoRA import: the busy and failure frames carry status and progress, and
the admin modal handles type "error": it shows the message, notifies and
closes, instead of staying on "Initializing..." at 0%.
- Legacy batch auto-tag: a refusal by another job is a 200 stream with one
error frame ({type, status, message}), which both pages already render;
a second batch keeps its exact JSON 409.
- Gallery cache clear: a refused server clear shows the reason and stops
before the success toast and the "Not generated" reset.
- Admin settings save and maintenance show the reply's error ("config.json
cannot be read...", "Scan for metadata already running") instead of
"Failed to save settings" or "HTTP 409: Conflict".
- Tests cover the read-only /maintenance branch: a read-only selection runs
during a scan as a read hold; a mixed or default selection is refused.
This commit is contained in:
@@ -64,6 +64,12 @@ def path_is_within(candidate, root) -> bool:
|
||||
return child != parent and child.is_relative_to(parent)
|
||||
|
||||
|
||||
async def _refusal(title):
|
||||
"""The single error event of an auto-tag batch refused by another job."""
|
||||
message = f"{title} already running"
|
||||
yield {"type": "error", "status": "error", "message": message}
|
||||
|
||||
|
||||
class AutotagRoutesMixin:
|
||||
"""Mixin providing auto-tagging API endpoints."""
|
||||
|
||||
@@ -454,7 +460,10 @@ class AutotagRoutesMixin:
|
||||
return web.json_response(
|
||||
{"success": False, "error": _BUSY_ERROR}, status=409
|
||||
)
|
||||
return self._long_job_busy_response(busy.title)
|
||||
# Both legacy pages read this route as a stream, where a 409 reads
|
||||
# as a lost connection: refuse with one error frame instead (the
|
||||
# admin page keys on ``type``, the gallery on ``status``).
|
||||
return await self._stream_sse(request, _refusal(busy.title))
|
||||
async with self._jobs.hold(
|
||||
AUTOTAG_BATCH_JOB_KIND,
|
||||
"Auto-tag images (legacy page)",
|
||||
|
||||
@@ -37,6 +37,15 @@ def _lora_settings_mutation(values):
|
||||
return mutate
|
||||
|
||||
|
||||
def _lora_error_frame(message):
|
||||
"""An import stream's error event, as the admin page's modal reads it.
|
||||
|
||||
The modal shows ``status`` and ``progress`` from every frame and closes
|
||||
on ``type: error``, naming ``message``.
|
||||
"""
|
||||
return {"type": "error", "status": message, "progress": 0, "message": message}
|
||||
|
||||
|
||||
class LoraIntegrationMixin:
|
||||
"""Mixin providing LoraManager detection, scanning, and trigger word endpoints."""
|
||||
|
||||
@@ -290,7 +299,7 @@ class LoraIntegrationMixin:
|
||||
if busy is not None:
|
||||
await response.prepare(request)
|
||||
message = f"{busy.title} already running"
|
||||
await send_progress({"type": "error", "message": message})
|
||||
await send_progress(_lora_error_frame(message))
|
||||
await response.write_eof()
|
||||
return response
|
||||
|
||||
@@ -303,9 +312,7 @@ class LoraIntegrationMixin:
|
||||
except Exception as e:
|
||||
self.logger.exception("LoRA scan failed")
|
||||
job.finish("failed")
|
||||
await send_progress(
|
||||
{"type": "error", "message": self._public_error(e)}
|
||||
)
|
||||
await send_progress(_lora_error_frame(self._public_error(e)))
|
||||
|
||||
await response.write_eof()
|
||||
return response
|
||||
|
||||
@@ -0,0 +1,150 @@
|
||||
// The legacy admin and gallery pages show a server refusal's reason (#220, PR #289 review I1 and M8): a job that
|
||||
// runs in the workspace refuses their record-changing requests, and a corrupt config.json refuses a settings save.
|
||||
// Each page must name the reason, not report a lost connection, a bare HTTP status or a success.
|
||||
const test = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const vm = require('node:vm');
|
||||
|
||||
function elements() {
|
||||
const nodes = {};
|
||||
return {
|
||||
nodes,
|
||||
getElementById: id => nodes[id] ||= { id, textContent: '', innerHTML: '', disabled: false, style: {},
|
||||
value: '', checked: false, addEventListener() {}, querySelectorAll: () => [] },
|
||||
};
|
||||
}
|
||||
|
||||
function jsonResponse(status, body) {
|
||||
return { ok: status >= 200 && status < 300, status, statusText: status === 409 ? 'Conflict' : 'OK',
|
||||
json: async () => body };
|
||||
}
|
||||
|
||||
function sseResponse(frames) {
|
||||
const bytes = new TextEncoder().encode(frames.map(frame => `data: ${JSON.stringify(frame)}\n\n`).join(''));
|
||||
let sent = false;
|
||||
return { ok: true, status: 200, body: { getReader: () => ({
|
||||
read: async () => (sent ? { done: true } : (sent = true, { done: false, value: bytes })),
|
||||
}) } };
|
||||
}
|
||||
|
||||
function load(file, className, fetchUrl, extra = {}, dom = elements()) {
|
||||
const timers = [];
|
||||
const notifications = [];
|
||||
const storage = new Map([['pm_cached', '1'], ['userData', 'keep']]);
|
||||
const localStorage = {
|
||||
getItem: key => storage.get(key) ?? null,
|
||||
setItem: (key, value) => storage.set(key, String(value)),
|
||||
removeItem: key => storage.delete(key),
|
||||
};
|
||||
const context = vm.createContext({
|
||||
document: { readyState: 'loading', addEventListener() {}, getElementById: dom.getElementById,
|
||||
querySelectorAll: () => [] },
|
||||
window: {}, console: { error() {}, warn() {}, debug() {} },
|
||||
setTimeout: callback => timers.push(callback),
|
||||
confirm: () => true,
|
||||
localStorage,
|
||||
Object: { ...Object, keys: target => (target === localStorage ? [...storage.keys()] : Object.keys(target)) },
|
||||
TextDecoder, TextEncoder, JSON,
|
||||
ApiClient: { fetchUrl },
|
||||
...extra,
|
||||
});
|
||||
const source = fs.readFileSync(path.join(__dirname, `../../web/js/${file}`), 'utf8');
|
||||
const Page = vm.runInContext(`${source}\n${className};`, context, { filename: path.join(__dirname, `../../web/js/${file}`) });
|
||||
const page = Object.create(Page.prototype);
|
||||
page.showNotification = (message, type) => notifications.push([type, message]);
|
||||
page.showModal = id => notifications.push(['show', id]);
|
||||
page.hideModal = id => notifications.push(['hide', id]);
|
||||
return { page, dom, timers, notifications, storage };
|
||||
}
|
||||
|
||||
const BUSY = 'Scan for metadata already running';
|
||||
|
||||
test('gallery cache clear reports a refused server clear and never claims success', async () => {
|
||||
const calls = [];
|
||||
const { page, dom, notifications, storage } = load('gallery.js', 'GalleryManager', async url => {
|
||||
calls.push(url);
|
||||
return jsonResponse(409, { success: false, error: BUSY });
|
||||
});
|
||||
page.getSettings = () => ({ thumbnailsGenerated: true });
|
||||
dom.getElementById('thumbnailStatus').textContent = 'Generated';
|
||||
|
||||
await page.clearCache();
|
||||
|
||||
assert.deepEqual(calls, ['/prompt_manager/images/clear-thumbnails']);
|
||||
assert.deepEqual(notifications, [['error', `Thumbnails were not cleared: ${BUSY}`]]);
|
||||
assert.equal(dom.nodes.thumbnailStatus.textContent, 'Generated');
|
||||
assert.equal(storage.get('gallerySettings'), undefined); // thumbnailsGenerated is not reset
|
||||
});
|
||||
|
||||
test('gallery cache clear still says so when the server clears', async () => {
|
||||
const { page, dom, notifications, storage } = load('gallery.js', 'GalleryManager',
|
||||
async () => jsonResponse(200, { success: true, cleared_count: 3 }));
|
||||
page.getSettings = () => ({ thumbnailsGenerated: true });
|
||||
|
||||
await page.clearCache();
|
||||
|
||||
assert.deepEqual(notifications, [['success', 'PromptManager cache cleared successfully! Original images untouched.']]);
|
||||
assert.equal(dom.nodes.thumbnailStatus.textContent, 'Not generated');
|
||||
assert.equal(JSON.parse(storage.get('gallerySettings')).thumbnailsGenerated, false);
|
||||
assert.equal(storage.get('userData'), 'keep');
|
||||
assert.equal(storage.has('pm_cached'), false);
|
||||
});
|
||||
|
||||
test('admin LoRA import shows a refusal frame and closes its modal', async () => {
|
||||
const { page, dom, timers, notifications } = load('admin.js', 'PromptAdmin',
|
||||
async () => sseResponse([{ type: 'error', status: BUSY, progress: 0, message: BUSY }]));
|
||||
page.saveLoraSettings = async () => true;
|
||||
page.loadStatistics = () => assert.fail('a refused import must not reload statistics');
|
||||
|
||||
await page.runLoraImport();
|
||||
|
||||
assert.equal(dom.nodes.loraImportStatus.textContent, `Error: ${BUSY}`);
|
||||
assert.deepEqual(notifications.filter(([type]) => type === 'error'), [['error', `LoRA import failed: ${BUSY}`]]);
|
||||
assert.equal(timers.length, 1);
|
||||
timers[0]();
|
||||
assert.deepEqual(notifications.at(-1), ['hide', 'loraImportModal']);
|
||||
});
|
||||
|
||||
test('admin settings save shows why config.json refused it', async () => {
|
||||
const message = 'config.json cannot be read, so nothing was saved. Fix or remove the file, then save again.';
|
||||
const { page, notifications } = load('admin.js', 'PromptAdmin',
|
||||
async () => jsonResponse(409, { success: false, persisted: false, code: 'config-unreadable', error: message }));
|
||||
page.settings = {};
|
||||
page._collectScanPaths = () => [];
|
||||
page.saveLoraSettings = async () => assert.fail('nothing else is saved after a refusal');
|
||||
|
||||
await page.saveSettings();
|
||||
|
||||
assert.deepEqual(notifications, [['error', `Failed to save settings: ${message}`]]);
|
||||
});
|
||||
|
||||
test('admin settings save keeps its plain message when the reply has no reason', async () => {
|
||||
const { page, notifications } = load('admin.js', 'PromptAdmin', async () => ({ ok: false, status: 500,
|
||||
json: async () => { throw new SyntaxError('not json'); } }));
|
||||
page.settings = {};
|
||||
page._collectScanPaths = () => [];
|
||||
|
||||
await page.saveSettings();
|
||||
|
||||
assert.deepEqual(notifications, [['error', 'Failed to save settings']]);
|
||||
});
|
||||
|
||||
test('admin maintenance names the job that refused it', async () => {
|
||||
const { page, dom, notifications } = load('admin.js', 'PromptAdmin',
|
||||
async () => jsonResponse(409, { success: false, error: BUSY }), {
|
||||
escapeHtml: text => String(text),
|
||||
document: { readyState: 'loading', addEventListener() {}, getElementById: id => dom.getElementById(id),
|
||||
querySelectorAll: selector => (selector === '.maintenance-option:checked' ? [{ id: 'vacuum' }] : []) },
|
||||
});
|
||||
|
||||
await page.runMaintenance();
|
||||
|
||||
const results = dom.nodes.maintenanceResults.innerHTML;
|
||||
assert.match(results, /Maintenance Failed/);
|
||||
assert.match(results, new RegExp(BUSY));
|
||||
assert.doesNotMatch(results, /HTTP 409/);
|
||||
assert.deepEqual(notifications, [['error', '❌ Maintenance failed']]);
|
||||
assert.equal(dom.nodes.runMaintenanceBtn.disabled, false);
|
||||
});
|
||||
@@ -767,6 +767,12 @@ class TestLoraScanRoute(LoraAPITestCase):
|
||||
body = await resp.text()
|
||||
self.assertNotIn(str(self.tmp_root), body)
|
||||
self.assertIn("x.db", body)
|
||||
# The admin page's modal shows the failure as its status and closes.
|
||||
error = self._sse_events(body)[-1]
|
||||
self.assertEqual(
|
||||
(error["type"], error["status"], error["progress"]),
|
||||
("error", error["message"], 0),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _sse_events(text):
|
||||
|
||||
@@ -295,6 +295,70 @@ class TestLegacyRoutesHoldTheRegistry(_LegacyHoldCase):
|
||||
await self._busy(resp)
|
||||
self.assertEqual(self._db_digest(), before)
|
||||
|
||||
async def test_read_only_maintenance_runs_during_scan(self):
|
||||
# The legacy statistics and consistency checks only read: they hold
|
||||
# the registry as "read", so a long scan does not block them.
|
||||
self.api.db.save_prompt(text="keep me", prompt_hash="h1")
|
||||
_gate, scan = self._run_job()
|
||||
listed = []
|
||||
real = self.api._run_maintenance_operations
|
||||
|
||||
def observed(operations):
|
||||
listed.extend(
|
||||
(job.kind, job.admission, job.state) for job in self.api._jobs.list()
|
||||
)
|
||||
return real(operations)
|
||||
|
||||
self.api._run_maintenance_operations = observed
|
||||
resp = await self.client.request(
|
||||
"POST",
|
||||
"/prompt_manager/maintenance",
|
||||
json={
|
||||
"operations": [
|
||||
"statistics",
|
||||
"check_consistency",
|
||||
"check_hash_duplicates",
|
||||
]
|
||||
},
|
||||
)
|
||||
body = await resp.json()
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertIs(body["success"], True)
|
||||
self.assertEqual(
|
||||
set(body["results"]),
|
||||
{"statistics", "check_consistency", "check_hash_duplicates"},
|
||||
)
|
||||
self.assertIn(("maintenance", "read", "running"), listed)
|
||||
self.assertEqual(scan.state, "running")
|
||||
maintenance = self.api._jobs.list()[-1]
|
||||
self.assertEqual(
|
||||
(maintenance.kind, maintenance.admission, maintenance.state),
|
||||
("maintenance", "read", "finished"),
|
||||
)
|
||||
|
||||
async def test_mixed_maintenance_selection_is_refused_during_scan(self):
|
||||
# One write operation among read-only ones makes the run "records".
|
||||
self.api.db.save_prompt(text="keep me", prompt_hash="h1")
|
||||
before = self._db_digest()
|
||||
self._run_job()
|
||||
resp = await self.client.request(
|
||||
"POST",
|
||||
"/prompt_manager/maintenance",
|
||||
json={"operations": ["statistics", "vacuum"]},
|
||||
)
|
||||
await self._busy(resp)
|
||||
self.assertEqual(self._db_digest(), before)
|
||||
|
||||
async def test_default_maintenance_selection_is_refused_during_scan(self):
|
||||
# No "operations" key means the default list, which vacuums.
|
||||
self.api.db.save_prompt(text="keep me", prompt_hash="h1")
|
||||
before = self._db_digest()
|
||||
self._run_job()
|
||||
resp = await self.client.request("POST", "/prompt_manager/maintenance", json={})
|
||||
await self._busy(resp)
|
||||
self.assertEqual(self._db_digest(), before)
|
||||
self.assertEqual([j.kind for j in self.api._jobs.list()], ["scan"])
|
||||
|
||||
async def test_legacy_maintenance_refused_while_image_tags_runs(self):
|
||||
self._run_job("image-tags", "Add tags to 3 images")
|
||||
resp = await self.client.request("POST", "/prompt_manager/maintenance")
|
||||
@@ -365,18 +429,43 @@ class TestLegacyRoutesHoldTheRegistry(_LegacyHoldCase):
|
||||
text = await resp.text()
|
||||
self.assertEqual(resp.status, 200) # the legacy page reads a stream
|
||||
events = [json.loads(line[6:]) for line in text.split("\n\n") if line]
|
||||
# The admin page's modal shows ``status`` and closes on ``type: error``.
|
||||
message = "Scan for metadata already running"
|
||||
self.assertEqual(
|
||||
events,
|
||||
[{"type": "error", "message": "Scan for metadata already running"}],
|
||||
[{"type": "error", "status": message, "progress": 0, "message": message}],
|
||||
)
|
||||
self.assertEqual(imported, [])
|
||||
|
||||
async def test_legacy_autotag_refused_while_scan_runs(self):
|
||||
# Both pages read this route as a stream (a 409 reads as a lost
|
||||
# connection there), so a refusal by another job is one error frame:
|
||||
# the admin page keys on ``type``, the gallery on ``status``.
|
||||
self._run_job()
|
||||
resp = await self.client.request(
|
||||
"POST", "/prompt_manager/autotag/start", json={}
|
||||
)
|
||||
await self._busy(resp)
|
||||
text = await resp.text()
|
||||
self.assertEqual(resp.status, 200)
|
||||
self.assertEqual(resp.headers["Content-Type"], "text/event-stream")
|
||||
events = [json.loads(line[6:]) for line in text.split("\n\n") if line]
|
||||
message = "Scan for metadata already running"
|
||||
self.assertEqual(
|
||||
events, [{"type": "error", "status": "error", "message": message}]
|
||||
)
|
||||
self.assertEqual(
|
||||
[j.kind for j in self.api._jobs.list() if j.kind == "autotag"], []
|
||||
)
|
||||
|
||||
async def test_second_legacy_autotag_batch_keeps_its_json_409(self):
|
||||
from py.api.autotag_routes import _BUSY_ERROR
|
||||
|
||||
self._run_job("autotag", "Auto-tag images (legacy page)")
|
||||
resp = await self.client.request(
|
||||
"POST", "/prompt_manager/autotag/start", json={}
|
||||
)
|
||||
self.assertEqual(resp.status, 409)
|
||||
self.assertEqual(await resp.json(), {"success": False, "error": _BUSY_ERROR})
|
||||
|
||||
async def test_legacy_payloads_unchanged_when_idle(self):
|
||||
resp = await self.client.request(
|
||||
|
||||
+13
-3
@@ -1180,6 +1180,11 @@
|
||||
`Done — ${data.imported} imported, ${data.skipped} skipped`;
|
||||
this.loadStatistics();
|
||||
setTimeout(() => this.hideModal("loraImportModal"), 2000);
|
||||
} else if (data.type === "error") {
|
||||
// A refusal (another job runs) or a failed import: name it and close the modal.
|
||||
document.getElementById("loraImportStatus").textContent = `Error: ${data.message}`;
|
||||
this.showNotification(`LoRA import failed: ${data.message}`, "error");
|
||||
setTimeout(() => this.hideModal("loraImportModal"), 3000);
|
||||
}
|
||||
} catch (e) { /* skip malformed SSE */ }
|
||||
}
|
||||
@@ -1231,10 +1236,13 @@
|
||||
}
|
||||
this.hideModal("settingsModal");
|
||||
} else {
|
||||
throw new Error("Failed to save settings");
|
||||
// A refusal names its reason (a corrupt config.json, a changed file): show it.
|
||||
const data = await response.json().catch(() => ({}));
|
||||
throw new Error(data.error ? `Failed to save settings: ${data.error}` : "Failed to save settings");
|
||||
}
|
||||
} catch (error) {
|
||||
this.showNotification("Failed to save settings", "error");
|
||||
const reason = error.message.startsWith("Failed to save settings:") ? error.message : "Failed to save settings";
|
||||
this.showNotification(reason, "error");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1848,7 +1856,9 @@
|
||||
throw new Error(data.error || 'Maintenance failed');
|
||||
}
|
||||
} else {
|
||||
throw new Error(`HTTP ${response.status}: ${response.statusText}`);
|
||||
// A refusal names the job that runs ("Scan for metadata already running").
|
||||
const data = await response.json().catch(() => ({}));
|
||||
throw new Error(data.error || `HTTP ${response.status}: ${response.statusText}`);
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Maintenance error:', error);
|
||||
|
||||
+5
-2
@@ -2129,8 +2129,11 @@ Seed: ${this.currentMetadata.seed || 'Unknown'}`;
|
||||
headers: { 'Content-Type': 'application/json' }
|
||||
});
|
||||
|
||||
if (response.ok) {
|
||||
const data = await response.json();
|
||||
if (!response.ok) {
|
||||
// Refused (a running job holds the thumbnails): say why and claim nothing.
|
||||
const data = await response.json().catch(() => ({}));
|
||||
this.showNotification(`Thumbnails were not cleared: ${data.error || `HTTP ${response.status}`}`, 'error');
|
||||
return;
|
||||
}
|
||||
} catch (e) {
|
||||
console.warn('Could not clear server thumbnails:', e);
|
||||
|
||||
Reference in New Issue
Block a user