From 27cae28e125ccfd38ed9297e48b2c7f178246987 Mon Sep 17 00:00:00 2001 From: debont80 Date: Sun, 5 Jul 2026 14:56:09 -0400 Subject: [PATCH] feat(diagnostics): persist full stderr detail behind a "Show details" toggle describeDownloadError condenses a postprocessing failure to one line for the queue/notification, discarding the rest of yt-dlp's stderr. The persisted error log now also keeps the fuller non-noise transcript (stderrDetail) as an optional `detail` field, and the Diagnostics card gets a collapsible "Show details" expander per row so a failure like "Postprocessing: Conversion failed!" can be inspected -- and copied into a bug report -- without re-running the download from a terminal. --- src/main/core/validation.ts | 3 +- src/main/download.ts | 12 +++- src/main/log.ts | 17 ++++++ src/renderer/src/store/errorlog.ts | 17 ++++++ .../src/views/settings/DiagnosticsCard.tsx | 58 +++++++++++++++---- .../src/views/settings/settingsStyles.ts | 16 +++++ src/shared/ipc.ts | 7 +++ test/log.test.ts | 37 +++++++++++- test/validation.test.ts | 5 ++ 9 files changed, 155 insertions(+), 17 deletions(-) diff --git a/src/main/core/validation.ts b/src/main/core/validation.ts index 691aed9..fcfb599 100644 --- a/src/main/core/validation.ts +++ b/src/main/core/validation.ts @@ -74,7 +74,8 @@ export function isValidErrorLogEntry(o: unknown): o is ErrorLogEntry { (e.kind === 'video' || e.kind === 'audio') && typeof e.error === 'string' && typeof e.occurredAt === 'number' && - isOptionalString(e.title) + isOptionalString(e.title) && + isOptionalString(e.detail) ) } diff --git a/src/main/download.ts b/src/main/download.ts index 360fb1c..d26373d 100644 --- a/src/main/download.ts +++ b/src/main/download.ts @@ -33,7 +33,7 @@ import { FILEPATH_MARKER, DEST_MARKER } from './core/buildArgs' -import { describeDownloadError } from './log' +import { describeDownloadError, stderrDetail } from './log' import { addErrorLog } from './errorlog' import { STALL_TIMEOUT_MS, @@ -117,7 +117,12 @@ function notify(wc: WebContents, title: string, body: string, incognito = false) if (flashWin && !flashWin.isDestroyed() && !flashWin.isFocused()) flashWin.flashFrame(true) } -function logFailure(opts: StartDownloadOptions, title: string | undefined, error: string): void { +function logFailure( + opts: StartDownloadOptions, + title: string | undefined, + error: string, + detail?: string +): void { // L136: incognito downloads are never recorded — not even a failure entry, which // would persist the title + URL in the user-facing diagnostics log. if (opts.incognito) return @@ -127,6 +132,7 @@ function logFailure(opts: StartDownloadOptions, title: string | undefined, error url: opts.url, kind: opts.kind, error, + detail, occurredAt: Date.now() }) } @@ -532,7 +538,7 @@ function wireChildProcess(params: { } else { const msg = describeDownloadError(stderrTail) || `yt-dlp exited with code ${code}` send(wc, { type: 'error', id: opts.id, error: msg }) - logFailure(opts, getTitle(), msg) + logFailure(opts, getTitle(), msg, stderrDetail(stderrTail)) notify( wc, opts.incognito ? 'Download failed' : (getTitle() ?? 'Download failed'), diff --git a/src/main/log.ts b/src/main/log.ts index bcda965..c261b8b 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -53,3 +53,20 @@ export function describeDownloadError(stderr: string): string { } return context.length > 0 ? `${headline} — ${context.join(' · ')}` : headline } + +/** + * Full non-noise stderr, for the persisted error log's "Show details" expander + * (Diagnostics card) — everything describeDownloadError condenses to one line, + * kept in full (progress ticks and delete-bookkeeping still stripped) so a bug + * report carries the real yt-dlp/ffmpeg transcript instead of a paraphrase. + * Returns undefined when there's nothing beyond a single line — the headline + * the caller already shows — so a plain, self-explanatory error doesn't grow a + * details toggle with nothing new in it. + */ +export function stderrDetail(stderr: string): string | undefined { + const lines = stderr + .split('\n') + .map((l) => l.trim()) + .filter((l) => l && !CONTEXT_NOISE.test(l)) + return lines.length > 1 ? lines.join('\n') : undefined +} diff --git a/src/renderer/src/store/errorlog.ts b/src/renderer/src/store/errorlog.ts index 9e53799..842e6dd 100644 --- a/src/renderer/src/store/errorlog.ts +++ b/src/renderer/src/store/errorlog.ts @@ -13,6 +13,23 @@ const seed: ErrorLogEntry[] = PREVIEW kind: 'video', error: '[youtube] err0r: Private video. Sign in if you have access.', occurredAt: Date.now() - 1000 * 60 * 12 + }, + { + id: 'e2', + title: 'you’re about to get hacked!! (7 reasons why)', + url: 'https://youtube.com/watch?v=q2err0r', + kind: 'video', + error: + 'Postprocessing: Conversion failed! — [ModifyChapters] Re-encoding "video.mp4" with appropriate keyframes', + detail: [ + '[info] q2err0r: Downloading 1 format(s): 137+251', + '[SponsorBlock] Found 1 segments in the SponsorBlock database', + '[Merger] Merging formats into "video.mp4"', + '[ModifyChapters] Re-encoding "video.mp4" with appropriate keyframes', + '[ModifyChapters] Removing chapters from video.mp4', + 'ERROR: Postprocessing: Conversion failed!' + ].join('\n'), + occurredAt: Date.now() - 1000 * 60 * 5 } ] : [] diff --git a/src/renderer/src/views/settings/DiagnosticsCard.tsx b/src/renderer/src/views/settings/DiagnosticsCard.tsx index 0a852eb..743b3ea 100644 --- a/src/renderer/src/views/settings/DiagnosticsCard.tsx +++ b/src/renderer/src/views/settings/DiagnosticsCard.tsx @@ -1,6 +1,12 @@ import { useState } from 'react' import { Button, Card, Subtitle2, Caption1, Text } from '@fluentui/react-components' -import { BugRegular, CopyRegular, DeleteRegular } from '@fluentui/react-icons' +import { + BugRegular, + ChevronDownRegular, + ChevronRightRegular, + CopyRegular, + DeleteRegular +} from '@fluentui/react-icons' import { useErrorLog } from '../../store/errorlog' import { useErrorTextStyles } from '../../components/ui/errorText' import { EmptyState } from '../../components/ui/EmptyState' @@ -13,13 +19,25 @@ export function DiagnosticsCard(): React.JSX.Element { const clearErrorLog = useErrorLog((s) => s.clear) const [confirmClearLog, setConfirmClearLog] = useState(false) + // Which rows have their "Show details" transcript expanded; keyed by entry id. + const [expanded, setExpanded] = useState>(new Set()) + function toggleExpanded(id: string): void { + setExpanded((s) => { + const next = new Set(s) + if (next.has(id)) next.delete(id) + else next.add(id) + return next + }) + } function copyErrorReport(): void { // The button is disabled when there are no entries, so `report` is always // non-empty here -- no need for an unreachable empty-report fallback (L159). const report = errorEntries .map((e) => - [new Date(e.occurredAt).toLocaleString(), e.title ?? e.url, e.url, e.error].join('\n') + [new Date(e.occurredAt).toLocaleString(), e.title ?? e.url, e.url, e.error, e.detail] + .filter(Boolean) + .join('\n') ) .join('\n\n---\n\n') navigator.clipboard.writeText(report).catch(() => {}) @@ -78,17 +96,33 @@ export function DiagnosticsCard(): React.JSX.Element { ) : (
- {errorEntries.slice(0, 20).map((e) => ( -
-
- {e.title ?? e.url} - - {new Date(e.occurredAt).toLocaleString()} - + {errorEntries.slice(0, 20).map((e) => { + const isExpanded = expanded.has(e.id) + return ( +
+
+ {e.title ?? e.url} + + {new Date(e.occurredAt).toLocaleString()} + +
+ {e.error} + {e.detail && ( + <> + + {isExpanded && 
{e.detail}
} + + )}
- {e.error} -
- ))} + ) + })} {errorEntries.length > 20 && ( Showing 20 of {errorEntries.length} errors. )} diff --git a/src/renderer/src/views/settings/settingsStyles.ts b/src/renderer/src/views/settings/settingsStyles.ts index 0d94d8a..61465ce 100644 --- a/src/renderer/src/views/settings/settingsStyles.ts +++ b/src/renderer/src/views/settings/settingsStyles.ts @@ -113,5 +113,21 @@ export const useSettingsStyles = makeStyles({ overflow: 'hidden', textOverflow: 'ellipsis', whiteSpace: 'nowrap' + }, + // The Diagnostics card's "Show details" expander -- the fuller non-noise + // stderr transcript behind a failure's one-line headline. Scrollable and + // capped so a long SponsorBlock/ffmpeg transcript can't blow out the card. + errorDetail: { + fontFamily: tokens.fontFamilyMonospace, + fontSize: tokens.fontSizeBase200, + whiteSpace: 'pre-wrap', + wordBreak: 'break-word', + color: tokens.colorNeutralForeground3, + backgroundColor: tokens.colorNeutralBackground1, + marginTop: '6px', + padding: '8px', + maxHeight: '200px', + overflowY: 'auto', + ...shorthands.borderRadius(tokens.borderRadiusMedium) } }) diff --git a/src/shared/ipc.ts b/src/shared/ipc.ts index f5f2678..77b3ee6 100644 --- a/src/shared/ipc.ts +++ b/src/shared/ipc.ts @@ -845,6 +845,13 @@ export interface ErrorLogEntry { url: string kind: MediaKind error: string + /** + * Fuller non-noise stderr context behind the Diagnostics card's "Show + * details" toggle — undefined when `error` already says everything (most + * failures), present for the generic "Postprocessing: Conversion failed!" + * wrapper where the useful detail is the tagged step lines that preceded it. + */ + detail?: string /** epoch milliseconds */ occurredAt: number } diff --git a/test/log.test.ts b/test/log.test.ts index 480d029..29fbacc 100644 --- a/test/log.test.ts +++ b/test/log.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest' -import { cleanError, describeDownloadError } from '../src/main/log' +import { cleanError, describeDownloadError, stderrDetail } from '../src/main/log' describe('cleanError', () => { it('returns the last ERROR line, stripped of its prefix', () => { @@ -76,3 +76,38 @@ describe('describeDownloadError', () => { expect(describeDownloadError('')).toBe('') }) }) + +describe('stderrDetail', () => { + it('keeps the full non-noise transcript for the Diagnostics "Show details" expander', () => { + const stderr = [ + '[info] qkUTB65OfAc: Downloading 1 format(s): 137+251', + '[SponsorBlock] Found 1 segments in the SponsorBlock database', + '[ModifyChapters] Re-encoding "video.mp4" with appropriate keyframes', + 'ERROR: Postprocessing: Conversion failed!' + ].join('\n') + const detail = stderrDetail(stderr) + expect(detail).toContain('[SponsorBlock] Found 1 segments') + expect(detail).toContain('[ModifyChapters] Re-encoding') + expect(detail).toContain('ERROR: Postprocessing: Conversion failed!') + }) + + it('strips [download] progress ticks and "Deleting original file" bookkeeping', () => { + const stderr = [ + '[download] 42.0% of 23.21MiB at 15.11MiB/s ETA 00:00', + 'Deleting original file video.f251.webm (pass -k to keep)', + '[ModifyChapters] Re-encoding "video.mp4" with appropriate keyframes', + 'ERROR: Postprocessing: Conversion failed!' + ].join('\n') + const detail = stderrDetail(stderr) + expect(detail).not.toMatch(/\[download\]/) + expect(detail).not.toContain('Deleting original file') + }) + + it('returns undefined when there is nothing beyond a single headline', () => { + expect(stderrDetail('ERROR: Postprocessing: Conversion failed!')).toBeUndefined() + }) + + it('returns undefined for blank stderr', () => { + expect(stderrDetail('')).toBeUndefined() + }) +}) diff --git a/test/validation.test.ts b/test/validation.test.ts index fa80a4e..05d3c50 100644 --- a/test/validation.test.ts +++ b/test/validation.test.ts @@ -175,6 +175,11 @@ describe('isValidErrorLogEntry', () => { expect(isValidErrorLogEntry({ ...validError, title: 'optional' })).toBe(true) }) + it('accepts an optional detail string and rejects a non-string one', () => { + expect(isValidErrorLogEntry({ ...validError, detail: 'the full transcript' })).toBe(true) + expect(isValidErrorLogEntry({ ...validError, detail: 42 })).toBe(false) + }) + it('rejects a missing error message', () => { const { error: _error, ...noError } = validError expect(isValidErrorLogEntry(noError)).toBe(false)