From 00b96d2eaab071e65a02452b0416088866a2c451 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Tue, 7 Jul 2026 17:10:17 -0400 Subject: [PATCH] fix(producer): fall back to copying extracted frames when symlink hits EPERM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(producer): fall back to copying extracted frames when symlink hits EPERM materializeExtractedFramesForCompiledDir stages each video's extracted frames into the compiled dir via a single symlink (the in-process renderer's default; distributed plan() already copies via materializeSymlinks). Windows without Developer Mode (or Administrator) cannot create symlinks and rejects with EPERM, so high/standard-quality renders failed there — while draft quality worked because it avoids the symlinked-cache path entirely. Fix: a new stageExtractedFrameDir helper catches EPERM/EACCES from symlinkSync and falls back to the same recursive cpSync the materializeSymlinks path already uses. The extra disk is far better than a hard render failure on a default Windows configuration. Non-permission errors (ENOSPC, etc.) still propagate so real failures aren't masked as silent copies. Extracting the helper also keeps the main function under the complexity gate. Test: two new cases via the injected fileSystem — symlinkSync throwing EPERM triggers exactly one recursive cpSync (frames still remapped under compiledDir), and a non-permission error (ENOSPC) rethrows without falling back to copy. Full renderOrchestrator suite (81) passes. * fix(producer): recover from a stale dangling frame-symlink (EEXIST) Follow-up to this PR's EPERM copy fallback, from a further Windows report: the symlink fails with EEXIST after the extraction cache is GC'd. A prior render's symlink at the compiled linkPath dangles once its target is removed; the caller's existsSync() guard follows the dead link and reads it as absent, so staging runs again, but the link file still exists and symlinkSync collides with EEXIST -> the render hard-fails. Catch EEXIST in stageExtractedFrameDir, clear the stale entry (rmSync), and re-stage (link, or copy on EPERM/EACCES). Factored the link-or-copy into a helper reused by both the first attempt and the retry. rmSync is an optional injected fs method (default fs supplies it; only the EEXIST path calls it). New unit test covers the dangling-symlink recovery. * fix(producer): widen symlink fallback to UNKNOWN and cover EEXIST on copy path Addresses review nits on the frame-staging fallback: - Widen the symlink no-privilege catch from EPERM/EACCES to also include UNKNOWN (some Windows builds surface a symlink privilege denial as UNKNOWN). - Wrap the EEXIST stale-entry recovery around BOTH staging branches, not just the symlink one: after #2025 Windows uses the eager cpSync path, which can collide with a dangling symlink left by a prior Linux run — now it clears the stale entry and re-stages either way. - Emit a one-time INFO log when symlinking degrades to copying, so a heavier Windows render is self-explanatory. --- .../producer/src/services/render/shared.ts | 96 ++++++++- .../src/services/renderOrchestrator.test.ts | 182 ++++++++++++++++++ 2 files changed, 272 insertions(+), 6 deletions(-) diff --git a/packages/producer/src/services/render/shared.ts b/packages/producer/src/services/render/shared.ts index 4e26e9eb5..e6c8b8a35 100644 --- a/packages/producer/src/services/render/shared.ts +++ b/packages/producer/src/services/render/shared.ts @@ -10,7 +10,15 @@ * backwards compatibility with existing test files and external callers. */ -import { copyFileSync, cpSync, existsSync, mkdirSync, symlinkSync, writeFileSync } from "node:fs"; +import { + copyFileSync, + cpSync, + existsSync, + mkdirSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; import { CANVAS_DIMENSIONS, @@ -275,6 +283,10 @@ type MaterializeFileSystem = { mkdirSync: (path: string, options: { recursive: true }) => unknown; symlinkSync: (target: string, path: string) => unknown; cpSync: (src: string, dest: string, options: { recursive: true }) => unknown; + // Optional: only the stale-entry (EEXIST) recovery path calls it, and the + // default fileSystem always supplies it. Test doubles that never trigger + // EEXIST may omit it. + rmSync?: (path: string, options: { recursive: true; force: true }) => unknown; }; type MaterializeExtractedFramesOptions = { @@ -304,6 +316,7 @@ const materializeFileSystem: MaterializeFileSystem = { mkdirSync, symlinkSync, cpSync, + rmSync, }; /** @@ -373,6 +386,76 @@ export function createMemorySampler(intervalMs: number = 250): MemorySampler { * Exported for integration tests; not part of the stable public API — * external callers should use `executeRenderJob` instead. */ +// Stage one video's extracted-frame dir into the compiled dir. Default is a +// single symlink (cheap; the in-process renderer); `materializeSymlinks` copies +// instead (distributed plan() needs a self-contained dir). On Windows without +// Developer Mode/Administrator symlink creation is rejected with EPERM/EACCES, +// which failed high/standard renders — degrade to a copy there rather than +// throwing. Non-permission errors still propagate so real failures aren't hidden. +// One-time guard for the symlink→copy fallback notice below. +let warnedSymlinkFallback = false; + +// Create the symlink, degrading to a copy on Windows' no-symlink-privilege +// errors (EPERM/EACCES, plus UNKNOWN — some Windows builds surface a symlink +// privilege denial as an UNKNOWN-coded error rather than EPERM). Non-permission +// errors propagate. +function linkOrCopyFrameDir(fileSystem: MaterializeFileSystem, src: string, dest: string): void { + try { + fileSystem.symlinkSync(src, dest); + } catch (err) { + const code = (err as NodeJS.ErrnoException | undefined)?.code; + if (code !== "EPERM" && code !== "EACCES" && code !== "UNKNOWN") throw err; + // Copying is measurably slower than symlinking, so surface the degrade once + // — it explains a render that suddenly got heavier and saves a support + // round-trip diagnosing slow frame staging on Windows. + if (!warnedSymlinkFallback) { + warnedSymlinkFallback = true; + defaultLogger.info( + `[Render] Symlinking extracted frames was rejected (${code}); copying them into the compiled dir instead. Expected on Windows without Developer Mode/Administrator.`, + ); + } + fileSystem.cpSync(src, dest, { recursive: true }); + } +} + +function stageExtractedFrameDir( + fileSystem: MaterializeFileSystem, + src: string, + dest: string, + materializeSymlinks: boolean, +): void { + try { + stageExtractedFrameDirOnce(fileSystem, src, dest, materializeSymlinks); + } catch (err) { + if ((err as NodeJS.ErrnoException | undefined)?.code !== "EEXIST") throw err; + // A stale entry already sits at `dest` — typically a DANGLING symlink left + // after the extraction cache was GC'd (its target removed), or a Windows + // machine reusing a dir a prior Linux run populated with symlinks (the eager + // cpSync then collides with the existing link). The caller's existsSync() + // guard follows the link, so the dead link reads as absent and we reach + // here; the entry itself still exists, so re-staging collides with EEXIST. + // Clear the stale entry and re-stage. Applies to BOTH the symlink and + // eager-copy paths so a cross-platform dir reuse self-heals either way. + fileSystem.rmSync?.(dest, { recursive: true, force: true }); + stageExtractedFrameDirOnce(fileSystem, src, dest, materializeSymlinks); + } +} + +// One staging attempt: eager copy for the distributed self-contained dir, +// otherwise a symlink (degrading to a copy on no-symlink-privilege errors). +function stageExtractedFrameDirOnce( + fileSystem: MaterializeFileSystem, + src: string, + dest: string, + materializeSymlinks: boolean, +): void { + if (materializeSymlinks) { + fileSystem.cpSync(src, dest, { recursive: true }); + return; + } + linkOrCopyFrameDir(fileSystem, src, dest); +} + export function materializeExtractedFramesForCompiledDir( extracted: MaterializedExtractedFrames[], compiledDir: string, @@ -390,11 +473,12 @@ export function materializeExtractedFramesForCompiledDir( const linkPath = pathModule.join(compiledFrameRoot, ext.videoId); if (!fileSystem.existsSync(linkPath)) { fileSystem.mkdirSync(pathModule.dirname(linkPath), { recursive: true }); - if (options.materializeSymlinks) { - fileSystem.cpSync(resolvedOut, linkPath, { recursive: true }); - } else { - fileSystem.symlinkSync(resolvedOut, linkPath); - } + stageExtractedFrameDir( + fileSystem, + resolvedOut, + linkPath, + options.materializeSymlinks === true, + ); } const remapped = new Map(); diff --git a/packages/producer/src/services/renderOrchestrator.test.ts b/packages/producer/src/services/renderOrchestrator.test.ts index da40f3ac5..0d1516bbb 100644 --- a/packages/producer/src/services/renderOrchestrator.test.ts +++ b/packages/producer/src/services/renderOrchestrator.test.ts @@ -554,6 +554,188 @@ describe("materializeExtractedFramesForCompiledDir", () => { expect(extracted.framePaths.get(0)).toBe(win32.join(linkPath, "frame_000001.jpg")); expect(copies).toEqual([{ src: outputDir, dest: linkPath, recursive: true }]); }); + + // fallow-ignore-next-line code-duplication + it("falls back to copying frames when symlinkSync fails with EPERM (Windows, no Developer Mode)", () => { + // Windows without Developer Mode/Administrator rejects symlink creation with + // EPERM — high/standard-quality renders failed here while draft worked. The + // helper must degrade to a recursive copy instead of throwing. + const compiledDir = win32.resolve("C:\\compiled"); + const outputDir = win32.resolve("D:\\cache\\abc123"); + const framePath = win32.join(outputDir, "frame_000001.jpg"); + const extracted = createExtractedFrames(outputDir, framePath); + const copies: Array<{ src: string; dest: string; recursive: boolean }> = []; + + // fallow-ignore-next-line code-duplication + materializeExtractedFramesForCompiledDir([extracted], compiledDir, { + pathModule: win32, + fileSystem: { + existsSync: () => false, + mkdirSync: () => undefined, + symlinkSync: () => { + const err: NodeJS.ErrnoException = new Error("EPERM: operation not permitted, symlink"); + err.code = "EPERM"; + throw err; + }, + cpSync: (src, dest, options) => { + copies.push({ src, dest, recursive: options.recursive }); + }, + }, + }); + + const linkPath = win32.join(compiledDir, "__hyperframes_video_frames", "video-1"); + expect(copies).toEqual([{ src: outputDir, dest: linkPath, recursive: true }]); + expect(extracted.outputDir).toBe(linkPath); + expect(extracted.framePaths.get(0)).toBe(win32.join(linkPath, "frame_000001.jpg")); + }); + + it("rethrows a non-permission symlink error instead of masking it with a copy", () => { + const compiledDir = win32.resolve("C:\\compiled"); + const outputDir = win32.resolve("D:\\cache\\abc123"); + const framePath = win32.join(outputDir, "frame_000001.jpg"); + const extracted = createExtractedFrames(outputDir, framePath); + + expect(() => + materializeExtractedFramesForCompiledDir([extracted], compiledDir, { + pathModule: win32, + fileSystem: { + existsSync: () => false, + mkdirSync: () => undefined, + symlinkSync: () => { + const err: NodeJS.ErrnoException = new Error("ENOSPC: no space left"); + err.code = "ENOSPC"; + throw err; + }, + cpSync: () => { + throw new Error("must not fall back to copy for a non-permission error"); + }, + }, + }), + ).toThrow(/ENOSPC/); + }); + + // fallow-ignore-next-line code-duplication + it("clears a stale dangling entry and re-stages when symlinkSync fails with EEXIST", () => { + // After the extraction cache is GC'd, a symlink from a prior render dangles + // (its target removed). existsSync() follows the dead link so the caller's + // guard reads it as absent and reaches staging, but the link file itself + // still exists, so symlinkSync collides with EEXIST. The helper must clear + // the stale entry (rmSync) and re-stage, not hard-fail the render. + const compiledDir = win32.resolve("C:\\compiled"); + const outputDir = win32.resolve("D:\\cache\\abc123"); + const framePath = win32.join(outputDir, "frame_000001.jpg"); + const extracted = createExtractedFrames(outputDir, framePath); + const linkPath = win32.join(compiledDir, "__hyperframes_video_frames", "video-1"); + const removed: string[] = []; + const symlinks: Array<{ target: string; path: string }> = []; + let symlinkCalls = 0; + + // fallow-ignore-next-line code-duplication + materializeExtractedFramesForCompiledDir([extracted], compiledDir, { + pathModule: win32, + fileSystem: { + existsSync: () => false, + mkdirSync: () => undefined, + symlinkSync: (target, path) => { + symlinkCalls += 1; + if (symlinkCalls === 1) { + const err: NodeJS.ErrnoException = new Error("EEXIST: file already exists, symlink"); + err.code = "EEXIST"; + throw err; + } + symlinks.push({ target, path }); + }, + cpSync: () => { + throw new Error("EEXIST recovery should re-link, not copy"); + }, + rmSync: (path) => { + removed.push(path); + }, + }, + }); + + expect(removed).toEqual([linkPath]); + expect(symlinks).toEqual([{ target: outputDir, path: linkPath }]); + expect(extracted.framePaths.get(0)).toBe(win32.join(linkPath, "frame_000001.jpg")); + }); + + // fallow-ignore-next-line code-duplication + it("falls back to copying when symlinkSync fails with UNKNOWN (some Windows privilege denials)", () => { + // Some Windows builds surface a no-symlink-privilege denial as an + // UNKNOWN-coded error rather than EPERM/EACCES — it must still degrade to a + // copy, not hard-fail the render. + const compiledDir = win32.resolve("C:\\compiled"); + const outputDir = win32.resolve("D:\\cache\\abc123"); + const framePath = win32.join(outputDir, "frame_000001.jpg"); + const extracted = createExtractedFrames(outputDir, framePath); + const copies: Array<{ src: string; dest: string; recursive: boolean }> = []; + + // fallow-ignore-next-line code-duplication + materializeExtractedFramesForCompiledDir([extracted], compiledDir, { + pathModule: win32, + fileSystem: { + existsSync: () => false, + mkdirSync: () => undefined, + symlinkSync: () => { + const err: NodeJS.ErrnoException = new Error("UNKNOWN: unknown error, symlink"); + err.code = "UNKNOWN"; + throw err; + }, + cpSync: (src, dest, options) => { + copies.push({ src, dest, recursive: options.recursive }); + }, + }, + }); + + const linkPath = win32.join(compiledDir, "__hyperframes_video_frames", "video-1"); + expect(copies).toEqual([{ src: outputDir, dest: linkPath, recursive: true }]); + expect(extracted.framePaths.get(0)).toBe(win32.join(linkPath, "frame_000001.jpg")); + }); + + // fallow-ignore-next-line code-duplication + it("clears a stale entry and re-copies when the eager-copy path (materializeSymlinks) hits EEXIST", () => { + // #2025 routes Windows through the eager-copy branch. Reusing a dir a prior + // Linux run populated with a (now dangling) symlink makes cpSync collide + // with EEXIST — the recovery must clear the stale entry and re-copy, exactly + // like the symlink path does. + const compiledDir = win32.resolve("C:\\compiled"); + const outputDir = win32.resolve("D:\\cache\\abc123"); + const framePath = win32.join(outputDir, "frame_000001.jpg"); + const extracted = createExtractedFrames(outputDir, framePath); + const linkPath = win32.join(compiledDir, "__hyperframes_video_frames", "video-1"); + const removed: string[] = []; + const copies: Array<{ src: string; dest: string }> = []; + let cpCalls = 0; + + // fallow-ignore-next-line code-duplication + materializeExtractedFramesForCompiledDir([extracted], compiledDir, { + pathModule: win32, + materializeSymlinks: true, + fileSystem: { + existsSync: () => false, + mkdirSync: () => undefined, + symlinkSync: () => { + throw new Error("eager-copy path must not symlink"); + }, + cpSync: (src, dest) => { + cpCalls += 1; + if (cpCalls === 1) { + const err: NodeJS.ErrnoException = new Error("EEXIST: file already exists, cp"); + err.code = "EEXIST"; + throw err; + } + copies.push({ src, dest }); + }, + rmSync: (path) => { + removed.push(path); + }, + }, + }); + + expect(removed).toEqual([linkPath]); + expect(copies).toEqual([{ src: outputDir, dest: linkPath }]); + expect(extracted.framePaths.get(0)).toBe(win32.join(linkPath, "frame_000001.jpg")); + }); }); describe("writeCompiledArtifacts — external assets on Windows drive-letter paths (GH #321)", () => {