Command failure tolerance - #114
Conversation
Commands like exit that cause the shell to close should not produce errors on subsequent write or resize calls. Instead of throwing when the terminal is not running, silently drop the operation. Also wrap the raw PTY write/resize calls in try/catch to handle the race where the process exits between the status check and the call. Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| } | ||
| session.cols = input.cols; | ||
| session.rows = input.rows; | ||
| session.updatedAt = new Date().toISOString(); | ||
| session.process.resize(input.cols, input.rows); | ||
| try { | ||
| session.process.resize(input.cols, input.rows); | ||
| } catch { | ||
| // Process may have exited between the status check and the resize call. |
There was a problem hiding this comment.
🟢 Low Layers/Manager.ts:455
Consider moving session.cols and session.rows assignments inside the try block, after session.process.resize() succeeds. Currently, if resize() throws, the session state will reflect dimensions the PTY never adopted.
- session.cols = input.cols;
- session.rows = input.rows;
- session.updatedAt = new Date().toISOString();
try {
session.process.resize(input.cols, input.rows);
+ session.cols = input.cols;
+ session.rows = input.rows;
+ session.updatedAt = new Date().toISOString();
} catch {🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file apps/server/src/terminal/Layers/Manager.ts around lines 455-462:
Consider moving `session.cols` and `session.rows` assignments inside the `try` block, after `session.process.resize()` succeeds. Currently, if `resize()` throws, the session state will reflect dimensions the PTY never adopted.
Evidence trail:
apps/server/src/terminal/Layers/Manager.ts lines 450-464 at REVIEWED_COMMIT: The `resize` method shows `session.cols = input.cols` (line 456) and `session.rows = input.rows` (line 457) are assigned BEFORE the `try` block. The `session.process.resize(input.cols, input.rows)` call is inside the `try` block (line 460). The catch block comment confirms the known failure mode: "Process may have exited between the status check and the resize call."
Make terminal
writeandresizetolerant of exited processes to prevent errors when a terminal closes.The terminal's
writeandresizemethods were throwing errors when called on a process that had already exited (e.g., after anexitcommand), causing error messages to appear in the UI. This change makes these operations silently return or catch errors, allowing the terminal to close gracefully without user-facing errors.Note
Make
TerminalManagerRuntime.writeandTerminalManagerRuntime.resizetolerate command failures in Manager.ts by returning early when the terminal is missing or exitedUpdate terminal runtime methods to return without throwing when the session is absent or exited and wrap process calls in try/catch; add tests asserting resolved promises on exited sessions in Manager.test.ts.
📍Where to Start
Start with the
TerminalManagerRuntime.writeandTerminalManagerRuntime.resizeimplementations in Manager.ts.📊 Macroscope summarized 25ea66c. 1 file reviewed, 2 issues evaluated, 0 issues filtered, 1 comment posted
🗂️ Filtered Issues