diff --git a/src/download-video.ts b/src/download-video.ts index 2204f82..021fd78 100644 --- a/src/download-video.ts +++ b/src/download-video.ts @@ -57,10 +57,10 @@ export class YtdlpError extends Error { // A signal kill keeps its message: the timeout is the real story there, // and any ERROR line in a killed process's output is a stale partial. const lines = signalled ? [] : errorLines(stderr); - const errLine = (lines.findLast((l) => !PHOTO_ITEM.test(l)) ?? lines.at(-1)) + const real = lines.filter((l) => !isPhotoItemError(l)); + const errLine = (real.at(-1) ?? lines.at(-1)) ?.slice('ERROR:'.length) - // De-noising is lossless: the raw line still streams to the chat - // verbatim, and classification reads raw stderr. + // De-noising is lossless: classification reads the raw stderr. .replace(/^\s*\[[^\]]+\]\s*/, '') // yt-dlp appends this after any "(caused by ...)", so it has to be // stripped first @@ -70,7 +70,11 @@ export class YtdlpError extends Error { ) .replace(/\s*\(caused by .*\)\s*$/, '') .trim(); - super(errLine || message); + // Every item failing as a photo is a verdict on the post, and the last id + // is no more the reason than its siblings. A lone line has no siblings: it + // is the extractor's own verdict on the post. + const allPhotos = !real.length && lines.length > 1; + super((allPhotos ? 'There is no video in this post' : errLine) || message); this.name = 'YtdlpError'; // hidden from console.error, which prints an Error's own properties whole: // a playlist's dump-json runs to megabytes (see STDERR_TAIL for the cap on @@ -91,6 +95,8 @@ const NOT_A_VIDEO_PATTERNS = [ // carousel is a playlist, whose photo items each fall through to yt-dlp's // generic no-formats error, a verdict on the item rather than the post. const PHOTO_ITEM = /\[instagram\] .+: no video formats found/i; +const isPhotoItemError = (line: string) => + line.startsWith('ERROR:') && PHOTO_ITEM.test(line); const PERMANENT_PATTERNS = [ /unable to extract/i, /no video formats found/i, @@ -113,7 +119,7 @@ export const classifyFailure = (e: unknown): FailureKind => { if (!all.length) return 'transient'; // one line is the whole post reporting no formats, a failure the chat hears // about - const rest = all.length > 1 ? all.filter((l) => !PHOTO_ITEM.test(l)) : all; + const rest = all.length > 1 ? all.filter((l) => !isPhotoItemError(l)) : all; if (!rest.length) return 'not-a-video'; if (rest.some((l) => NOT_A_VIDEO_PATTERNS.some((re) => re.test(l)))) return 'not-a-video'; @@ -401,15 +407,30 @@ const execYtdlp = limit( // streaming decoder, so a multi-byte char split across chunks isn't garbled // (which would both mis-render and could defeat the classifier). let stderr = ''; + let held = ''; let firstLine = true; const decoder = new TextDecoder(); + // A photo item's ERROR answers nothing the chat asked for. /verbose is a + // debugging request for yt-dlp's actual output, so it is exempt. + const show = (block: string) => { + const text = ( + verbose + ? block + : block + .split('\n') + .filter((l) => !isPhotoItemError(l)) + .join('\n') + ).trim(); + if (!text) return; + if (firstLine) { + // visually separate the streamed stderr from the progress above it + logMsg.append(''); + firstLine = false; + } + logMsg.append(`${Bun.escapeHTML(text)}`); + }; try { for await (const chunk of proc.stderr) { - if (firstLine) { - // visually separate the streamed stderr from the progress above it - logMsg.append(''); - firstLine = false; - } const text = decoder.decode(chunk, { stream: true }); stderr += text; if (stderr.length > 2 * STDERR_TAIL) { @@ -418,9 +439,23 @@ const execYtdlp = limit( const nl = stderr.indexOf('\n', stderr.length - STDERR_TAIL); stderr = nl === -1 ? stderr.slice(-STDERR_TAIL) : stderr.slice(nl + 1); } - logMsg.append(`${Bun.escapeHTML(text.trim())}`); + // the filter judges whole lines, so a line the chunk boundary cut waits + // for its other half + held += text; + const nl = held.lastIndexOf('\n'); + if (nl !== -1) { + show(held.slice(0, nl)); + held = held.slice(nl + 1); + } else if (held.length > STDERR_TAIL) { + // nothing this long is a photo item, and a stream that never sends a + // newline would otherwise hold all of it (stderr above is capped) + show(held); + held = ''; + } } - stderr += decoder.decode(); // flush any buffered trailing bytes + const tail = decoder.decode(); // flush any buffered trailing bytes + stderr += tail; + show(held + tail); await proc.exited; } finally { @@ -482,7 +517,7 @@ const onlyPhotoItemsFailed = (e: YtdlpError) => { const lines = errorLines(e.stderr); // no ERROR line at all means the run failed for a reason this stderr no // longer holds: the tail trim drops the oldest lines - return !!lines.length && lines.every((line) => PHOTO_ITEM.test(line)); + return !!lines.length && lines.every(isPhotoItemError); }; const parseEntries = (out: string): VideoInfo[] => diff --git a/test/bin/yt-dlp b/test/bin/yt-dlp index c688c89..e8d0c97 100755 --- a/test/bin/yt-dlp +++ b/test/bin/yt-dlp @@ -7,7 +7,21 @@ d=/tmp/stub [ "$STUB_BIN" = "1" ] && [ -d "$d" ] || exec /opt/yt-dlp/yt-dlp "$@" echo "$0 $*" >> "$d/args" while [ -f "$d/block" ]; do sleep 0.05; done -[ -f "$d/stderr" ] && cat "$d/stderr" >&2 +# stderr_split holds comma-separated byte offsets: deliver stderr as one write +# per segment, far enough apart that the reader gets each as its own chunk +if [ -f "$d/stderr" ]; then + if [ -f "$d/stderr_split" ]; then + prev=0 + for n in $(tr ',' ' ' < "$d/stderr_split"); do + tail -c "+$((prev + 1))" "$d/stderr" | head -c "$((n - prev))" >&2 + sleep 0.2 + prev=$n + done + tail -c "+$((prev + 1))" "$d/stderr" >&2 + else + cat "$d/stderr" >&2 + fi +fi # the real binary streams entries as it resolves them, so a kill lands on a # run that has already emitted some [ -f "$d/stdout" ] && cat "$d/stdout" diff --git a/test/download-video.test.ts b/test/download-video.test.ts index 71ae67f..6e09af3 100644 --- a/test/download-video.test.ts +++ b/test/download-video.test.ts @@ -542,6 +542,92 @@ describe('getInfo', () => { expect(infoCount()).toBe(0); }); + it("keeps a carousel's photo errors out of the chat entirely", async () => { + await stub({ + exit: '1', + stdout: JSON.stringify({ ...VideoInfo, id: 'v1', webpage_url: url }), + stderr: + 'ERROR: [Instagram] photo1: No video formats found!; please report this issue on https://github.com/yt-dlp/yt-dlp/issues?q=\n' + + 'ERROR: [Instagram] photo2: No video formats found!\n', + }); + + await getInfos(log as any, url); + + // not even the blank line that sets streamed stderr off from the progress + expect(mockAppend.mock.calls.map(([s]) => s)).toEqual([ + `🧐 Scraping ${url}...`, + ]); + }); + + it('streams a real error the photo errors are mixed in with', async () => { + await stub({ + exit: '1', + stderr: + 'ERROR: [Instagram] photo1: No video formats found!\n' + + 'ERROR: [Instagram] v2: Requested content is not available, rate-limit reached\n' + + 'ERROR: [Instagram] photo3: No video formats found!\n', + }); + + await expect(getInfos(log as any, url)).rejects.toBeInstanceOf(YtdlpError); + + expect(appendedText()).toContain('rate-limit reached'); + expect(appendedText()).not.toContain('photo1'); + expect(appendedText()).not.toContain('photo3'); + }); + + it('streams the photo errors under verbose, which asks for them', async () => { + await stub({ + exit: '1', + stderr: 'ERROR: [Instagram] photo1: No video formats found!\n', + }); + + await getInfos(log as any, url, true); + + expect(appendedText()).toContain('photo1: No video formats found!'); + }); + + it('filters a photo error the chunk boundary cut in half', async () => { + await stub({ + exit: '1', + stdout: JSON.stringify({ ...VideoInfo, id: 'v1', webpage_url: url }), + stderr: 'ERROR: [Instagram] photo1: No video formats found!\n', + // mid-line: the leading half alone reads as a plain unrecognized error + stderr_split: '25', + }); + + await getInfos(log as any, url); + + expect(mockAppend.mock.calls.map(([s]) => s)).toEqual([ + `🧐 Scraping ${url}...`, + ]); + }); + + it('streams a final line yt-dlp left unterminated', async () => { + await stub({ exit: '1', stderr: 'ERROR: Unsupported URL: https://x' }); + + await expect(getInfos(log as any, url)).rejects.toBeInstanceOf(YtdlpError); + + expect(appendedText()).toContain('Unsupported URL: https://x'); + }); + + it('stops holding a line that never ends', async () => { + // three writes with no newline anywhere, each on its own past the hold + // bound: unbounded holding would keep all of it until the process exits, + // however long it runs, and emit it as a single block at the end + await stub({ + exit: '1', + stderr: 'x'.repeat(210 * 1024), + stderr_split: `${70 * 1024},${140 * 1024}`, + }); + + await expect(getInfos(log as any, url)).rejects.toBeInstanceOf(YtdlpError); + + const blocks = mockAppend.mock.calls.filter(([s]) => + s.startsWith(''), + ); + expect(blocks.length).toBeGreaterThan(1); + }); + it('keeps the salvage payload out of the logged error', async () => { const e = new YtdlpError('failed', 'stderr', false, 'x'.repeat(1000)); expect(Object.keys(e)).not.toContain('stdout'); @@ -883,6 +969,17 @@ describe('downloadVideo', () => { 'ERROR: [Instagram] Dbnd91uAyMW: No video formats found!; please report this issue on https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template. Confirm you are on the latest version using yt-dlp -U\n', message: 'Dbnd91uAyMW: No video formats found!', }, + // more than one item: no single id is the reason the post held no video + { + exit: '1', + stderr: ['Dbi3QbCCZYU', 'Dbi3Qa4Cdzq', 'Dbi3QbCCcTd'] + .map( + (id) => + `ERROR: [Instagram] ${id}: No video formats found!; please report this issue on https://github.com/yt-dlp/yt-dlp/issues?q= , filling out the appropriate issue template. Confirm you are on the latest version using yt-dlp -U\n`, + ) + .join(''), + message: 'There is no video in this post', + }, { exit: '1', stderr: @@ -1088,12 +1185,19 @@ describe('isPermanentError', () => { ); }); - it('falls back to the photo line when photos are all a post has', () => { + it('blames the post, not a photo, when photos are all a post has', () => { const stderr = 'ERROR: [Instagram] photo1: No video formats found!\n' + 'ERROR: [Instagram] photo2: No video formats found!'; expect(new YtdlpError('failed', stderr).message).toBe( - 'photo2: No video formats found!', + 'There is no video in this post', + ); + }); + + it("keeps a lone no-formats line, the extractor's verdict on the post", () => { + const stderr = 'ERROR: [Instagram] DaaWmzzAH9s: No video formats found!'; + expect(new YtdlpError('failed', stderr).message).toBe( + 'DaaWmzzAH9s: No video formats found!', ); });