From 5de4b40c1d007c05c4c31d00a263b19d7f4f4e5f Mon Sep 17 00:00:00 2001 From: debont80 Date: Sun, 5 Jul 2026 14:39:29 -0400 Subject: [PATCH] fix(download): name the failing step in postprocessing errors yt-dlp collapses an ffmpeg postprocessing failure to a bare "Postprocessing: Conversion failed!" with no indication of which step died, and without --verbose the underlying ffmpeg reason is never emitted at all. describeDownloadError() appends the last non-noise trailing lines (e.g. the [ModifyChapters] SponsorBlock re-encode) so the error log and completion notification point at the actual cause instead of a dead end. --- src/main/download.ts | 4 +-- src/main/log.ts | 41 +++++++++++++++++++++++ test/log.test.ts | 78 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 test/log.test.ts diff --git a/src/main/download.ts b/src/main/download.ts index 688e3d1..360fb1c 100644 --- a/src/main/download.ts +++ b/src/main/download.ts @@ -33,7 +33,7 @@ import { FILEPATH_MARKER, DEST_MARKER } from './core/buildArgs' -import { cleanError } from './log' +import { describeDownloadError } from './log' import { addErrorLog } from './errorlog' import { STALL_TIMEOUT_MS, @@ -530,7 +530,7 @@ function wireChildProcess(params: { opts.incognito ) } else { - const msg = cleanError(stderrTail) || `yt-dlp exited with code ${code}` + const msg = describeDownloadError(stderrTail) || `yt-dlp exited with code ${code}` send(wc, { type: 'error', id: opts.id, error: msg }) logFailure(opts, getTitle(), msg) notify( diff --git a/src/main/log.ts b/src/main/log.ts index ea8cf84..bcda965 100644 --- a/src/main/log.ts +++ b/src/main/log.ts @@ -12,3 +12,44 @@ export function cleanError(stderr: string): string { const errLine = [...lines].reverse().find((l) => /^error/i.test(l)) return (errLine ?? lines[lines.length - 1] ?? '').replace(/^error:\s*/i, '').trim() } + +// Lines that carry no diagnostic value as post-processing context: per-tick +// download progress and the "Deleting original file" cleanup bookkeeping yt-dlp +// prints between merge steps. +const CONTEXT_NOISE = /^\[download\]|^Deleting original file/i +// A single context line, capped so a long output path can't bloat the message. +const CONTEXT_LINE_MAX = 200 + +/** + * Richer error for the DOWNLOAD failure path (probe/indexer/updater keep the + * concise cleanError line). + * + * yt-dlp collapses an ffmpeg post-processing failure to a bare + * "ERROR: Postprocessing: Conversion failed!" that names neither the step that + * died nor the reason — and without `--verbose` the underlying ffmpeg detail is + * never emitted at all. What yt-dlp DOES print is a tagged progress line per + * post-processor ([Merger], [ModifyChapters], [ExtractAudio], …; its phase tags + * like [download]/[info] are lowercase). So for that generic wrapper we append + * the last couple of non-noise trailing lines, which name the failing step (e.g. + * the SponsorBlock re-encode) and surface any ffmpeg diagnostic when one is + * present. Self-explanatory errors (a private video, a 403) are returned + * unchanged. + */ +export function describeDownloadError(stderr: string): string { + const headline = cleanError(stderr) + if (!headline || !/conversion failed|postprocessing/i.test(headline)) return headline + const lines = stderr + .split('\n') + .map((l) => l.trim()) + .filter(Boolean) + const context: string[] = [] + for (const line of [...lines].reverse()) { + if (context.length >= 2) break + if (CONTEXT_NOISE.test(line) || /^error/i.test(line)) continue + // Skip anything already conveyed by the headline (avoids echoing it back). + if (headline.includes(line) || line.includes(headline)) continue + const capped = line.length > CONTEXT_LINE_MAX ? `${line.slice(0, CONTEXT_LINE_MAX - 1)}…` : line + if (!context.includes(capped)) context.unshift(capped) + } + return context.length > 0 ? `${headline} — ${context.join(' · ')}` : headline +} diff --git a/test/log.test.ts b/test/log.test.ts new file mode 100644 index 0000000..480d029 --- /dev/null +++ b/test/log.test.ts @@ -0,0 +1,78 @@ +import { describe, it, expect } from 'vitest' +import { cleanError, describeDownloadError } from '../src/main/log' + +describe('cleanError', () => { + it('returns the last ERROR line, stripped of its prefix', () => { + const stderr = [ + '[youtube] gHiZA4O5PQM: Downloading webpage', + 'ERROR: [youtube] gHiZA4O5PQM: Video unavailable. This video is private' + ].join('\n') + expect(cleanError(stderr)).toBe( + '[youtube] gHiZA4O5PQM: Video unavailable. This video is private' + ) + }) + + it('falls back to the last line when nothing matches /error/', () => { + expect(cleanError('one\ntwo\nthree')).toBe('three') + }) + + it('returns empty string for blank stderr', () => { + expect(cleanError('')).toBe('') + }) +}) + +describe('describeDownloadError', () => { + // A real non-verbose SponsorBlock-remove failure: yt-dlp emits only the + // [ModifyChapters] step lines and the bare wrapper error. + const modifyChaptersFailure = [ + '[info] qkUTB65OfAc: Downloading 1 format(s): 137+251', + '[SponsorBlock] Found 1 segments in the SponsorBlock database', + '[Merger] Merging formats into "video.mp4"', + 'Deleting original file video.f251.webm (pass -k to keep)', + 'Deleting original file video.f137.mp4 (pass -k to keep)', + '[ModifyChapters] Re-encoding "video.mp4" with appropriate keyframes', + '[ModifyChapters] Removing chapters from video.mp4', + 'ERROR: Postprocessing: Conversion failed!' + ].join('\n') + + it('names the failing post-processing step for the bare "Conversion failed!" wrapper', () => { + const msg = describeDownloadError(modifyChaptersFailure) + expect(msg.startsWith('Postprocessing: Conversion failed!')).toBe(true) + expect(msg).toContain('ModifyChapters') + }) + + it('skips [download] progress and "Deleting original file" bookkeeping as context', () => { + const msg = describeDownloadError(modifyChaptersFailure) + expect(msg).not.toContain('Deleting original file') + expect(msg).not.toMatch(/\[download\]/) + }) + + it('leaves a self-explanatory (non-postprocessing) error unchanged', () => { + const stderr = [ + '[youtube] gHiZA4O5PQM: Downloading webpage', + 'ERROR: [youtube] gHiZA4O5PQM: Video unavailable. This video is private' + ].join('\n') + expect(describeDownloadError(stderr)).toBe(cleanError(stderr)) + }) + + it('does not echo the headline back as its own context', () => { + const stderr = 'ERROR: Postprocessing: Conversion failed!' + expect(describeDownloadError(stderr)).toBe('Postprocessing: Conversion failed!') + }) + + it('caps an over-long context line', () => { + const longPath = 'x'.repeat(400) + const stderr = [ + `[ModifyChapters] Re-encoding "${longPath}" with appropriate keyframes`, + 'ERROR: Postprocessing: Conversion failed!' + ].join('\n') + const msg = describeDownloadError(stderr) + expect(msg).toContain('…') + // headline + separator + a single capped context line, nowhere near 400 chars. + expect(msg.length).toBeLessThan(300) + }) + + it('returns empty string for blank stderr (falls through to the caller fallback)', () => { + expect(describeDownloadError('')).toBe('') + }) +})