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.
This commit is contained in:
2026-07-05 14:39:29 -04:00
parent 1be2708d15
commit 5de4b40c1d
3 changed files with 121 additions and 2 deletions
+2 -2
View File
@@ -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(
+41
View File
@@ -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
}
+78
View File
@@ -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('')
})
})