Skip to content

fix(cli): handle stdout backpressure#2169

Closed
hyoban wants to merge 5 commits into
voidzero-dev:mainfrom
hyoban:0714-stdout-backpressure
Closed

fix(cli): handle stdout backpressure#2169
hyoban wants to merge 5 commits into
voidzero-dev:mainfrom
hyoban:0714-stdout-backpressure

Conversation

@hyoban

@hyoban hyoban commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • retry partial stdout writes when a non-blocking output stream applies backpressure
  • wait for Unix file descriptors with poll(POLLOUT) and propagate non-retryable I/O errors
  • make vp check output helpers return write failures instead of discarding them or panicking
  • cover short writes, WouldBlock, interrupted writes, terminal errors, and a real non-blocking Unix stream

Root cause

vp check captures Oxlint diagnostics and replays them to stdout. When a consumer could not drain a non-blocking stdout pipe quickly enough, write_all returned WouldBlock. The diagnostic replay path discarded that error, so the diagnostics were truncated and a later standard-library print could panic while stdout was still unavailable.

The new write loop preserves the unwritten suffix, waits until the stream is writable, and retries. Permanent failures are returned through the existing command result path.

Fixes #2165.

Validation

  • cargo test -p vite_shared waits_until_a_nonblocking_writer_is_ready_before_retrying
  • just check
  • just lint
  • just test
  • pnpm -F vite-plus build
  • just snapshot-test check_ (36 passed)

@netlify

netlify Bot commented Jul 14, 2026

Copy link
Copy Markdown

Deploy Preview for viteplus-preview canceled.

Name Link
🔨 Latest commit fa6a005
🔍 Latest deploy log https://app.netlify.com/projects/viteplus-preview/deploys/6a56058048c7fb00080b3d53

@hyoban

hyoban commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

CI update:

The Linux/macOS failures are unrelated snapshot drift: a run-day install date (already addressed by #2150) and newer package versions resolved from the registry.

This PR does not touch those paths, so I have not included unrelated snapshot updates here.

@hyoban hyoban marked this pull request as ready for review July 14, 2026 02:42
@hyoban

hyoban commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 1458b01027

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@liangmiQwQ liangmiQwQ requested a review from cpojer July 14, 2026 05:00
@cpojer cpojer requested review from jong-kyung and wan9chi and removed request for cpojer July 14, 2026 05:08

@jong-kyung jong-kyung left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 😁

@jong-kyung jong-kyung requested a review from liangmiQwQ July 14, 2026 06:08

@liangmiQwQ liangmiQwQ left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@wan9chi

wan9chi commented Jul 15, 2026

Copy link
Copy Markdown
Member

Thank you for tracking this down and putting together a fix! This PR correctly identifies the underlying issue.

However the same problem can affect any code that writes to stdout or stderr in the Vite+ process, including vp check and vite-task. Because vite-task is a Cargo dependency, we cannot patch its write sites in this repository. Building on your findings, I’m fixing it in a more general way in #2173 to cover all writers.

@wan9chi wan9chi closed this Jul 15, 2026
graphite-app Bot pushed a commit that referenced this pull request Jul 15, 2026
Fixes #2165.

## Root cause

Node marks non-TTY stdio as non-blocking, and the flag is shared with inherited child descriptors. Rust's standard I/O and embedded dependencies assume blocking writes, so a full consumer pipe can return `EAGAIN`, truncate diagnostics, or panic.

## Why this approach

#2169 retries writes in code we control. Dependencies like vite-task write directly to the same stdio, so those retries cannot protect them. Clearing `O_NONBLOCK` at startup covers every writer.
wan9chi added a commit that referenced this pull request Jul 15, 2026
Fixes #2165.

## Root cause

Node marks non-TTY stdio as non-blocking, and the flag is shared with inherited child descriptors. Rust's standard I/O and embedded dependencies assume blocking writes, so a full consumer pipe can return `EAGAIN`, truncate diagnostics, or panic.

## Why this approach

#2169 retries writes in code we control. Dependencies like vite-task write directly to the same stdio, so those retries cannot protect them. Clearing `O_NONBLOCK` at startup covers every writer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vp check panics on EAGAIN while replaying large lint diagnostics

4 participants