Skip to content

fix: include stderr in process exceptions#212

Merged
AlexandrSuhinin merged 1 commit into
mainfrom
suhinin/exceptions-with-stderr
Jun 18, 2026
Merged

fix: include stderr in process exceptions#212
AlexandrSuhinin merged 1 commit into
mainfrom
suhinin/exceptions-with-stderr

Conversation

@AlexandrSuhinin

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread src/CodexAcpServer.ts
this.codexAcpClient = codexAcpClient;
this.defaultAuthRequest = defaultAuthRequest ?? null;
this.getExitCode = getExitCode ?? (() => null);
this.getRecentStderr = getRecentStderr ?? (() => "");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lastError instead of recent maybe?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's not actually last, it's all stderr limited to some size.
I'm not sure we can actually get single error - it still can be spitted into a few data events.

import { createMockConnections } from './test-utils';

describe('CodexACPAgent - process exit error', () => {
it.skipIf(process.platform === 'win32')('includes the crashed process stderr', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe add win case also? :)

@AlexandrSuhinin AlexandrSuhinin Jun 18, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We don't have CI on win, also the change is not OS specific. I suggest to keep it as is, otherwise I need to blindly add win case support without actual checking or spend unreasonable time to check it.

@AlexandrSuhinin AlexandrSuhinin merged commit f7d1125 into main Jun 18, 2026
4 checks passed
@AlexandrSuhinin AlexandrSuhinin deleted the suhinin/exceptions-with-stderr branch June 18, 2026 19:38
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.

2 participants