-
Notifications
You must be signed in to change notification settings - Fork 0
feat(process-io): harden stream error handling and add write queue timeout #160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…meout Improve ProcessIO transport resilience with: 1. Auto-restart on write failures (#107) - Add markForRestart() method to trigger process restart on next send - Call markForRestart() after stdin write errors, EPIPE, and queue failures 2. Add stdout/stderr error handlers (#91) - Add handleStdoutError() and handleStderrError() methods - Register error handlers in spawnProcess() to prevent unhandled crashes - Reject pending requests and mark for restart on stream errors 3. Add write queue timeout (#59) - Add writeQueueTimeoutMs option (default: 30000ms) - Track queuedAt timestamp for each queued write - Reject timed-out writes in flushWriteQueue() with BridgeTimeoutError Fixes #107, #91, #59 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b6dfc0e46
ℹ️ 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".
1. Fix markForRestart() no-op when restartAfterRequests=0 - Added needsRestart flag that works independently - Restart check now checks: needsRestart || scheduled restart 2. Fix write queue timeout not firing without drain event - Added createQueuedWrite() helper with timeout timer - Timer fires after writeQueueTimeoutMs even if drain never happens - Timer is unref'd so it doesn't keep process alive - Timeouts cleared properly on resolve/reject/dispose Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Addressed Codex Review FeedbackBoth issues identified by Codex have been fixed in commit 600dfac: 1.
|
Summary
Improve ProcessIO transport resilience with:
BridgeTimeoutErrorChanges
markForRestart()method to trigger process restart on next sendhandleStdoutError()andhandleStderrError()handlerswriteQueueTimeoutMsoption (default: 30000ms)queuedAttimestamp for each queued writemarkForRestart()after all write/stream error pathsTest plan
Fixes #107, #91, #59
🤖 Generated with Claude Code