Skip to content

Go: include CLI subprocess stderr output in error messages#606

Open
chlowell wants to merge 2 commits intogithub:mainfrom
chlowell:go-stderr
Open

Go: include CLI subprocess stderr output in error messages#606
chlowell wants to merge 2 commits intogithub:mainfrom
chlowell:go-stderr

Conversation

@chlowell
Copy link
Contributor

When the CLI subprocess fails (crash, bad args, etc.), the Go SDK returns a generic error that doesn't explain why the CLI exited. With this PR, the SDK appends the subprocess's stderr to these errors:

 process exited unexpectedly
 failed to kill CLI process: os: process already finished
+stderr: something went wrong

The implementation is a little complex because of the synchronization required to prevent races and leaks, and enable a client instance to start a new subprocess while concurrently cleaning up after an old one. The high-level gist is that the client uses a ring buffer (capped at 64 KB) to track stderr and drains that into an error message when the subprocess exits.

@chlowell chlowell marked this pull request as ready for review February 27, 2026 22:17
@chlowell chlowell requested a review from a team as a code owner February 27, 2026 22:17
Copilot AI review requested due to automatic review settings February 27, 2026 22:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves error messages in the Go SDK by capturing the CLI subprocess's stderr output and appending it to errors when the subprocess exits unexpectedly. A goroutine drains stderr into a ring buffer (capped at 64 KB), and the buffer contents are appended to error messages when the process fails.

Changes:

  • Adds a stderr drain goroutine (startStderrDrain) and a 64 KB ring buffer (stderrBuf) to the Client struct, protected by stderrBufMux and synchronized via a stderrDone channel.
  • Updates killProcess, monitorProcess, and all error paths in startCLIServer and Start to wait for the drain goroutine and include stderr in error messages.
  • Adds TestClient_StderrCapture with three subtests covering startup failure capture, buffer capping, and buffer clearing on stop.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
go/client.go Core implementation: stderrBuf/stderrBufMux/stderrDone fields, startStderrDrain, getStderrOutput, stderrError, updated killProcess/monitorProcess/startCLIServer/Start/Stop/ForceStop
go/client_test.go New TestClient_StderrCapture test: builds a Go fixture binary that writes to stderr and exits, then verifies capture, capping, and clearing behavior

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