ci: run vp through a file so forwarding output cannot hit EAGAIN - #2697
Merged
Conversation
✅ Deploy Preview for vjs-registry canceled.
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@videojs/cdn
@videojs/core
@videojs/element
@videojs/html
@videojs/media
@videojs/react
@videojs/spf
@videojs/store
@videojs/utils
@videojs/cloudflare-video
@videojs/dash-video
@videojs/hlsjs-video
@videojs/mux-audio
@videojs/mux-video
@videojs/native-hls-video
@videojs/shaka-video
@videojs/spotify-audio
@videojs/tiktok-video
@videojs/twitch-video
@videojs/vimeo-video
@videojs/wistia-video
@videojs/youtube-video
@videojs/google-cast
@videojs/mux-data
commit: |
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (18)
Players (5)
Skins (29)
UI Components (62)
⚛️ @videojs/react — no changesPresets (7)
Media (22)
Extensions (2)
Players (5)
Skins (18)
UI Components (39)
🧩 @videojs/core — no changesEntries (76)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (13)
📦 @videojs/cdn — no changes📦 @videojs/cloudflare-video — no changes📦 @videojs/dash-video — no changes📦 @videojs/google-cast — no changes📦 @videojs/hlsjs-video — no changes📦 @videojs/media — no changesEntries (3)
📦 @videojs/mux — no changes📦 @videojs/mux-audio — no changesEntries (2)
📦 @videojs/mux-data — no changes📦 @videojs/mux-video — no changesEntries (2)
📦 @videojs/native-hls-video — no changes📦 @videojs/shaka-video — no changes📦 @videojs/spf — no changesEntries (7)
📦 @videojs/spotify-audio — no changes📦 @videojs/tiktok-video — no changes📦 @videojs/twitch-video — no changes📦 @videojs/vimeo-video — no changes📦 @videojs/wistia-video — no changesEntries (2)
📦 @videojs/youtube-video — no changesℹ️ How to interpretEach entry is independently bundled, minified, and brotli-compressed. Initial size includes its static import graph; lazy dynamic chunks are reported separately. Entries are not additive because their dependency graphs overlap. Preset rows represent realistic combined bundles. Changes of 300 B or less across initial, lazy, and total size are collapsed, not discarded. Run |
Vite Task forwards task output to its own stdout. In GitHub Actions that
stdout is a pipe, and the Node processes vp spawns for tasks set
O_NONBLOCK on the shared file description while they run (Node does this
to every pipe it touches; vp clears the flag at startup but its children
re-set it). When a large burst of forwarded output lands while such a
child is alive, the pipe fills and the write fails with
✗ Failed to forward task process output: Resource temporarily
unavailable (os error 11)
and vp records the task as failed even though it finished. Six "Test
packages" shards across three CI runs on 2026-09-09 died this way, under
pnpm 11 and pnpm 12 alike, with every vitest suite passing. Nothing about
the step's exit code distinguishes this from a real test failure.
The release job in cd.yml builds every package the same way before
`pnpm -r publish`. Its build has survived cold so far, but release-please
has already tagged the release by the time that step runs, and a failure
there leaves nothing on npm.
Route the vp-driven steps in ci.yml and cd.yml through a small wrapper
that sends their output to a regular file, then prints the file. Node only
makes pipes non-blocking, and writes to a regular file never return
EAGAIN. Exit status is preserved. The cost is that step output appears
when the step ends rather than streaming.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JZ5RTqfnRMiiASH1T7tWVj
decepulis
force-pushed
the
claude/rc-release-readiness-qaqw5t
branch
from
September 9, 2026 22:45
9ee5eaf to
47342f7
Compare
decepulis
changed the title
decepulis
marked this pull request as ready for review
September 9, 2026 22:51
sampotts
approved these changes
Sep 9, 2026
Collaborator
Author
|
Hold off merging for a few minutes: the head commit (c660ef2, "DO NOT MERGE") is a temporary experiment that deletes the Vite task cache before the test steps, so this run exercises the wrapper on the cold path that failed three of three cold runs today. I'll drop that commit once the run finishes and post the result here. Generated by Claude Code |
decepulis
force-pushed
the
claude/rc-release-readiness-qaqw5t
branch
from
September 9, 2026 23:02
c660ef2 to
47342f7
Compare
Collaborator
Author
|
Experiment done, commit dropped; head is back to 47342f7 (the reviewed change, nothing else). Result of run 34414800964 with the task cache deleted before each test step, so every shard ran the fully cold path through the wrapper:
Shards 2/4 and 4/4 are the two that failed in all three cold runs earlier today without the wrapper. One cold pass through the wrapper is consistent with the mechanism, not proof against a race, so I'd still call this hardening rather than a verified fix. Safe to merge. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Six
Test packagesshards across three CI runs on 2026-09-09 failed with no failing test:Runs: 34411239444 (release branch, pnpm 12), 34412176604 (pnpm 11), 34406370628 (
chore/pnpm-12). Every vitest suite in those jobs passed; vp gave up while children were still printing passing results.Mechanism, verified by two independent reviews of the logs and of
vite-plus@0.2.8:SavedExecutionError::ForwardTaskProcessOutputin the Rust core, a failed write of forwarded output to vp's own stdout. Not a read.ensureBlockingStdio()at startup, but the Node processes it spawns for tasks setO_NONBLOCKon the shared stdout file description while they run (observed by sampling/proc/<pid>/fdinfo/1during a realvp run). Node does this to any pipe it touches, never to a regular file.vp packdumping ~1,400dist/...lines in ~60 ms) while such a child is alive fills the pipe; the write returns EAGAIN; vp marks the task failed.Release exposure: cd.yml's
releasejob builds every package with vp beforepnpm -r publish, and release-please has already tagged the release by then. A re-run would find nothing to release and skip publish. That build has passed cold twice (rc.1 and today), so the risk is probabilistic, not observed.What
.github/scripts/run-logged.sh <name> <command...>: runs the command with stdout and stderr in$RUNNER_TEMP/<name>.log, prints the log, exits with the command's status.clean,build:packages,build:cdn,build:site,build:clirun through it.Test packagesandTest SPFrun through it. These are the steps that actually failed.Cost: step output appears when the step ends instead of streaming, and stdout/stderr are merged.
Verified
actionlintpasses on both workflows; script is100755.O_NONBLOCKclear.node flags.js | catreportsO_NONBLOCK=true;node flags.js > out.txtreportsO_NONBLOCK=false.Follow-ups
EAGAINor re-clearO_NONBLOCKwhile forwarding.vp run --last-detailsin a failure path so failed tasks are attributable from the log.🤖 Generated with Claude Code
https://claude.ai/code/session_01JZ5RTqfnRMiiASH1T7tWVj
Note
Low Risk
CI/CD workflow and helper-script changes only; no application runtime behavior, aside from non-streaming GitHub Actions logs.
Overview
Adds
.github/scripts/run-logged.shsovpand relatedpnpmtasks write stdout/stderr to a temp log file (avoiding pipeEAGAINwhen Node sets non-blocking mode on forwarded output), then replay the log and preserve the exit code.CI routes
Test packagesand Test SPF through the wrapper; CD routes releaseclean, package/CDN/site/CLI builds the same way. Step logs show up when the step finishes and streams are merged.Also adds temporary “Drop task cache (experiment, do not merge)” steps in
ci.ymlthat deletenode_modules/.vite/task-cachebefore those tests—likely for debugging and probably should be removed before merge.Reviewed by Cursor Bugbot for commit c660ef2. Bugbot is set up for automated code reviews on this repo. Configure here.