From aa39815e2e866a2c36aba66b5e3c6af07e20594f Mon Sep 17 00:00:00 2001 From: Thales <> Date: Sun, 16 Aug 2026 20:48:07 +0100 Subject: [PATCH 1/4] feat(ui): report a failure from the notification centre A failure used to live in a transient #error banner. Dismiss it, or reload, and the evidence was gone -- which is the position #359 complained about, where a reporter has nothing to paste and guesses at a cause instead. #343 is the standing proof: its author blamed a GPU and sent the investigation the wrong way. This session hit the same wall, a "demucs exited 1 (no stderr captured)" that was really a missing ffmpeg on PATH. Failures now land in the notification centre, survive a reload, and open a dialog that can hand the whole thing to GitHub as a pre-filled bug report -- version, OS, install method, stage, device, model and the stderr tail already in the form. The user adds what they were doing and ticks the two preflight boxes, which GitHub cannot prefill and which are the point. Covers import (foreground and background), playback, export and update failures. A background import that failed used to say nothing whatsoever: no banner, no queue UI, just a console warning and a library row identical to a healthy one. Queue three tracks, lose one, never find out. - Deliberately not wired into showError wholesale: it also carries benign validation ("Only MP3, WAV... are supported"), which must not file a bug. - One failure, one card. The foreground SSE handler and the background queue reconciler can both notice the same dead job, and applyState can run its error branch on more than one frame, so records key on the job id. - classify_failure()'s "unknown" sentinel is dropped rather than shown: as a card it read "Import failed - unknown", and as an issue title it grouped every unclassified failure under one meaningless heading. Privacy: the report carries technical details only. Track title and source URL are never included -- issues are public, and the user adds them if they help. GET /api/jobs/{id}/failure enforces that server-side by parsing error.txt and serving a whitelist, rather than trusting the client to filter the file. That endpoint also closes a gap: the pipeline has written the quarantined error.txt since #277 -- classified cause, device, model, timings, 40-line stderr tail -- and nothing ever read it back, so the UI had only the one-line error_detail. It is the difference between "demucs failed" and "CUDA out of memory: tried to allocate 2.40 GiB". The notification centre had no generic add-a-card path: one hardcoded release card, and badge/empty-state toggled inline at its two call sites assuming exactly one card. That is centralised in notifications.js now, with the release card keeping its own per-version dismissal key. Tests: tests/js/report-url.test.mjs pins the dropdown strings (an OS that does not match an option exactly is dropped by GitHub without complaint), the URL length ceiling, tail truncation keeping the end where the error is, and that no title or source URL can appear. tests/e2e/report-failure.spec.mjs covers the desktop path, where the link is intercepted and handed to open_url rather than navigating -- a break there would do nothing in the shipped app while working in every browser a developer tests in. --- app/api/jobs.py | 55 +++++ static/css/daw.css | 62 +++++ static/index.html | 25 ++ static/js/catalog.js | 68 +++++- static/js/job.js | 20 +- static/js/main.js | 16 +- static/js/notifications.js | 389 ++++++++++++++++++++++++++++++ static/js/player.js | 4 +- tests/e2e/report-failure.spec.mjs | 107 ++++++++ tests/js/report-url.test.mjs | 152 ++++++++++++ tests/test_failure_api.py | 100 ++++++++ 11 files changed, 986 insertions(+), 12 deletions(-) create mode 100644 static/js/notifications.js create mode 100644 tests/e2e/report-failure.spec.mjs create mode 100644 tests/js/report-url.test.mjs create mode 100644 tests/test_failure_api.py diff --git a/app/api/jobs.py b/app/api/jobs.py index 1e78141..cdc9440 100644 --- a/app/api/jobs.py +++ b/app/api/jobs.py @@ -424,6 +424,61 @@ def _beats_paths(job_id: str) -> tuple[Path, Path]: return stems / "beats.json", stems / "beats.user.json" +# Keys of error.txt that may be handed to the client. `title` and `source` are +# deliberately absent: this feeds a "report it on GitHub" flow whose issues are +# public, and the user adds what they were working on if they want to. The +# server is the right place to enforce that -- not the client that builds the +# report body. +_FAILURE_PUBLIC_KEYS = frozenset( + ("time", "stage", "device", "model", "cause", "timings", "exception") +) + + +@router.get("/{job_id}/failure") +def get_failure(job_id: str) -> dict: + """Return the quarantined failure evidence for a job that errored. + + _quarantine_failed_job writes jobs/failed//error.txt on every pipeline + failure (#277) and until now nothing ever read it back: the UI had only the + one-line `error_detail`, so a bug report could not carry the stderr tail + that says *why* demucs died. Read-only, and never serves the whole file -- + only the technical keys above, plus the tail. + """ + if not JOB_ID_RE.match(job_id): + raise HTTPException(status_code=404, detail="job not found") + + # JOB_ID_RE rejects "failed", so the quarantine dir can never be addressed + # as a job id; join it explicitly and re-verify the result stays inside. + failed_dir = (JOBS_DIR / "failed" / job_id).resolve() + if not failed_dir.is_relative_to((JOBS_DIR / "failed").resolve()): + raise HTTPException(status_code=404, detail="job not found") + path = failed_dir / "error.txt" + if not path.is_file(): + raise HTTPException(status_code=404, detail="no failure evidence for this job") + + try: + text = path.read_text(encoding="utf-8") + except OSError as exc: + logger.exception("unreadable failure evidence for %s", job_id) + raise HTTPException(status_code=404, detail="no failure evidence for this job") from exc + + fields: dict[str, str] = {} + tail: list[str] = [] + in_tail = False + for line in text.splitlines(): + if line.strip() == "--- stderr tail ---": + in_tail = True + continue + if in_tail: + tail.append(line) + continue + key, sep, value = line.partition(":") + if sep and key in _FAILURE_PUBLIC_KEYS: + fields[key] = value.strip() + + return {"job_id": job_id, **fields, "tail": tail} + + @router.get("/{job_id}/beats") def get_beats(job_id: str) -> Response: """Return the beat grid, preferring the user's edits over the detected one. diff --git a/static/css/daw.css b/static/css/daw.css index 491e78b..ceddb87 100644 --- a/static/css/daw.css +++ b/static/css/daw.css @@ -331,6 +331,15 @@ input, textarea { font-family: inherit; } border-radius: 5px; } .daw-notif-close:hover { background: var(--panel-3); color: var(--fg); } +/* Sits between the label and the close button; the header is space-between, so + push it right and leave a gap before the X. */ +.daw-notif-clear { + margin-left: auto; margin-right: 8px; + background: none; border: none; padding: 0; + color: var(--muted); cursor: pointer; + font-family: inherit; font-size: 11px; +} +.daw-notif-clear:hover { color: var(--fg); text-decoration: underline; } .daw-notif-list { padding: 8px; display: flex; flex-direction: column; gap: 6px; } .daw-notif-card { display: flex; @@ -349,6 +358,15 @@ input, textarea { font-family: inherit; } border-radius: 6px; color: var(--accent); } +/* A failure, not an announcement. The amber tint lives in the base icon rule + rather than in .daw-notif-release, so both properties need overriding. */ +.daw-notif-error { border-color: rgba(214,90,74,0.35); cursor: pointer; } +.daw-notif-error:hover { background: var(--panel-2); } +.daw-notif-error .daw-notif-card-icon { + background: rgba(214,90,74,0.14); + color: var(--danger); +} +.daw-notif-error:focus-visible { outline: 2px solid var(--accent); outline-offset: 2px; } .daw-notif-card-body { flex: 1; min-width: 0; } .daw-notif-card-title { font-size: 12px; font-weight: 600; color: var(--fg); } .daw-notif-card-desc { font-size: 11px; color: var(--muted); margin-top: 2px; } @@ -2740,6 +2758,50 @@ input, textarea { font-family: inherit; } /* The "New release available" card opens the dialog on click. */ .daw-notif-release { cursor: pointer; } +/* ── Failure dialog ── */ +/* Same shell as the release dialog, in the danger colour, with the technical + block the report will carry shown verbatim -- nothing is sent that the user + has not been able to read first. */ +.failure-card { + width: min(560px, calc(100vw - 32px)); + max-height: calc(100vh - 64px); + display: flex; + flex-direction: column; + align-items: center; +} +.failure-logo { color: var(--danger); } +.failure-when { color: var(--muted); } +.failure-message { + margin: 12px 0 0; + font-size: 13px; line-height: 1.5; color: var(--fg-2); + white-space: pre-line; text-align: center; +} +.failure-hint { + margin: 10px 0 0; + font-size: 11px; line-height: 1.5; color: var(--muted); + text-align: center; +} +.failure-tech { + width: 100%; flex: 1 1 auto; min-height: 0; + margin: 12px 0 0; padding: 10px 12px; + overflow: auto; text-align: left; + background: var(--bg); border: 1px solid var(--border); border-radius: 8px; +} +.failure-tech code { + font-family: var(--font-mono); font-size: 11px; line-height: 1.55; + color: var(--fg-2); white-space: pre-wrap; overflow-wrap: anywhere; +} +/* Reporting is the reason this dialog exists, so it gets the app's primary + button treatment (as Export Mix does) rather than the muted link style the + About/release dialogs use for their several equal-weight links. */ +.failure-card .about-link-primary { + background: var(--accent); border-color: transparent; color: #1a1206; +} +.failure-card .about-link-primary:hover { + background: color-mix(in srgb, var(--accent) 85%, white); + color: #1a1206; +} + /* ── Library sections (Recent · Stem Collections · Tags) ── */ .lib-section { display: flex; diff --git a/static/index.html b/static/index.html index b43b3bb..bf64997 100644 --- a/static/index.html +++ b/static/index.html @@ -109,6 +109,7 @@ + + + diff --git a/static/js/catalog.js b/static/js/catalog.js index 8f3828f..26d86cc 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -10,6 +10,7 @@ import { startQueueStream, } from "./queue.js"; import { fmtTime, storeGet, storeSet } from "./utils.js"; +import { notifyFailure, setReleasePending } from "./notifications.js"; // Escape user-supplied strings before inserting into innerHTML. function esc(s) { @@ -1548,6 +1549,26 @@ async function completeSettledJob(jobId) { render(); return; } + // A background job that failed used to say nothing at all: no banner (that + // belongs to the foreground import), no queue UI, just a console warning + // and a library row indistinguishable from a healthy one. Queue three + // tracks, lose one, never find out. It gets a notification like any other + // failure now. + if (state.status === "error") { + notifyFailure({ + kind: "import", + message: state.error || "Audio processing failed.", + detail: state.error_detail || null, + context: { + jobId, + stage: state.stage, + device: state.compute_device, + gpuFallback: state.gpu_fallback, + timings: state.stage_timings ? JSON.stringify(state.stage_timings) : null, + }, + }); + } + const track = stateMetadataToTrack(state, { ...existing, id: jobId }); track.id = jobId; track.channel = state.status === "done" ? "Extracted" : existing.channel; @@ -1993,15 +2014,31 @@ function setDisplayedVersion(version) { if (about) about.textContent = `v${currentVersion}`; } +// Kept from the health check so a bug report can state the running version, +// model and ffmpeg status without a second round trip. +let healthInfo = {}; + async function loadCurrentVersion() { try { const res = await fetch("/api/health", { cache: "no-store" }); if (!res.ok) return; const data = await res.json(); + healthInfo = data; setDisplayedVersion(data.version); } catch (e) { console.warn("[catalog] version fetch failed:", e); } } +/** Everything a bug report needs about this install. */ +export async function collectDiagnostics() { + return { + version: currentVersion, + model: healthInfo.demucs_model, + ffmpegConfigured: healthInfo.ffmpeg_configured, + buildTarget: await getBuildTarget(), + isDesktop: Boolean(window.__TAURI__?.core?.invoke), + }; +} + function escapeHtml(value) { return String(value == null ? "" : value) .replace(/&/g, "&") @@ -2111,7 +2148,7 @@ function renderReleaseNotes(markdown) { // Resolve the running build's OS/arch/GPU variant. On desktop this is exact // (Rust build_target); on web/server there is no reliable signal, so guess the // OS from the user agent and leave the variant as CPU. -async function getBuildTarget() { +export async function getBuildTarget() { if (cachedBuildTarget) return cachedBuildTarget; const invoke = window.__TAURI__?.core?.invoke; if (invoke) { @@ -2230,14 +2267,13 @@ async function checkForUpdate() { const card = document.getElementById("notifReleaseCard"); const desc = document.getElementById("notifReleaseDesc"); - const badge = document.getElementById("notifBadge"); - const empty = document.getElementById("notifEmpty"); const dismissBtn = document.getElementById("notifReleaseDismiss"); if (desc) desc.textContent = `v${latest}`; card?.classList.remove("hidden"); - badge?.classList.remove("hidden"); - empty?.classList.add("hidden"); + // The badge and empty state are shared with failure cards now, so they are + // decided in one place from the full set rather than toggled from here. + setReleasePending(true); // Clicking the card (anywhere but the dismiss button) opens the release dialog. card?.addEventListener("click", (e) => { @@ -2249,10 +2285,20 @@ async function checkForUpdate() { e.stopPropagation(); try { localStorage.setItem(DISMISSED_UPDATE_KEY, latest); } catch (e) { console.warn(e); } card?.classList.add("hidden"); - badge?.classList.add("hidden"); - empty?.classList.remove("hidden"); + setReleasePending(false); }, { once: true }); - } catch (e) { console.warn("[catalog] update check failed:", e); } + } catch (e) { + console.warn("[catalog] update check failed:", e); + // Only report a genuine failure, not "we are offline": an update check that + // cannot reach GitHub is not a StemDeck bug and must not file one. + if (!(e instanceof TypeError)) { + notifyFailure({ + kind: "update", + message: "Could not check for updates.", + detail: String(e?.message || e), + }); + } + } } function wireAboutDialog() { @@ -2940,6 +2986,12 @@ async function exportLogs(btn) { } catch (e) { console.warn("[settings] log export failed:", e); showError("Could not export the logs.", null, { retry: false }); + notifyFailure({ + kind: "export", + message: "Could not export the logs.", + detail: String(e?.message || e), + context: { stage: "Exporting logs" }, + }); } finally { if (btn) { btn.disabled = false; btn.textContent = "Export logs"; } } diff --git a/static/js/job.js b/static/js/job.js index 5f491a5..fc38dc5 100644 --- a/static/js/job.js +++ b/static/js/job.js @@ -7,6 +7,7 @@ import { selectedStems, } from "./state.js"; import { destroyPlayer, wireUpAudio, setWaveformLoading, updateFooterTrack } from "./player.js"; +import { notifyFailure } from "./notifications.js"; import { stagePhrases } from "./phrases.js"; import { addTrackToLibrary, setCurrentTrack, updateTrackStatus, applyStemPresenceCards } from "./catalog.js"; import { initSections } from "./sections.js"; @@ -134,9 +135,12 @@ function clearImportError() { // the user loads a different track, without wiping an import failure the user // has not read yet. Always retry:false -- "Try again" sends the user to the URL // field, which is not what a broken stem file calls for. -export function showPlaybackError(message, detail) { +export function showPlaybackError(message, detail, context = {}) { showError(message, detail, { retry: false }); errorEl.dataset.kind = "playback"; + // The class of failure #359 was written about: a track that loads and then + // does nothing. Recording it is what makes it reportable. + notifyFailure({ kind: "playback", message, detail, context }); } export function clearPlaybackError() { @@ -320,6 +324,20 @@ function applyState(state) { updateTrackStatus(state.job_id, "error"); setWaveformLoading(false); showError(state.error || "Unknown error", state.error_detail); + // Also record it: the banner above is transient and the user may well + // dismiss it before deciding to report anything. + notifyFailure({ + kind: "import", + message: state.error || "Unknown error", + detail: state.error_detail || null, + context: { + jobId: state.job_id, + stage: state.stage, + device: state.compute_device, + gpuFallback: state.gpu_fallback, + timings: state.stage_timings ? JSON.stringify(state.stage_timings) : null, + }, + }); setForegroundJobId(null); } else if (state.status === "cancelled") { stopJobPolling(); diff --git a/static/js/main.js b/static/js/main.js index 5e56b55..72bdc67 100644 --- a/static/js/main.js +++ b/static/js/main.js @@ -9,7 +9,8 @@ import { wireTransportButtons } from "./transport.js"; import { wireBeatGridUi } from "./beatgridUi.js"; import { togglePlayPause, updateLoopRegionVisual, toggleMetronome } from "./transport.js"; import { wireStemListControls, wireMixerToolbar } from "./mixer.js"; -import { initCatalog } from "./catalog.js"; +import { initCatalog, collectDiagnostics } from "./catalog.js"; +import { initNotifications, notifyFailure } from "./notifications.js"; import { runStoreMigrationIfNeeded } from "./utils.js"; // ─── Stem choice toggles on the import page ─── @@ -115,6 +116,10 @@ wireAppShellControls(); await runStoreMigrationIfNeeded(); await stemSelectionReady; refreshStemChoiceVisuals(); + // Before initCatalog: it runs the update check, which can itself notify. + // collectDiagnostics is injected rather than imported by notifications.js, + // which would make the two modules import each other. + await initNotifications({ diagnostics: collectDiagnostics }); await initCatalog(); })().catch(console.error); @@ -249,7 +254,14 @@ function wireFooterControls() { .catch((err) => { // A cancelled dialog resolves false without ever entering the busy // state, so anything here is a real failure. - showError(typeof err === "string" && err ? err : "Export failed.", null, { retry: false }); + const message = typeof err === "string" && err ? err : "Export failed."; + showError(message, null, { retry: false }); + notifyFailure({ + kind: "export", + message, + detail: err instanceof Error ? String(err.message) : null, + context: { stage: `Exporting ${format}` }, + }); }) .finally(() => { window.clearTimeout(backstop); diff --git a/static/js/notifications.js b/static/js/notifications.js new file mode 100644 index 0000000..02817b4 --- /dev/null +++ b/static/js/notifications.js @@ -0,0 +1,389 @@ +// ─── Notification centre ─── +// +// Until now the bell held exactly one hardcoded card ("New release available") +// and the badge/empty-state were toggled inline at its two call sites. This +// module owns the panel instead: any number of cards, persisted across +// reloads, each one openable into a dialog that can hand the failure to GitHub +// as a pre-filled bug report. +// +// Why persist: a failure used to live in a transient #error banner. Dismiss it +// (or reload) and the evidence was gone -- which is exactly the position issue +// #359 complained about, where a reporter has nothing to paste and guesses at +// a cause instead. A failure the user scrolled past is still reportable here. + +import { storeGet, storeSet } from "./utils.js"; + +const FAILURES_KEY = "stemdeck:failures"; +// Enough to cover a bad session without letting a crash loop fill the store. +const MAX_FAILURES = 20; +const NEW_ISSUE_URL = "https://github.com/stemdeckapp/stemdeck/issues/new"; +// Practical ceiling for a URL handed to a browser or, on Windows, to +// explorer.exe. GitHub itself tolerates more, but nothing here is worth +// risking a silently truncated link for -- the tail is trimmed to fit. +const MAX_URL_LENGTH = 6000; + +// What each failure class is called in the report and on the card. +const KIND_LABELS = { + import: "Import failed", + playback: "Playback failed", + export: "Export failed", + update: "Update check failed", +}; + +let failures = []; +// Set by catalog.js when an update is pending; owned here so the badge and the +// empty state have a single source of truth. +let releasePending = false; +// Injected at init so this module never imports catalog.js (which imports it). +let getDiagnostics = async () => ({}); + +// ─── Report URL ─────────────────────────────────────────────────────────── +// +// The bug form is a GitHub issue form, so its field ids can be prefilled from +// the query string. `preflight` is deliberately absent: checkboxes cannot be +// prefilled, so the user still confirms they are on the latest release and +// searched for duplicates. blank_issues_enabled is false in the repo config, +// so `template` is mandatory rather than decorative. + +/** The exact string from the form's OS dropdown, or prefill silently no-ops. */ +export function osOption(target) { + if (!target) return "Other"; + if (target.os === "macos") return target.arch === "arm64" ? "macOS (Apple Silicon)" : "macOS (Intel)"; + if (target.os === "windows") return "Windows"; + if (target.os === "linux") return "Linux"; + return "Other"; +} + +/** Likewise for "How did you install it?". No Tauri means it is served. */ +export function installOption(target, isDesktop) { + if (!isDesktop) return "Docker / self-hosted"; + if (target?.os === "macos") return "macOS DMG"; + if (target?.os === "windows") return "Windows ZIP"; + if (target?.os === "linux") return "Linux tar.gz"; + return "From source"; +} + +function technicalBlock(record, diag) { + const ctx = record.context || {}; + const rows = [ + ["StemDeck", diag.version ? `v${diag.version}` : "(unknown)"], + ["Failure", `${record.kind}${record.cause ? ` / ${record.cause}` : ""}`], + ["Stage", ctx.stage], + ["Device", ctx.device && ctx.gpuFallback ? `${ctx.device} (fell back to CPU)` : ctx.device], + ["Model", ctx.model || diag.model], + ["Engine", ctx.engine], + ["ffmpeg", diag.ffmpegConfigured === undefined ? undefined : String(diag.ffmpegConfigured)], + ["Timings", ctx.timings], + ]; + return rows + .filter(([, v]) => v !== undefined && v !== null && v !== "") + .map(([k, v]) => `${k}: ${v}`) + .join("\n"); +} + +/** + * Build the pre-filled bug-report URL. + * + * Pure and exported so the field mapping can be tested without a browser -- + * an OS string that does not match the dropdown option exactly is dropped by + * GitHub without complaint, which is the kind of bug only a test catches. + */ +export function buildReportUrl(record, diag = {}) { + const label = KIND_LABELS[record.kind] || "Something failed"; + const title = `[Bug]: ${label}${record.cause ? ` — ${record.cause}` : ""}`; + + const what = [ + `${label} in StemDeck.`, + "", + `**StemDeck said:** ${record.message || "(no message)"}`, + record.detail ? `**Detail:** ${record.detail}` : null, + "", + "", + ].filter((l) => l !== null).join("\n"); + + const steps = [ + "1. ", + record.context?.stage ? `2. StemDeck failed at: ${record.context.stage}` : "2. It failed.", + ].join("\n"); + + const tail = Array.isArray(record.tail) ? record.tail : []; + const build = (tailLines, note) => { + const parts = [technicalBlock(record, diag)]; + if (tailLines.length) parts.push("", "```", ...tailLines, "```"); + if (note) parts.push("", note); + const params = new URLSearchParams({ + template: "bug_report.yml", + title, + what, + steps, + os: osOption(diag.buildTarget), + version: diag.version ? `v${diag.version}` : "", + install: installOption(diag.buildTarget, Boolean(diag.isDesktop)), + extra: parts.join("\n"), + }); + return `${NEW_ISSUE_URL}?${params}`; + }; + + let url = build(tail, ""); + if (url.length <= MAX_URL_LENGTH) return url; + + // Too long: keep the end of the tail, which is where the actual error is, + // and point at the full logs rather than silently losing them. + const note = "_Tail truncated — full logs via Settings → Export logs._"; + for (let keep = Math.min(tail.length, 20); keep > 0; keep--) { + url = build(tail.slice(-keep), note); + if (url.length <= MAX_URL_LENGTH) return url; + } + return build([], note); +} + +// ─── Records ────────────────────────────────────────────────────────────── + +function nowId() { + return `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`; +} + +// classify_failure() returns the literal string "unknown" when it cannot place +// a failure. That is a backend sentinel, not a description: shown to a user it +// reads as "Import failed — unknown", and in an issue title it groups every +// unclassified failure under one meaningless heading. Drop it and let the real +// message speak instead. +function cleanCause(value) { + const text = String(value ?? "").trim(); + return text && text.toLowerCase() !== "unknown" ? text : null; +} + +async function persist() { + await storeSet(FAILURES_KEY, failures); +} + +/** + * Record a failure and surface it in the notification centre. + * + * Called at real failure sites only -- deliberately not wired into showError, + * which also carries benign validation ("All stems are muted - nothing to + * export."). A bug report about that would waste everyone's time. + */ +export function notifyFailure({ kind, message, detail = null, cause = null, context = {} } = {}) { + const record = { + id: nowId(), + at: Date.now(), + kind: kind in KIND_LABELS ? kind : "import", + message: String(message || "Something went wrong."), + detail: cleanCause(detail) ? String(detail) : null, + // The backend's classified cause (out-of-memory / disk-full / bad-input / + // unsupported-device) when error_detail carries one -- it leads the issue + // title, so duplicates group by cause rather than by wording. + cause: cleanCause(cause) || (detail ? cleanCause(String(detail).split("—")[0]) : null), + context, + }; + + // One failure, one card. A failed job is reported by whichever path noticed + // it -- the foreground SSE handler, the background queue reconciler, or both + // -- and applyState can run the error branch on more than one frame. Key on + // the job id where there is one, so the later sighting refreshes the record + // instead of stacking a duplicate the user has to dismiss twice. + const dupe = failures.findIndex((f) => + f.kind === record.kind && + (record.context?.jobId + ? f.context?.jobId === record.context.jobId + : f.message === record.message && record.at - f.at < 5000), + ); + if (dupe !== -1) { + // Keep whichever sighting carried the richer detail. + const prev = failures[dupe]; + record.id = prev.id; + record.detail = record.detail || prev.detail; + record.cause = record.cause || prev.cause; + record.context = { ...prev.context, ...record.context }; + record.tail = record.tail || prev.tail; + failures.splice(dupe, 1); + } + + failures.unshift(record); + failures = failures.slice(0, MAX_FAILURES); + persist(); + render(); + return record; +} + +export function dismissFailure(id) { + failures = failures.filter((f) => f.id !== id); + persist(); + render(); +} + +export function clearFailures() { + failures = []; + persist(); + render(); +} + +/** catalog.js tells us whether an update card is showing, for badge/empty. */ +export function setReleasePending(pending) { + releasePending = Boolean(pending); + render(); +} + +// ─── Rendering ──────────────────────────────────────────────────────────── + +function relativeTime(ts) { + const secs = Math.max(0, Math.round((Date.now() - ts) / 1000)); + if (secs < 60) return "just now"; + const mins = Math.round(secs / 60); + if (mins < 60) return `${mins} min ago`; + const hours = Math.round(mins / 60); + if (hours < 24) return `${hours} h ago`; + return new Date(ts).toLocaleDateString(); +} + +function iconSvg() { + const ns = "http://www.w3.org/2000/svg"; + const svg = document.createElementNS(ns, "svg"); + svg.setAttribute("width", "14"); + svg.setAttribute("height", "14"); + svg.setAttribute("viewBox", "0 0 24 24"); + svg.setAttribute("fill", "none"); + svg.setAttribute("stroke", "currentColor"); + svg.setAttribute("stroke-width", "1.9"); + const path = document.createElementNS(ns, "path"); + path.setAttribute("d", "M12 9v4 M12 17h.01 M10.3 3.9 1.8 18a2 2 0 0 0 1.7 3h17a2 2 0 0 0 1.7-3L13.7 3.9a2 2 0 0 0-3.4 0z"); + svg.appendChild(path); + return svg; +} + +function buildCard(record) { + const card = document.createElement("div"); + card.className = "daw-notif-card daw-notif-error"; + card.dataset.failureId = record.id; + card.setAttribute("role", "button"); + card.setAttribute("tabindex", "0"); + + const icon = document.createElement("div"); + icon.className = "daw-notif-card-icon"; + icon.setAttribute("aria-hidden", "true"); + icon.appendChild(iconSvg()); + + const body = document.createElement("div"); + body.className = "daw-notif-card-body"; + const title = document.createElement("div"); + title.className = "daw-notif-card-title"; + title.textContent = KIND_LABELS[record.kind] || "Something failed"; + const desc = document.createElement("div"); + desc.className = "daw-notif-card-desc"; + desc.textContent = `${record.cause || record.message} · ${relativeTime(record.at)}`; + body.append(title, desc); + + const dismiss = document.createElement("button"); + dismiss.className = "daw-notif-dismiss"; + dismiss.type = "button"; + dismiss.setAttribute("aria-label", "Dismiss notification"); + dismiss.textContent = "×"; + dismiss.addEventListener("click", (e) => { + e.stopPropagation(); + dismissFailure(record.id); + }); + + const open = () => openFailureDialog(record); + card.addEventListener("click", open); + card.addEventListener("keydown", (e) => { + if (e.code === "Enter" || e.code === "Space") { e.preventDefault(); open(); } + }); + + card.append(icon, body, dismiss); + return card; +} + +/** The one place the badge and empty state are decided, for every card kind. */ +export function render() { + const list = document.getElementById("notifList"); + const badge = document.getElementById("notifBadge"); + const empty = document.getElementById("notifEmpty"); + if (!list) return; + + for (const el of list.querySelectorAll(".daw-notif-error")) el.remove(); + const anchor = list.firstChild; + for (const record of failures) list.insertBefore(buildCard(record), anchor); + + const any = failures.length > 0 || releasePending; + badge?.classList.toggle("hidden", !any); + empty?.classList.toggle("hidden", any); +} + +// ─── Detail dialog ──────────────────────────────────────────────────────── + +async function openFailureDialog(record) { + const dialog = document.getElementById("failureDialog"); + if (!dialog) return; + const titleEl = document.getElementById("failureTitle"); + const whenEl = document.getElementById("failureWhen"); + const msgEl = document.getElementById("failureMessage"); + const techEl = document.getElementById("failureTech"); + const reportEl = document.getElementById("failureReport"); + + if (titleEl) titleEl.textContent = KIND_LABELS[record.kind] || "Something failed"; + if (whenEl) whenEl.textContent = new Date(record.at).toLocaleString(); + if (msgEl) msgEl.textContent = record.detail ? `${record.message}\n${record.detail}` : record.message; + if (techEl) techEl.textContent = "Collecting details…"; + if (reportEl) reportEl.removeAttribute("href"); + + dialog.classList.remove("hidden"); + + // The stderr tail lives in the quarantined error.txt and is fetched now + // rather than at capture time -- one request when a user actually looks, + // instead of one per failure whether or not they care. + if (!record.tail && record.context?.jobId) { + try { + const res = await fetch(`/api/jobs/${record.context.jobId}/failure`); + if (res.ok) { + const data = await res.json(); + record.tail = Array.isArray(data.tail) ? data.tail : []; + record.cause = record.cause || cleanCause(data.cause); + record.context = { + ...record.context, + stage: record.context.stage || data.stage, + device: record.context.device || data.device, + model: data.model, + timings: record.context.timings || data.timings, + }; + persist(); + } else { + record.tail = []; + } + } catch (e) { + console.warn("[notifications] failure detail fetch failed:", e); + record.tail = []; + } + } + + const diag = await getDiagnostics(); + if (techEl) { + const tail = record.tail?.length ? `\n\n${record.tail.join("\n")}` : ""; + techEl.textContent = `${technicalBlock(record, diag)}${tail}`; + } + // Set at open time; the global external-link handler reads href on click and + // routes it through Tauri's open_url on desktop, a new tab in a browser. + if (reportEl) reportEl.href = buildReportUrl(record, diag); +} + +function wireFailureDialog() { + const dialog = document.getElementById("failureDialog"); + const close = document.getElementById("failureClose"); + if (!dialog) return; + const hide = () => dialog.classList.add("hidden"); + close?.addEventListener("click", hide); + dialog.addEventListener("mousedown", (e) => { if (e.target === dialog) hide(); }); + dialog.addEventListener("keydown", (e) => { if (e.code === "Escape") hide(); }); + document.getElementById("notifClearAll")?.addEventListener("click", (e) => { + e.stopPropagation(); + clearFailures(); + }); +} + +export async function initNotifications({ diagnostics } = {}) { + if (typeof diagnostics === "function") getDiagnostics = diagnostics; + wireFailureDialog(); + const stored = await storeGet(FAILURES_KEY, []); + failures = Array.isArray(stored) ? stored.slice(0, MAX_FAILURES) : []; + render(); +} diff --git a/static/js/player.js b/static/js/player.js index 8f3eac0..56c553f 100644 --- a/static/js/player.js +++ b/static/js/player.js @@ -1295,6 +1295,7 @@ export function wireUpAudio(jobId, stems, duration, thumbnail, mixUrl = null, ti showPlaybackError( reason || "This track's audio could not be loaded.", "Playback is disabled for this track. Other tracks are not affected.", + { jobId, engine: kind, stage: "Loading stems" }, ); return; } @@ -1400,7 +1401,8 @@ export function wireUpAudio(jobId, stems, duration, thumbnail, mixUrl = null, ti if (audioEngine === eng) setAudioEngine(null); showPlaybackError( "This track's audio could not be loaded.", - "Playback is disabled for this track. Other tracks are not affected.", + String(e?.message || e) || "Playback is disabled for this track.", + { jobId, engine: kind, stage: "Starting the audio engine" }, ); }); }; diff --git a/tests/e2e/report-failure.spec.mjs b/tests/e2e/report-failure.spec.mjs new file mode 100644 index 0000000..d24ca71 --- /dev/null +++ b/tests/e2e/report-failure.spec.mjs @@ -0,0 +1,107 @@ +// The notification centre and the "Report on GitHub" flow. +// +// The thing worth testing in a real browser (rather than in the unit test that +// covers the URL itself) is the desktop path: on Tauri the link never +// navigates -- a global handler intercepts it and hands the URL to the Rust +// open_url command. If that interception breaks, the report button does +// nothing at all in the shipped desktop app while still working perfectly in +// every browser a developer tests in. That is exactly how #335 hid. + +import { test, expect } from "@playwright/test"; +import { openStudio } from "./helpers.mjs"; + +/** Fail an export, which is the quickest real failure to provoke in the UI. */ +async function failAnExport(page) { + await page.locator("#t-export-btn").click(); + await page.locator("#t-export-mix").click(); + await page.evaluate(() => window.__e2e.choosePath()); + await page.evaluate(() => window.__e2e.failSave("disk full")); + await expect(page.locator("#error")).not.toHaveClass(/hidden/); +} + +const openBell = async (page) => { + await page.locator("#notifBtn").click(); + await expect(page.locator(".daw-notif-panel")).toBeVisible(); +}; + +test.describe("failure notifications", () => { + test("a failure lands in the notification centre", async ({ page }) => { + await openStudio(page, { tauri: true }); + await expect(page.locator("#notifBadge")).toHaveClass(/hidden/); + + await failAnExport(page); + await expect(page.locator("#notifBadge")).not.toHaveClass(/hidden/); + + await openBell(page); + await expect(page.locator(".daw-notif-error")).toHaveCount(1); + await expect(page.locator(".daw-notif-error")).toContainText("Export failed"); + await expect(page.locator("#notifEmpty")).toHaveClass(/hidden/); + }); + + test("one failure produces one card, not one per sighting", async ({ page }) => { + await openStudio(page, { tauri: true }); + await failAnExport(page); + await page.locator(".retry-btn").click(); + await failAnExport(page); + + await openBell(page); + // Same kind, same message, seconds apart: the second sighting refreshes the + // first rather than stacking a duplicate the user dismisses twice. + await expect(page.locator(".daw-notif-error")).toHaveCount(1); + }); + + test("the card opens a dialog whose report link is pre-filled", async ({ page }) => { + await openStudio(page, { tauri: true }); + await failAnExport(page); + await openBell(page); + await page.locator(".daw-notif-error").first().click(); + + await expect(page.locator("#failureDialog")).not.toHaveClass(/hidden/); + await expect(page.locator("#failureTitle")).toHaveText("Export failed"); + // The user can read every technical detail before sending anything. + await expect(page.locator("#failureTech")).toContainText("StemDeck:"); + + const href = await page.locator("#failureReport").getAttribute("href"); + const url = new URL(href); + // blank_issues_enabled is false in the repo config: without a template the + // link lands on a chooser instead of a pre-filled form. + expect(url.searchParams.get("template")).toBe("bug_report.yml"); + expect(url.searchParams.get("title")).toContain("Export failed"); + expect(url.searchParams.get("version")).toMatch(/^v/); + expect(url.searchParams.get("what")).toContain("Export failed"); + }); + + test("desktop hands the report URL to the OS, never to the app window", async ({ page }) => { + await openStudio(page, { tauri: true }); + await failAnExport(page); + await openBell(page); + await page.locator(".daw-notif-error").first().click(); + await expect(page.locator("#failureDialog")).not.toHaveClass(/hidden/); + + await page.locator("#failureReport").click(); + + const calls = await page.evaluate(() => window.__e2e.callsFor("open_url")); + expect(calls).toHaveLength(1); + expect(calls[0].args.url).toContain("template=bug_report.yml"); + // The Rust side rejects anything that is not http(s). + expect(calls[0].args.url.startsWith("https://github.com/")).toBe(true); + // A navigation would have replaced the studio with GitHub. + expect(page.url()).not.toContain("github.com"); + }); + + test("a failure survives a reload, and dismissal clears the badge", async ({ page }) => { + await openStudio(page, { tauri: true }); + await failAnExport(page); + + await page.reload({ waitUntil: "domcontentloaded" }); + await expect(page.locator("#notifBadge")).not.toHaveClass(/hidden/); + await openBell(page); + // The banner is gone after a reload; the notification is the durable copy. + await expect(page.locator(".daw-notif-error")).toHaveCount(1); + + await page.locator(".daw-notif-error .daw-notif-dismiss").first().click(); + await expect(page.locator(".daw-notif-error")).toHaveCount(0); + await expect(page.locator("#notifBadge")).toHaveClass(/hidden/); + await expect(page.locator("#notifEmpty")).not.toHaveClass(/hidden/); + }); +}); diff --git a/tests/js/report-url.test.mjs b/tests/js/report-url.test.mjs new file mode 100644 index 0000000..ca98824 --- /dev/null +++ b/tests/js/report-url.test.mjs @@ -0,0 +1,152 @@ +// The pre-filled bug-report URL (notification centre → "Report on GitHub"). +// +// Two classes of bug live here and neither is visible by eye: +// +// 1. GitHub issue-form dropdowns prefill only on an EXACT match with an option +// string. "macOS" instead of "macOS (Apple Silicon)" is silently dropped -- +// the form opens with an empty OS field and nobody notices until a reporter +// leaves it blank. These strings must track .github/ISSUE_TEMPLATE/ +// bug_report.yml. +// 2. The report must never carry the track title or source URL. That is a +// product decision (issues are public), and a regression would leak it +// quietly, once per report, forever. +// +// Run: node tests/js/report-url.test.mjs + +import { buildReportUrl, osOption, installOption } from "../../static/js/notifications.js"; + +let pass = 0, + fail = 0; +const check = (name, cond, detail = "") => { + if (cond) { + pass++; + console.log(`PASS ${name}`); + } else { + fail++; + console.log(`FAIL ${name}${detail ? " -- " + detail : ""}`); + } +}; + +const params = (url) => new URL(url).searchParams; + +// The exact option strings from bug_report.yml. If the template changes, this +// list changes with it -- that is the point. +const OS_OPTIONS = ["macOS (Apple Silicon)", "macOS (Intel)", "Windows", "Linux", "Other"]; +const INSTALL_OPTIONS = ["macOS DMG", "Windows ZIP", "Linux tar.gz", "From source", "Docker / self-hosted"]; + +// ─── dropdown mapping ─── + +for (const [target, expected] of [ + [{ os: "macos", arch: "arm64" }, "macOS (Apple Silicon)"], + [{ os: "macos", arch: "x64" }, "macOS (Intel)"], + [{ os: "windows", arch: "x64" }, "Windows"], + [{ os: "linux", arch: "x64" }, "Linux"], + [{ os: "freebsd", arch: "x64" }, "Other"], + [null, "Other"], +]) { + const got = osOption(target); + check(`os: ${JSON.stringify(target)} -> ${expected}`, got === expected, got); + check(`os option is one the form offers: ${got}`, OS_OPTIONS.includes(got)); +} + +for (const [target, desktop, expected] of [ + [{ os: "macos", arch: "arm64" }, true, "macOS DMG"], + [{ os: "windows", arch: "x64" }, true, "Windows ZIP"], + [{ os: "linux", arch: "x64" }, true, "Linux tar.gz"], + [{ os: "freebsd", arch: "x64" }, true, "From source"], + // No Tauri means the UI is served by a backend the user runs themselves. + [{ os: "linux", arch: "x64" }, false, "Docker / self-hosted"], +]) { + const got = installOption(target, desktop); + check(`install: ${target.os}/${desktop ? "desktop" : "served"} -> ${expected}`, got === expected, got); + check(`install option is one the form offers: ${got}`, INSTALL_OPTIONS.includes(got)); +} + +// ─── a realistic report ─── + +const RECORD = { + kind: "import", + message: "Audio processing failed. Please try another video.", + detail: "out-of-memory — torch.OutOfMemoryError: CUDA out of memory.", + cause: "out-of-memory", + context: { + jobId: "abcdefabcdef", + stage: "Error: Processing failed", + device: "cuda", + gpuFallback: true, + timings: '{"download": 4.2, "separate": 61.0}', + }, + tail: ["torch.OutOfMemoryError: CUDA out of memory.", "Tried to allocate 2.40 GiB"], +}; + +const DIAG = { + version: "0.9.1", + model: "htdemucs_6s", + ffmpegConfigured: true, + buildTarget: { os: "windows", arch: "x64", gpu: "nvidia" }, + isDesktop: true, +}; + +const url = buildReportUrl(RECORD, DIAG); +const q = params(url); + +check("targets the bug form", q.get("template") === "bug_report.yml"); +check( + "blank issues are disabled, so template must be present", + url.startsWith("https://github.com/stemdeckapp/stemdeck/issues/new?"), +); +check("title leads with the cause", q.get("title") === "[Bug]: Import failed — out-of-memory"); +check("version is prefixed with v", q.get("version") === "v0.9.1"); +check("os matches the dropdown", q.get("os") === "Windows"); +check("install matches the dropdown", q.get("install") === "Windows ZIP"); +check("what carries StemDeck's own message", q.get("what").includes("Audio processing failed")); +check("what carries the classified detail", q.get("what").includes("out-of-memory")); +check("steps names the stage it died at", q.get("steps").includes("Error: Processing failed")); + +const extra = q.get("extra"); +check("extra reports the device and the CPU fallback", extra.includes("cuda (fell back to CPU)")); +check("extra reports the model", extra.includes("htdemucs_6s")); +check("extra carries the stderr tail", extra.includes("Tried to allocate 2.40 GiB")); +check("tail is fenced so GitHub renders it as code", extra.includes("```")); + +// preflight is a checkboxes field: GitHub cannot prefill it, and we must not +// pretend otherwise -- the user ticking it is the "I searched for duplicates" +// promise. +check("preflight is not faked", q.get("preflight") === null); + +// ─── privacy ─── + +const PRIVATE = { + ...RECORD, + context: { + ...RECORD.context, + title: "Someone's Private Demo Take 3", + sourceUrl: "https://www.youtube.com/watch?v=dQw4w9WgXcQ", + }, +}; +const privateUrl = buildReportUrl(PRIVATE, DIAG); +check("never carries a track title", !privateUrl.includes("Private%20Demo") && !privateUrl.includes("Private Demo")); +check("never carries a source URL", !privateUrl.includes("youtube.com") && !privateUrl.includes("dQw4w9WgXcQ")); + +// ─── length ceiling ─── + +const HUGE = { + ...RECORD, + tail: Array.from({ length: 400 }, (_, i) => `line ${i}: ${"x".repeat(120)}`), +}; +const hugeUrl = buildReportUrl(HUGE, DIAG); +check("stays inside the URL ceiling", hugeUrl.length <= 6000, `${hugeUrl.length} chars`); +const hugeExtra = params(hugeUrl).get("extra"); +check("truncation keeps the END of the tail, where the error is", hugeExtra.includes("line 399")); +check("truncation says so and points at the logs", hugeExtra.includes("Export logs")); +check("truncation does not eat the report body", params(hugeUrl).get("what").includes("Audio processing failed")); + +// ─── degenerate input ─── + +const bare = buildReportUrl({ kind: "playback", message: "This track's audio could not be loaded." }, {}); +check("survives no diagnostics at all", bare.includes("template=bug_report.yml")); +check("unknown OS falls back to Other", params(bare).get("os") === "Other"); +check("no version is empty, not 'vundefined'", params(bare).get("version") === ""); + +console.log(`\n${pass}/${pass + fail} checks passed`); +process.exit(fail ? 1 : 0); diff --git a/tests/test_failure_api.py b/tests/test_failure_api.py new file mode 100644 index 0000000..b986cb5 --- /dev/null +++ b/tests/test_failure_api.py @@ -0,0 +1,100 @@ +"""GET /api/jobs/{id}/failure — the quarantined evidence, minus the private bits. + +The pipeline has always written jobs/failed//error.txt on a failure (#277) +and nothing ever read it back, so a bug report could carry the classified cause +and one truncated stderr line at most. These tests pin the two things that make +the endpoint safe to feed into a public GitHub issue: it serves the technical +keys and the stderr tail, and it never serves the track title or source URL. +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +JOB = "abcdefabcdef" + +TITLE = "Someone's Private Demo Take 3" +SOURCE = "https://www.youtube.com/watch?v=dQw4w9WgXcQ" + +ERROR_TXT = f"""time: 2026-08-16T18:00:00+00:00 +job: {JOB} +title: {TITLE} +source: {SOURCE} +stage: Error: Processing failed +device: cuda, then cpu +model: htdemucs_6s +cause: out-of-memory +timings: {{"download": 4.2, "separate": 61.0}} +exception: SeparationError('demucs failed: exit status 1') + +--- stderr tail --- +torch.OutOfMemoryError: CUDA out of memory. +Tried to allocate 2.40 GiB +""" + + +@pytest.fixture +def client(tmp_path, monkeypatch): + from app.api import jobs as jobs_mod + + monkeypatch.setattr(jobs_mod, "JOBS_DIR", tmp_path) + from app.main import app + + return TestClient(app) + + +@pytest.fixture +def quarantined(tmp_path): + d = tmp_path / "failed" / JOB + d.mkdir(parents=True) + (d / "error.txt").write_text(ERROR_TXT, encoding="utf-8") + return d + + +def test_serves_the_technical_fields(client, quarantined): + r = client.get(f"/api/jobs/{JOB}/failure") + assert r.status_code == 200 + body = r.json() + assert body["job_id"] == JOB + assert body["cause"] == "out-of-memory" + assert body["device"] == "cuda, then cpu" + assert body["model"] == "htdemucs_6s" + # "stage: Error: Processing failed" must keep everything after the first + # colon, or the most useful field arrives as a bare "Error". + assert body["stage"] == "Error: Processing failed" + assert "SeparationError" in body["exception"] + + +def test_serves_the_stderr_tail(client, quarantined): + body = client.get(f"/api/jobs/{JOB}/failure").json() + assert body["tail"] == [ + "torch.OutOfMemoryError: CUDA out of memory.", + "Tried to allocate 2.40 GiB", + ] + + +def test_never_serves_the_title_or_source_url(client, quarantined): + """The whole point of parsing error.txt instead of serving it: these issues + are public, and what the user was working on is theirs to disclose.""" + r = client.get(f"/api/jobs/{JOB}/failure") + assert TITLE not in r.text + assert SOURCE not in r.text + body = r.json() + assert "title" not in body + assert "source" not in body + + +def test_404_when_the_job_never_failed(client, tmp_path): + assert client.get(f"/api/jobs/{JOB}/failure").status_code == 404 + + +def test_404_for_a_malformed_job_id(client): + assert client.get("/api/jobs/not-a-job-id/failure").status_code == 404 + + +def test_traversal_out_of_the_quarantine_is_refused(client): + """The id pattern already rejects separators; this pins it at the route so a + future loosening of JOB_ID_RE cannot turn this into an arbitrary file read.""" + for evil in ("../../etc/passwd", "..%2f..%2fsecret", "failed"): + assert client.get(f"/api/jobs/{evil}/failure").status_code == 404 From d04ddeb73eabda38d6ca8d21cf35c17481cc512d Mon Sep 17 00:00:00 2001 From: Thales <> Date: Sun, 16 Aug 2026 20:55:29 +0100 Subject: [PATCH 2/4] =?UTF-8?q?fix(settings):=20registry=20pane=20stuck=20?= =?UTF-8?q?on=20"Loading=E2=80=A6",=20and=20add=20the=20backend=20log=20vi?= =?UTF-8?q?ew?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Settings defects, both found by looking at the pane rather than the code. **Registry never loaded.** loadRegistryView selected `.settings-registry-view` unscoped, but the two log viewers reuse that class for its read-only-textarea styling and sit earlier in the markup. The lookup therefore returned the *application log* box: the registry JSON was written into a hidden textarea while the registry pane kept its literal "Loading…" placeholder for ever, and the application log showed registry JSON until it was refreshed. Scope the lookup to the registry pane. Not web-only -- it never worked anywhere. **backend.log had no viewer.** It was listed under Logs → Location and shipped in the logs zip, but the only two views were application and setup, so the one log that holds what killed a backend before its own logging was configured was the one log you could not read in the app. It gets a "Backend log" tab beside the other two, reading backend.log plus its two rotations. The sub-tab wiring is already generic (loadLogTail(overlay, name)), so the tab needed markup and a view entry, no new JS. Tests: the backend view's window filtering and rotation ordering, plus one that walks _LOG_FILES against _LOG_VIEWS and fails if a file the Settings pane advertises has no view to read it in -- which is exactly how backend.log stayed invisible. --- app/main.py | 6 ++++++ static/js/catalog.js | 18 +++++++++++++++++- tests/test_logs_api.py | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/app/main.py b/app/main.py index 38f4d5d..7e35b49 100644 --- a/app/main.py +++ b/app/main.py @@ -571,6 +571,12 @@ def get_logs_info() -> dict[str, object]: # otherwise make a busy log look empty. _LOG_VIEWS: dict[str, tuple[str, ...]] = { "application": ("stemdeck.log", "stemdeck.log.1"), + # The backend's raw stdout/stderr. Worth a view of its own because it holds + # what the application log cannot: anything the process printed before + # logging was configured, and anything that killed it before a handler ran. + # A backend that dies at startup leaves stemdeck.log empty and the answer + # here. + "backend": ("backend.log", "backend.log.1", "backend.log.2"), "setup": ("setup.log",), } # Bounds on what a single view returns. The application log rotates at 5 MB, so diff --git a/static/js/catalog.js b/static/js/catalog.js index 26d86cc..26f64ba 100644 --- a/static/js/catalog.js +++ b/static/js/catalog.js @@ -2890,7 +2890,12 @@ async function wireNetworkSetting(overlay) { } async function loadRegistryView(overlay) { - const view = overlay.querySelector(".settings-registry-view"); + // Scoped to the registry pane: the log viewers reuse .settings-registry-view + // for its read-only-textarea styling and sit earlier in the markup, so a bare + // class lookup returned the *application log* box. The registry JSON was + // being written into a hidden textarea while the registry pane sat on its + // literal "Loading…" placeholder for ever. + const view = overlay.querySelector('[data-pane="registry"] .settings-registry-view'); if (!view) return; view.value = "Loading…"; try { @@ -3227,6 +3232,7 @@ function openLibraryEditor() {
+
@@ -3250,6 +3256,16 @@ function openLibraryEditor() {
+