Skip to content

fix(pty): suppress benign EPIPE errors on session shutdown#1684

Open
Abdel-E wants to merge 1 commit intogeneralaction:mainfrom
Abdel-E:fix/main-process-epipe-on-session-close
Open

fix(pty): suppress benign EPIPE errors on session shutdown#1684
Abdel-E wants to merge 1 commit intogeneralaction:mainfrom
Abdel-E:fix/main-process-epipe-on-session-close

Conversation

@Abdel-E
Copy link
Copy Markdown

@Abdel-E Abdel-E commented Apr 7, 2026

Handle stdio EPIPE/EIO/ECONNRESET in lifecycle fallback and attach PTY pipe suppression on all platforms so closing agent sessions does not throw uncaught main-process errors.

Summary

Closing agent sessions can tear down PTYs/stdio pipes while Node is still reading from them, which can surface as an uncaught main-process exception (Error: read EPIPE, Pipe.onStreamRead). This PR:

  • Ignores benign shutdown errors (EPIPE/EIO/ECONNRESET) on lifecycle fallback child_process stdio streams
  • Attaches PTY pipe error suppression on all platforms (not only Windows)
  • Adds/updates tests to cover these shutdown paths

Fixes

Snapshot

N/A — runtime stability fix (no UI change)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (does not change functionality, e.g. code style improvements, linting)
  • This change requires a documentation update

Mandatory Tasks

  • I have self-reviewed the code

Checklist

  • I have read the contributing guide
  • I have commented my code, particularly in hard-to-understand areas
  • I have checked if my PR needs changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

Summary by CodeRabbit

  • Bug Fixes
    • Pseudoterminal error suppression now works consistently across all platforms, extending functionality that was previously limited to Windows
    • Stream error detection has been improved to identify and properly handle benign shutdown-related errors without propagating them
    • Process lifecycle error handling has been refined to prevent unnecessary error notifications and improve overall system stability

Handle stdio EPIPE/EIO/ECONNRESET in lifecycle fallback and attach PTY pipe suppression on all platforms so closing agent sessions does not throw uncaught main-process errors.

Made-with: Cursor
@vercel
Copy link
Copy Markdown

vercel bot commented Apr 7, 2026

@Abdel-E is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 7, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c1b38bf6-559f-4070-98d4-16de1b9b0930

📥 Commits

Reviewing files that changed from the base of the PR and between c797ed5 and a48ac34.

📒 Files selected for processing (2)
  • src/main/services/ptyManager.ts
  • src/test/main/ptyManager.test.ts

📝 Walkthrough

Walkthrough

Updates PTY and child process stream error handling to suppress benign shutdown-related errors (EPIPE, EIO, ECONNRESET) across all platforms. Extends error suppression from Windows-only to universal platform coverage and adds error propagation logic for non-benign errors with proper UTF-8 buffering.

Changes

Cohort / File(s) Summary
PTY Manager Implementation
src/main/services/ptyManager.ts
Removed platform guard from suppressPtyPipeErrors() to apply error suppression universally; added isBenignStreamShutdownError() helper to classify EPIPE/EIO/ECONNRESET errors; extended LifecycleSpawnFallbackChild type to support error listeners on stdout/stderr; implemented error handlers that suppress benign errors but propagate others with buffered output flushing.
PTY Manager Tests
src/test/main/ptyManager.test.ts
Updated PTY error suppression test to verify handler attachment on all platforms (not just Windows); added new test case validating benign EPIPE error suppression in lifecycle spawn fallback handlers without triggering the error callback.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Poem

🐰 Pipes were breaking, streams did cry,
EPIPE errors made processes die,
Now benign shutdowns gently rest,
While true errors still raise their test,
Hopping forth with buffered grace!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(pty): suppress benign EPIPE errors on session shutdown' directly and accurately summarizes the main change: suppressing EPIPE errors that occur during PTY session shutdown across platforms.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant