refactor(main): execFileAsync helper — unify the 5 spawn-and-read sites

Replace the hand-rolled `new Promise((resolve) => execFile(…, cb))` wrappers in
probe / ytdlp (×2) / ffmpeg / indexer.probeFlat / download.probeMeta with one
lib/exec.ts execFileAsync returning a normalized
{ok,stdout,stderr,timedOut,error} that never rejects. Each caller keeps its own
error mapping (timedOut -> "Timed out …" vs stderr) and JSON parsing; behaviour
is unchanged. windowsHide defaults on; per-call timeout/maxBuffer preserved.

Foundation for CC4/CC5 — the stdout line-buffer half lands with SIMP5, the
net.request half with SIMP16, at which point those checkboxes close.

Unit-tested (exec.test.ts) against the node binary: ok / stderr+nonzero-exit /
missing-binary / timeout. typecheck + 262 tests + eslint + prettier green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-07-01 09:06:08 -04:00
parent 66eaaac8fc
commit 3baba3b7bc
7 changed files with 192 additions and 151 deletions
+8 -11
View File
@@ -14,6 +14,7 @@ import {
YTDLP_MISSING_MSG YTDLP_MISSING_MSG
} from './binaries' } from './binaries'
import { getSettings } from './settings' import { getSettings } from './settings'
import { execFileAsync } from './lib/exec'
import { getDownloadArchivePath, getDefaultMediaDir } from './paths' import { getDownloadArchivePath, getDefaultMediaDir } from './paths'
import { ensureManagedYtdlp } from './ytdlp' import { ensureManagedYtdlp } from './ytdlp'
import { materializeCookies, hasStoredCookies } from './cookies' import { materializeCookies, hasStoredCookies } from './cookies'
@@ -123,9 +124,8 @@ function logFailure(opts: StartDownloadOptions, title: string | undefined, error
// itself contains a newline, since each --print field is emitted on its own line. // itself contains a newline, since each --print field is emitted on its own line.
const META_SEP = '\u001f' const META_SEP = '\u001f'
function probeMeta(ytdlp: string, url: string): Promise<DownloadMeta | null> { async function probeMeta(ytdlp: string, url: string): Promise<DownloadMeta | null> {
return new Promise((resolve) => { const r = await execFileAsync(
execFile(
ytdlp, ytdlp,
[ [
'--no-playlist', '--no-playlist',
@@ -136,19 +136,16 @@ function probeMeta(ytdlp: string, url: string): Promise<DownloadMeta | null> {
'--', '--',
url url
], ],
{ windowsHide: true, maxBuffer: META_MAX_BUFFER, timeout: META_PROBE_TIMEOUT_MS }, { maxBuffer: META_MAX_BUFFER, timeout: META_PROBE_TIMEOUT_MS }
(err, stdout) => { )
if (err) return resolve(null) if (!r.ok) return null
const [title, uploader, duration] = stdout.split(META_SEP).map((l) => l.trim()) const [title, uploader, duration] = r.stdout.split(META_SEP).map((l) => l.trim())
const clean = (v?: string): string | undefined => (v && v !== 'NA' ? v : undefined) const clean = (v?: string): string | undefined => (v && v !== 'NA' ? v : undefined)
resolve({ return {
title: clean(title), title: clean(title),
channel: clean(uploader), channel: clean(uploader),
durationLabel: clean(duration) durationLabel: clean(duration)
})
} }
)
})
} }
// --- Argv construction (shared by startDownload and the command preview) --- // --- Argv construction (shared by startDownload and the command preview) ---
+7 -18
View File
@@ -1,6 +1,6 @@
import { execFile } from 'child_process'
import { existsSync } from 'fs' import { existsSync } from 'fs'
import { getFfmpegPath, getFfprobePath } from './binaries' import { getFfmpegPath, getFfprobePath } from './binaries'
import { execFileAsync } from './lib/exec'
import { VERSION_TIMEOUT_MS } from './constants' import { VERSION_TIMEOUT_MS } from './constants'
import type { FfmpegVersionResult } from '@shared/ipc' import type { FfmpegVersionResult } from '@shared/ipc'
@@ -12,24 +12,13 @@ import type { FfmpegVersionResult } from '@shared/ipc'
* out, or the line doesn't parse — Settings then shows "not found" instead of * out, or the line doesn't parse — Settings then shows "not found" instead of
* failing. Unlike yt-dlp these are never self-updated, so there's no update path. * failing. Unlike yt-dlp these are never self-updated, so there's no update path.
*/ */
function readToolVersion(path: string): Promise<string | null> { async function readToolVersion(path: string): Promise<string | null> {
if (!existsSync(path)) return Promise.resolve(null) if (!existsSync(path)) return null
return new Promise((resolve) => { const r = await execFileAsync(path, ['-version'], { timeout: VERSION_TIMEOUT_MS })
execFile( if (!r.ok) return null
path, const firstLine = r.stdout.split('\n', 1)[0] ?? ''
['-version'],
{ windowsHide: true, timeout: VERSION_TIMEOUT_MS },
(err, stdout) => {
if (err) {
resolve(null)
return
}
const firstLine = stdout.split('\n', 1)[0] ?? ''
const m = firstLine.match(/version\s+(\S+)/i) const m = firstLine.match(/version\s+(\S+)/i)
resolve(m?.[1] ?? null) return m?.[1] ?? null
}
)
})
} }
/** Versions of the bundled ffmpeg + ffprobe, read in parallel for the Settings panel. */ /** Versions of the bundled ffmpeg + ffprobe, read in parallel for the Settings panel. */
+12 -16
View File
@@ -9,9 +9,9 @@
* this module is the impure shell that spawns yt-dlp and writes to disk. * this module is the impure shell that spawns yt-dlp and writes to disk.
*/ */
import { execFile } from 'child_process'
import { existsSync } from 'fs' import { existsSync } from 'fs'
import { getYtdlpPath, YTDLP_MISSING_MSG } from './binaries' import { getYtdlpPath, YTDLP_MISSING_MSG } from './binaries'
import { execFileAsync } from './lib/exec'
import { INDEX_MAX_BUFFER, INDEX_TIMEOUT_MS } from './constants' import { INDEX_MAX_BUFFER, INDEX_TIMEOUT_MS } from './constants'
import { cleanError } from './log' import { cleanError } from './log'
import { assertHttpUrl } from './url' import { assertHttpUrl } from './url'
@@ -46,29 +46,25 @@ interface FlatInfo {
* flat upload list can be a few MB of JSON; the timeout is generous for the same * flat upload list can be a few MB of JSON; the timeout is generous for the same
* reason. `--` terminates option parsing so the URL can't be read as a flag. * reason. `--` terminates option parsing so the URL can't be read as a flag.
*/ */
function probeFlat(url: string): Promise<FlatInfo> { async function probeFlat(url: string): Promise<FlatInfo> {
return new Promise((resolve, reject) => { const r = await execFileAsync(
execFile(
getYtdlpPath(), getYtdlpPath(),
['-J', '--flat-playlist', '--no-warnings', '--', url], ['-J', '--flat-playlist', '--no-warnings', '--', url],
{ windowsHide: true, maxBuffer: INDEX_MAX_BUFFER, timeout: INDEX_TIMEOUT_MS }, { maxBuffer: INDEX_MAX_BUFFER, timeout: INDEX_TIMEOUT_MS }
(err, stdout, stderr) => { )
if (err) { if (!r.ok) {
const msg = (err as { killed?: boolean }).killed throw new Error(
r.timedOut
? 'Timed out indexing source. Check the link or your connection.' ? 'Timed out indexing source. Check the link or your connection.'
: cleanError(stderr) || err.message : cleanError(r.stderr) || r.error?.message || ''
reject(new Error(msg)) )
return
} }
try { try {
resolve(JSON.parse(stdout) as FlatInfo) return JSON.parse(r.stdout) as FlatInfo
} catch { } catch {
reject(new Error('Could not parse source info from yt-dlp.')) throw new Error('Could not parse source info from yt-dlp.')
} }
} }
)
})
}
/** Probe a channel tab (e.g. /videos, /playlists); never throws — returns null. */ /** Probe a channel tab (e.g. /videos, /playlists); never throws — returns null. */
function probeTab(url: string): Promise<FlatInfo | null> { function probeTab(url: string): Promise<FlatInfo | null> {
+52
View File
@@ -0,0 +1,52 @@
import { execFile, type ExecFileOptions } from 'child_process'
/**
* Normalized result of a one-shot child-process run. `ok` is true on a clean
* exit-0; `timedOut` distinguishes a kill-by-timeout (Node sets `err.killed` when
* it terminates the process on the `timeout` option) from an ordinary non-zero
* exit, so callers can show "Timed out …" versus the captured stderr.
*/
export interface ExecResult {
ok: boolean
stdout: string
stderr: string
timedOut: boolean
/** The raw error on a non-zero exit / spawn failure (undefined when ok). */
error?: Error
}
/**
* `execFile` as a promise that never rejects (audit SIMP2/CC4/CC5). Replaces the
* six hand-rolled `new Promise((resolve) => execFile(…, cb))` wrappers across
* probe / ytdlp (×2) / ffmpeg / indexer / download.probeMeta / schedule — each
* caller now maps this normalized result to its own shape instead of repeating the
* `err.killed` timeout check and the resolve/reject plumbing.
*
* `windowsHide` defaults on (every caller wants the console window suppressed);
* pass `timeout` / `maxBuffer` per call exactly as before.
*/
export function execFileAsync(
file: string,
args: readonly string[],
options: ExecFileOptions = {}
): Promise<ExecResult> {
return new Promise((resolve) => {
execFile(file, [...args], { windowsHide: true, ...options }, (err, stdout, stderr) => {
// With the default (utf8) encoding stdout/stderr are strings, but ExecFileOptions
// permits a Buffer encoding, so normalize defensively.
const out = typeof stdout === 'string' ? stdout : stdout.toString()
const errOut = typeof stderr === 'string' ? stderr : stderr.toString()
if (err) {
resolve({
ok: false,
stdout: out,
stderr: errOut,
timedOut: (err as { killed?: boolean }).killed === true,
error: err
})
} else {
resolve({ ok: true, stdout: out, stderr: errOut, timedOut: false })
}
})
})
}
+17 -29
View File
@@ -1,6 +1,6 @@
import { execFile } from 'child_process'
import { existsSync } from 'fs' import { existsSync } from 'fs'
import { getYtdlpPath, YTDLP_MISSING_MSG } from './binaries' import { getYtdlpPath, YTDLP_MISSING_MSG } from './binaries'
import { execFileAsync } from './lib/exec'
import { PROBE_MAX_BUFFER, PROBE_TIMEOUT_MS } from './constants' import { PROBE_MAX_BUFFER, PROBE_TIMEOUT_MS } from './constants'
import { fmtBytes } from '@shared/format' import { fmtBytes } from '@shared/format'
import { cleanError } from './log' import { cleanError } from './log'
@@ -114,52 +114,40 @@ function buildPlaylist(data: RawInfo): PlaylistInfo {
* its format list) or, when the URL is a playlist, the flat list of its entries. * its format list) or, when the URL is a playlist, the flat list of its entries.
* `--flat-playlist` is a no-op for a lone video, so one call covers both cases. * `--flat-playlist` is a no-op for a lone video, so one call covers both cases.
*/ */
export function probeMedia(url: string): Promise<ProbeResult> { export async function probeMedia(url: string): Promise<ProbeResult> {
const ytdlp = getYtdlpPath() const ytdlp = getYtdlpPath()
if (!existsSync(ytdlp)) { if (!existsSync(ytdlp)) {
return Promise.resolve({ return { ok: false, error: YTDLP_MISSING_MSG }
ok: false,
error: YTDLP_MISSING_MSG
})
} }
let target: string let target: string
try { try {
target = assertHttpUrl(url) // normalised form (audit F5) target = assertHttpUrl(url) // normalised form (audit F5)
} catch (e) { } catch (e) {
return Promise.resolve({ ok: false, error: (e as Error).message }) return { ok: false, error: (e as Error).message }
} }
return new Promise((resolve) => {
execFile(
ytdlp,
// `--` terminates option parsing so the URL can never be read as a flag. // `--` terminates option parsing so the URL can never be read as a flag.
['-J', '--flat-playlist', '--no-warnings', '--', target], const r = await execFileAsync(ytdlp, ['-J', '--flat-playlist', '--no-warnings', '--', target], {
{ windowsHide: true, maxBuffer: PROBE_MAX_BUFFER, timeout: PROBE_TIMEOUT_MS }, maxBuffer: PROBE_MAX_BUFFER,
(err, stdout, stderr) => { timeout: PROBE_TIMEOUT_MS
if (err) { })
// execFile sets `killed` when it terminated the process on timeout. if (!r.ok) {
const msg = (err as { killed?: boolean }).killed const msg = r.timedOut
? 'Timed out fetching video info. Check the link or your connection.' ? 'Timed out fetching video info. Check the link or your connection.'
: cleanError(stderr) || err.message : cleanError(r.stderr) || r.error?.message || ''
resolve({ ok: false, error: msg }) return { ok: false, error: msg }
return
} }
try { try {
const data = JSON.parse(stdout) as RawInfo const data = JSON.parse(r.stdout) as RawInfo
if (data._type === 'playlist' || Array.isArray(data.entries)) { if (data._type === 'playlist' || Array.isArray(data.entries)) {
const playlist = buildPlaylist(data) const playlist = buildPlaylist(data)
if (playlist.count === 0) { if (playlist.count === 0) {
resolve({ ok: false, error: 'This playlist has no downloadable entries.' }) return { ok: false, error: 'This playlist has no downloadable entries.' }
return
} }
resolve({ ok: true, kind: 'playlist', playlist }) return { ok: true, kind: 'playlist', playlist }
} else {
resolve({ ok: true, kind: 'video', info: buildInfo(data) })
} }
return { ok: true, kind: 'video', info: buildInfo(data) }
} catch { } catch {
resolve({ ok: false, error: 'Could not parse video info from yt-dlp.' }) return { ok: false, error: 'Could not parse video info from yt-dlp.' }
} }
} }
)
})
}
+21 -43
View File
@@ -1,7 +1,7 @@
import { execFile } from 'child_process'
import { existsSync, mkdirSync, copyFileSync } from 'fs' import { existsSync, mkdirSync, copyFileSync } from 'fs'
import { dirname } from 'path' import { dirname } from 'path'
import { getYtdlpPath, getBundledYtdlpPath, YTDLP_MISSING_MSG } from './binaries' import { getYtdlpPath, getBundledYtdlpPath, YTDLP_MISSING_MSG } from './binaries'
import { execFileAsync } from './lib/exec'
import { VERSION_TIMEOUT_MS, YTDLP_UPDATE_TIMEOUT_MS } from './constants' import { VERSION_TIMEOUT_MS, YTDLP_UPDATE_TIMEOUT_MS } from './constants'
import { getSettings, setSettings } from './settings' import { getSettings, setSettings } from './settings'
import { shouldAutoCheckYtdlp } from './ytdlpPolicy' import { shouldAutoCheckYtdlp } from './ytdlpPolicy'
@@ -34,33 +34,21 @@ export function ensureManagedYtdlp(): void {
} }
/** Spawn the bundled yt-dlp and read back its `--version` for the Settings panel. */ /** Spawn the bundled yt-dlp and read back its `--version` for the Settings panel. */
export function getYtdlpVersion(): Promise<YtdlpVersionResult> { export async function getYtdlpVersion(): Promise<YtdlpVersionResult> {
const ytdlpPath = getYtdlpPath() const ytdlpPath = getYtdlpPath()
if (!existsSync(ytdlpPath)) { if (!existsSync(ytdlpPath)) {
return Promise.resolve({ return { ok: false, error: YTDLP_MISSING_MSG }
ok: false,
error: YTDLP_MISSING_MSG
})
} }
return new Promise((resolve) => { const r = await execFileAsync(ytdlpPath, ['--version'], { timeout: VERSION_TIMEOUT_MS })
execFile( if (!r.ok) {
ytdlpPath, const msg = r.timedOut
['--version'],
{ windowsHide: true, timeout: VERSION_TIMEOUT_MS },
(err, stdout, stderr) => {
if (err) {
const msg = (err as { killed?: boolean }).killed
? 'Timed out running yt-dlp.' ? 'Timed out running yt-dlp.'
: (stderr || err.message).trim() : (r.stderr || r.error?.message || '').trim()
resolve({ ok: false, error: msg }) return { ok: false, error: msg }
return
} }
resolve({ ok: true, version: stdout.trim() }) return { ok: true, version: r.stdout.trim() }
}
)
})
} }
/** /**
@@ -69,13 +57,13 @@ export function getYtdlpVersion(): Promise<YtdlpVersionResult> {
* yt-dlp.exe lives in, which holds for both the dev resources/bin checkout and * yt-dlp.exe lives in, which holds for both the dev resources/bin checkout and
* a per-user install; it would fail under a locked-down system install. * a per-user install; it would fail under a locked-down system install.
*/ */
export function updateYtdlp(channel: YtdlpUpdateChannel): Promise<YtdlpUpdateResult> { export async function updateYtdlp(channel: YtdlpUpdateChannel): Promise<YtdlpUpdateResult> {
// Validate against the channel allowlist BEFORE the value reaches `--update-to`. // Validate against the channel allowlist BEFORE the value reaches `--update-to`.
// That flag also accepts `OWNER/REPO@TAG`, which would download and install an // That flag also accepts `OWNER/REPO@TAG`, which would download and install an
// arbitrary binary over yt-dlp.exe — so an unrecognised value (e.g. forged by a // arbitrary binary over yt-dlp.exe — so an unrecognised value (e.g. forged by a
// compromised renderer over IPC) must never be forwarded. (audit F1) // compromised renderer over IPC) must never be forwarded. (audit F1)
if (!isYtdlpUpdateChannel(channel)) { if (!isYtdlpUpdateChannel(channel)) {
return Promise.resolve({ ok: false, error: 'Unsupported update channel.' }) return { ok: false, error: 'Unsupported update channel.' }
} }
// Self-heal: restore the managed copy from the bundled seed before updating, so // Self-heal: restore the managed copy from the bundled seed before updating, so
@@ -84,29 +72,19 @@ export function updateYtdlp(channel: YtdlpUpdateChannel): Promise<YtdlpUpdateRes
const ytdlpPath = getYtdlpPath() const ytdlpPath = getYtdlpPath()
if (!existsSync(ytdlpPath)) { if (!existsSync(ytdlpPath)) {
return Promise.resolve({ return { ok: false, error: YTDLP_MISSING_MSG }
ok: false,
error: YTDLP_MISSING_MSG
})
} }
return new Promise((resolve) => { const r = await execFileAsync(ytdlpPath, ['--update-to', channel], {
execFile( timeout: YTDLP_UPDATE_TIMEOUT_MS
ytdlpPath,
['--update-to', channel],
{ windowsHide: true, timeout: YTDLP_UPDATE_TIMEOUT_MS },
(err, stdout, stderr) => {
if (err) {
const msg = (err as { killed?: boolean }).killed
? 'Timed out updating yt-dlp.'
: (stderr || err.message).trim()
resolve({ ok: false, error: msg })
return
}
resolve({ ok: true, output: stdout.trim() })
}
)
}) })
if (!r.ok) {
const msg = r.timedOut
? 'Timed out updating yt-dlp.'
: (r.stderr || r.error?.message || '').trim()
return { ok: false, error: msg }
}
return { ok: true, output: r.stdout.trim() }
} }
/** /**
+41
View File
@@ -0,0 +1,41 @@
import { describe, it, expect } from 'vitest'
import { execFileAsync } from '../src/main/lib/exec'
// Exercised against the Node binary itself so the test needs no fixtures and is
// deterministic. Covers the contract the six migrated spawn sites rely on
// (SIMP2/CC4/CC5): normalized ok/stdout/stderr, never-rejects, and the timedOut flag.
describe('execFileAsync (SIMP2/CC4/CC5)', () => {
it('resolves ok with stdout on a clean exit', async () => {
const r = await execFileAsync(process.execPath, ['-e', 'process.stdout.write("hello")'])
expect(r.ok).toBe(true)
expect(r.stdout).toBe('hello')
expect(r.timedOut).toBe(false)
expect(r.error).toBeUndefined()
})
it('captures stderr and reports not-ok on a non-zero exit', async () => {
const r = await execFileAsync(process.execPath, [
'-e',
'process.stderr.write("boom"); process.exit(2)'
])
expect(r.ok).toBe(false)
expect(r.stderr).toContain('boom')
expect(r.error).toBeInstanceOf(Error)
expect(r.timedOut).toBe(false)
})
it('never rejects — a missing binary resolves not-ok with an error', async () => {
const r = await execFileAsync('definitely-not-a-real-binary-xyz', ['--version'])
expect(r.ok).toBe(false)
expect(r.error).toBeDefined()
expect(r.timedOut).toBe(false)
})
it('flags timedOut when the timeout kills the process', async () => {
const r = await execFileAsync(process.execPath, ['-e', 'setTimeout(() => {}, 5000)'], {
timeout: 100
})
expect(r.ok).toBe(false)
expect(r.timedOut).toBe(true)
})
})