Repository navigation
fix(producer): a drive that refuses frame links gets a copy, whatever code it refuses with - #7163
Conversation
… code it refuses with
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Requesting changes at 1e8c7768. Widening the fallback swallows EEXIST, and that breaks the stale-link self-heal in stageExtractedFrameDir.
Blocker: a dangling frame link left by a GC'd extraction cache now fails the render
stageExtractedFrameDir relies on EEXIST reaching it from the symlink attempt. Its handler removes the stale entry and stages again. The comment there describes the case: a dangling symlink at dest left after the extraction cache was GC'd, or a dir reused from a Linux run.
Before this PR, linkOrCopyFrameDir rethrew EEXIST, because it wasn't in the allow-list. Now every non-full-disk code goes on to the fallback:
- On POSIX, the junction is skipped and
cpSync(src, dest)runs onto the existing link. - On Windows, the junction also gets
EEXIST, returns false, and the samecpSyncruns.
cpSync throws ERR_FS_CP_DIR_TO_NON_DIR, not EEXIST, so the self-heal never runs and the render fails.
I reproduced it on Linux with a real filesystem. I put a dangling dir symlink at compiled/__hyperframes_video_frames/clip, then called materializeExtractedFramesForCompiledDir:
- main: passes, and
clipis relinked as a symlink. - this head: throws
ERR_FS_CP_DIR_TO_NON_DIR: Cannot overwrite non-directory …/clip with directory …/cache.
Suggested fix: let EEXIST through as well as a full disk, in the symlink catch and in tryWindowsFrameJunction. For example, if (isDiskFull(err) || code === "EEXIST") throw err;. Then add a test with a real dangling link at dest that ends as a link to the frames. That test fails at this head.
Non-blocking
- The comment above
linkOrCopyFrameDirstill says it falls back only on "Windows' no-symlink-privilege errors (EPERM/EACCES, plus UNKNOWN)" and that "Other symlink errors … propagate". Both are false now. Editing it in place keeps the comment-line count the same. ENOENT(a missingsrc) now goes through the junction and copy before it fails, and the message comes fromcpSync. That's harmless, but the "copying instead" log now namesENOENT.
Checked
- The
fails without the fix:line names a test that exists.windowsFrameStaging.test.tspasses at this head: 14 passed, 1 skipped.
|
Right, thank you: that broke the stale-link self-heal. Fixed at |
There was a problem hiding this comment.
Approving at 13dbb39a. The blocker from my review at 1e8c7768 is fixed.
What I ran on this head
copyCannotFixnow letsEEXISTandENOENTthrough as well as a full disk, in both the symlink catch andtryWindowsFrameJunction. So a stale entry atdestreaches the self-heal instageExtractedFrameDiragain.- On Linux,
windowsFrameStaging.test.tspasses at this head: 15 passed, 1 skipped. - With the previous head's
shared.ts, only the new "relinks over a dangling frame link left where the frames go" test fails. - With main's
shared.ts, only the three fallback cases fail, including the test named on thefails without the fix:line. That matches my earlier reproduction. - The comment above
linkOrCopyFrameDirnow matches the code. Comments, PR captures, preview-regression and Player perf are green, and one CI run is green on this head. A second CI run, Windows render, regression and CodeQL were still running when I approved.
Non-blocking
- The dangling-link test asserts that
destis a symlink and reads the frame throughextracted.framePaths(the source path). A dangling link is also a symlink, so the test proves the re-link mainly because nothing throws. Reading the frame throughdest, or checkingreadlinkSync(dest), would pin the result. ENOENTpropagating matches main's behaviour, so nothing changes there.
|
Taken at |
There was a problem hiding this comment.
Approving at 5b04fd6e. The only change since my approval at 13dbb39a is one line in windowsFrameStaging.test.ts. The dangling-link test now reads the frame through dest (join(dest, "frame_000001.jpg")). It no longer reads the source path, so the test now checks that the link really points at the frames. That was my non-blocking note.
shared.ts is unchanged. The test file passes on this head (15 passed, 1 skipped). CI, CodeQL, Comments, PR captures, preview-regression and Player perf are green; Windows render and regression were still running when I approved.
Ports: heygen-com#7158 (failed run exits 1 when its output pipe closes early), heygen-com#7161 (ENOSPC during frame extraction reports disk_full), heygen-com#7163 (copy frames when a drive refuses links), heygen-com#7171 (retry with fewer workers when every page times out), heygen-com#7172 (split a clip whose start names another clip, adapted), heygen-com#7169 (lint rule audio_attribute_unreadable), and early heygen-com#7179, heygen-com#7173. Watermarks: commit 370f886, PR 7164; monitored 91. Co-authored-by: Miguel Ángel <[email protected]> Co-authored-by: WaterrrForever <[email protected]> Co-Authored-By: Claude Opus 5.5 <[email protected]>
What
When a drive refuses the link that puts a render's extracted frames into its work folder, the render now falls back to a junction and then a copy, whatever error code the drive refuses with. Three failures copying cannot fix still propagate: a full disk (
ENOSPC,EDQUOT), an entry already at the destination (EEXIST, whichstageExtractedFrameDirclears and relinks, e.g. a dangling link left after the extraction cache was collected), and a missing source (ENOENT).Why
linkOrCopyFrameDirfell back only when the symlink failed withEPERM,EACCESorUNKNOWN, andtryWindowsFrameJunctiononly for a fixed list of codes; any other code was rethrown and the render failed. A Windows desktop user exporting to an external drive (E:\…) had the render fail at once onsymlink '<frame cache>' -> '<work folder>'with a code outside that list, while the same render would have worked with the copy the fallback already does. A drive can refuse links in more ways than the lists named (FAT32 and exFAT drives cannot hold links at all), so the lists were the wrong shape: every refusal is a reason to copy, except the few failures a copy cannot fix.Related work
#3740 added the junction before the copy and its rule that a disk failure is never hidden by copying; that rule stays and now covers the symlink attempt too.
How
packages/producer/src/services/render/shared.ts: onecopyCannotFixcheck (ENOSPC,EDQUOT,EEXIST,ENOENT). The symlink attempt rethrows only those, then tries the junction; the junction attempt rethrows only those, then returns false so the existing copy runs.EEXISTkeeps reachingstageExtractedFrameDir's self-heal (review). The comment abovelinkOrCopyFrameDirnow says this, at the same line count. The once-only "copying instead" log line is unchanged.fails without the fix: packages/producer/src/services/windowsFrameStaging.test.ts › Windows link fallbacks › copies after the drive refuses the symlink with EISDIR and the junction too
Size
Small on purpose: two allow-lists become one check of what a copy cannot fix, at the one place frame folders are linked, with its tests.
Test plan
windowsFrameStaging.test.ts, new: a symlink and junction refused withEISDIR,EIOorEINVALend in a copy with the frames readable; a symlink that finds the disk full fails the render with no junction and no copy. A real danglingdirlink at the destination ends as a link to the frames (real filesystem, not on Windows). The existing cases still pass, including "does not hide a disk failure from the junction attempt". On Linux at this commit: 15 passed, 1 skipped (the real-Windows case), 3 runs in a row. With main'sshared.ts, the three new fallback cases fail; with the first head's (1e8c7768), the dangling-link case fails.oxfmt --check,oxlinton both files: clean.check-comment-citationsandcomment-ratchetagainst main: OK.tsc --noEmitinpackages/producer: no errors in the changed files.