Skip to content

Commit 393ef10

Browse files
SteveSandersonMSCopilot
SteveSandersonMS
and
Copilot
committed
Add CLI error reporting tests and fixes for all SDKs
Node.js: - Add stderrBuffer and processExitPromise for early failure detection - Race verifyProtocolVersion against process exit - Add test: should report error with stderr when CLI fails to start Python: - Add cli_args option to CopilotClientOptions - Add ProcessExitedError and stderr capture in jsonrpc - Add test: test_should_report_error_with_stderr_when_cli_fails_to_start .NET: - Add StderrBuffer to capture CLI stderr output - Handle ConnectionLostException with stderr in error message - Add test: Should_Report_Error_With_Stderr_When_CLI_Fails_To_Start Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 57535b4 commit 393ef10

File tree

8 files changed

+266
-17
lines changed

8 files changed

+266
-17
lines changed

‎dotnet/src/Client.cs‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using System.Diagnostics;
1212
using System.Diagnostics.CodeAnalysis;
1313
using System.Net.Sockets;
14+
using System.Text;
1415
using System.Text.Json;
1516
using System.Text.Json.Serialization;
1617
using System.Text.RegularExpressions;
@@ -183,13 +184,13 @@ async Task<Connection> StartCoreAsync(CancellationToken ct)
183184
if (_optionsHost is not null && _optionsPort is not null)
184185
{
185186
// External server (TCP)
186-
result = ConnectToServerAsync(null, _optionsHost, _optionsPort, ct);
187+
result = ConnectToServerAsync(null, _optionsHost, _optionsPort, null, ct);
187188
}
188189
else
189190
{
190191
// Child process (stdio or TCP)
191-
var (cliProcess, portOrNull) = await StartCliServerAsync(_options, _logger, ct);
192-
result = ConnectToServerAsync(cliProcess, portOrNull is null ? null : "localhost", portOrNull, ct);
192+
var (cliProcess, portOrNull, stderrBuffer) = await StartCliServerAsync(_options, _logger, ct);
193+
result = ConnectToServerAsync(cliProcess, portOrNull is null ? null : "localhost", portOrNull, stderrBuffer, ct);
193194
}
194195

195196
var connection = await result;
@@ -842,11 +843,33 @@ private void DispatchLifecycleEvent(SessionLifecycleEvent evt)
842843
}
843844

844845
internal static async Task<T> InvokeRpcAsync<T>(JsonRpc rpc, string method, object?[]? args, CancellationToken cancellationToken)
846+
{
847+
return await InvokeRpcAsync<T>(rpc, method, args, null, cancellationToken);
848+
}
849+
850+
internal static async Task<T> InvokeRpcAsync<T>(JsonRpc rpc, string method, object?[]? args, StringBuilder? stderrBuffer, CancellationToken cancellationToken)
845851
{
846852
try
847853
{
848854
return await rpc.InvokeWithCancellationAsync<T>(method, args, cancellationToken);
849855
}
856+
catch (StreamJsonRpc.ConnectionLostException ex)
857+
{
858+
string? stderrOutput = null;
859+
if (stderrBuffer is not null)
860+
{
861+
lock (stderrBuffer)
862+
{
863+
stderrOutput = stderrBuffer.ToString().Trim();
864+
}
865+
}
866+
867+
if (!string.IsNullOrEmpty(stderrOutput))
868+
{
869+
throw new IOException($"CLI process exited unexpectedly.\nstderr: {stderrOutput}", ex);
870+
}
871+
throw new IOException($"Communication error with Copilot CLI: {ex.Message}", ex);
872+
}
850873
catch (StreamJsonRpc.RemoteRpcException ex)
851874
{
852875
throw new IOException($"Communication error with Copilot CLI: {ex.Message}", ex);
@@ -868,7 +891,7 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
868891
{
869892
var expectedVersion = SdkProtocolVersion.GetVersion();
870893
var pingResponse = await InvokeRpcAsync<PingResponse>(
871-
connection.Rpc, "ping", [new PingRequest()], cancellationToken);
894+
connection.Rpc, "ping", [new PingRequest()], connection.StderrBuffer, cancellationToken);
872895

873896
if (!pingResponse.ProtocolVersion.HasValue)
874897
{
@@ -887,7 +910,7 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
887910
}
888911
}
889912

890-
private static async Task<(Process Process, int? DetectedLocalhostTcpPort)> StartCliServerAsync(CopilotClientOptions options, ILogger logger, CancellationToken cancellationToken)
913+
private static async Task<(Process Process, int? DetectedLocalhostTcpPort, StringBuilder StderrBuffer)> StartCliServerAsync(CopilotClientOptions options, ILogger logger, CancellationToken cancellationToken)
891914
{
892915
// Use explicit path or bundled CLI - no PATH fallback
893916
var cliPath = options.CliPath ?? GetBundledCliPath(out var searchedPath)
@@ -957,14 +980,19 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
957980
var cliProcess = new Process { StartInfo = startInfo };
958981
cliProcess.Start();
959982

960-
// Forward stderr to logger
983+
// Capture stderr for error messages and forward to logger
984+
var stderrBuffer = new StringBuilder();
961985
_ = Task.Run(async () =>
962986
{
963987
while (cliProcess != null && !cliProcess.HasExited)
964988
{
965989
var line = await cliProcess.StandardError.ReadLineAsync(cancellationToken);
966990
if (line != null)
967991
{
992+
lock (stderrBuffer)
993+
{
994+
stderrBuffer.AppendLine(line);
995+
}
968996
logger.LogDebug("[CLI] {Line}", line);
969997
}
970998
}
@@ -991,7 +1019,7 @@ private async Task VerifyProtocolVersionAsync(Connection connection, Cancellatio
9911019
}
9921020
}
9931021

994-
return (cliProcess, detectedLocalhostTcpPort);
1022+
return (cliProcess, detectedLocalhostTcpPort, stderrBuffer);
9951023
}
9961024

9971025
private static string? GetBundledCliPath(out string searchedPath)
@@ -1035,7 +1063,7 @@ private static (string FileName, IEnumerable<string> Args) ResolveCliCommand(str
10351063
return (cliPath, args);
10361064
}
10371065

1038-
private async Task<Connection> ConnectToServerAsync(Process? cliProcess, string? tcpHost, int? tcpPort, CancellationToken cancellationToken)
1066+
private async Task<Connection> ConnectToServerAsync(Process? cliProcess, string? tcpHost, int? tcpPort, StringBuilder? stderrBuffer, CancellationToken cancellationToken)
10391067
{
10401068
Stream inputStream, outputStream;
10411069
TcpClient? tcpClient = null;
@@ -1080,7 +1108,7 @@ private async Task<Connection> ConnectToServerAsync(Process? cliProcess, string?
10801108

10811109
_rpc = new ServerRpc(rpc);
10821110

1083-
return new Connection(rpc, cliProcess, tcpClient, networkStream);
1111+
return new Connection(rpc, cliProcess, tcpClient, networkStream, stderrBuffer);
10841112
}
10851113

10861114
[UnconditionalSuppressMessage("Trimming", "IL2026", Justification = "Using happy path from https://microsoft.github.io/vs-streamjsonrpc/docs/nativeAOT.html")]
@@ -1321,12 +1349,14 @@ private class Connection(
13211349
JsonRpc rpc,
13221350
Process? cliProcess, // Set if we created the child process
13231351
TcpClient? tcpClient, // Set if using TCP
1324-
NetworkStream? networkStream) // Set if using TCP
1352+
NetworkStream? networkStream, // Set if using TCP
1353+
StringBuilder? stderrBuffer = null) // Captures stderr for error messages
13251354
{
13261355
public Process? CliProcess => cliProcess;
13271356
public TcpClient? TcpClient => tcpClient;
13281357
public JsonRpc Rpc => rpc;
13291358
public NetworkStream? NetworkStream => networkStream;
1359+
public StringBuilder? StderrBuffer => stderrBuffer;
13301360
}
13311361

13321362
private static class ProcessArgumentEscaper

‎dotnet/test/ClientTests.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,4 +224,27 @@ public async Task Should_Not_Throw_When_Disposing_Session_After_Stopping_Client(
224224

225225
await client.StopAsync();
226226
}
227+
228+
[Fact]
229+
public async Task Should_Report_Error_With_Stderr_When_CLI_Fails_To_Start()
230+
{
231+
var client = new CopilotClient(new CopilotClientOptions
232+
{
233+
CliArgs = new[] { "--nonexistent-flag-for-testing" },
234+
UseStdio = true
235+
});
236+
237+
var ex = await Assert.ThrowsAsync<IOException>(async () =>
238+
{
239+
await client.StartAsync();
240+
});
241+
242+
var errorMessage = ex.Message;
243+
// Verify we get the stderr output in the error message
244+
Assert.Contains("stderr", errorMessage, StringComparison.OrdinalIgnoreCase);
245+
Assert.Contains("nonexistent", errorMessage, StringComparison.OrdinalIgnoreCase);
246+
247+
// Cleanup - ForceStop should handle the disconnected state gracefully
248+
try { await client.ForceStopAsync(); } catch { /* Expected */ }
249+
}
227250
}

‎nodejs/src/client.ts‎

Lines changed: 63 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ export class CopilotClient {
128128
private actualHost: string = "localhost";
129129
private state: ConnectionState = "disconnected";
130130
private sessions: Map<string, CopilotSession> = new Map();
131+
private stderrBuffer: string = ""; // Captures CLI stderr for error messages
131132
private options: Required<
132133
Omit<CopilotClientOptions, "cliUrl" | "githubToken" | "useLoggedInUser">
133134
> & {
@@ -145,6 +146,7 @@ export class CopilotClient {
145146
Set<(event: SessionLifecycleEvent) => void>
146147
> = new Map();
147148
private _rpc: ReturnType<typeof createServerRpc> | null = null;
149+
private processExitPromise: Promise<never> | null = null; // Rejects when CLI process exits
148150

149151
/**
150152
* Typed server-scoped RPC methods.
@@ -395,6 +397,8 @@ export class CopilotClient {
395397

396398
this.state = "disconnected";
397399
this.actualPort = null;
400+
this.stderrBuffer = "";
401+
this.processExitPromise = null;
398402

399403
return errors;
400404
}
@@ -465,6 +469,8 @@ export class CopilotClient {
465469

466470
this.state = "disconnected";
467471
this.actualPort = null;
472+
this.stderrBuffer = "";
473+
this.processExitPromise = null;
468474
}
469475

470476
/**
@@ -746,7 +752,15 @@ export class CopilotClient {
746752
*/
747753
private async verifyProtocolVersion(): Promise<void> {
748754
const expectedVersion = getSdkProtocolVersion();
749-
const pingResult = await this.ping();
755+
756+
// Race ping against process exit to detect early CLI failures
757+
let pingResult: Awaited<ReturnType<typeof this.ping>>;
758+
if (this.processExitPromise) {
759+
pingResult = await Promise.race([this.ping(), this.processExitPromise]);
760+
} else {
761+
pingResult = await this.ping();
762+
}
763+
750764
const serverVersion = pingResult.protocolVersion;
751765

752766
if (serverVersion === undefined) {
@@ -1002,6 +1016,9 @@ export class CopilotClient {
10021016
*/
10031017
private async startCLIServer(): Promise<void> {
10041018
return new Promise((resolve, reject) => {
1019+
// Clear stderr buffer for fresh capture
1020+
this.stderrBuffer = "";
1021+
10051022
const args = [
10061023
...this.options.cliArgs,
10071024
"--headless",
@@ -1085,6 +1102,8 @@ export class CopilotClient {
10851102
}
10861103

10871104
this.cliProcess.stderr?.on("data", (data: Buffer) => {
1105+
// Capture stderr for error messages
1106+
this.stderrBuffer += data.toString();
10881107
// Forward CLI stderr to parent's stderr so debug logs are visible
10891108
const lines = data.toString().split("\n");
10901109
for (const line of lines) {
@@ -1097,14 +1116,55 @@ export class CopilotClient {
10971116
this.cliProcess.on("error", (error) => {
10981117
if (!resolved) {
10991118
resolved = true;
1100-
reject(new Error(`Failed to start CLI server: ${error.message}`));
1119+
const stderrOutput = this.stderrBuffer.trim();
1120+
if (stderrOutput) {
1121+
reject(
1122+
new Error(
1123+
`Failed to start CLI server: ${error.message}\nstderr: ${stderrOutput}`
1124+
)
1125+
);
1126+
} else {
1127+
reject(new Error(`Failed to start CLI server: ${error.message}`));
1128+
}
11011129
}
11021130
});
11031131

1132+
// Set up a promise that rejects when the process exits (used to race against RPC calls)
1133+
this.processExitPromise = new Promise<never>((_, rejectProcessExit) => {
1134+
this.cliProcess!.on("exit", (code) => {
1135+
// Give a small delay for stderr to be fully captured
1136+
setTimeout(() => {
1137+
const stderrOutput = this.stderrBuffer.trim();
1138+
if (stderrOutput) {
1139+
rejectProcessExit(
1140+
new Error(
1141+
`CLI server exited with code ${code}\nstderr: ${stderrOutput}`
1142+
)
1143+
);
1144+
} else {
1145+
rejectProcessExit(
1146+
new Error(`CLI server exited unexpectedly with code ${code}`)
1147+
);
1148+
}
1149+
}, 50);
1150+
});
1151+
});
1152+
// Prevent unhandled rejection when process exits normally (we only use this in Promise.race)
1153+
this.processExitPromise.catch(() => {});
1154+
11041155
this.cliProcess.on("exit", (code) => {
11051156
if (!resolved) {
11061157
resolved = true;
1107-
reject(new Error(`CLI server exited with code ${code}`));
1158+
const stderrOutput = this.stderrBuffer.trim();
1159+
if (stderrOutput) {
1160+
reject(
1161+
new Error(
1162+
`CLI server exited with code ${code}\nstderr: ${stderrOutput}`
1163+
)
1164+
);
1165+
} else {
1166+
reject(new Error(`CLI server exited with code ${code}`));
1167+
}
11081168
} else if (this.options.autoRestart && this.state === "connected") {
11091169
void this.reconnect();
11101170
}

‎nodejs/test/e2e/client.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,21 @@ describe("Client", () => {
132132

133133
await client.stop();
134134
});
135+
136+
it("should report error with stderr when CLI fails to start", async () => {
137+
const client = new CopilotClient({
138+
cliArgs: ["--nonexistent-flag-for-testing"],
139+
useStdio: true,
140+
});
141+
onTestFinishedForceStop(client);
142+
143+
try {
144+
await client.start();
145+
expect.fail("Expected start() to throw an error");
146+
} catch (error) {
147+
const message = (error as Error).message;
148+
expect(message).toContain("stderr");
149+
expect(message).toContain("nonexistent");
150+
}
151+
});
135152
});

‎python/copilot/client.py‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525

2626
from .generated.rpc import ServerRpc
2727
from .generated.session_events import session_event_from_dict
28-
from .jsonrpc import JsonRpcClient
28+
from .jsonrpc import JsonRpcClient, ProcessExitedError
2929
from .sdk_protocol_version import get_sdk_protocol_version
3030
from .session import CopilotSession
3131
from .types import (
@@ -185,6 +185,8 @@ def __init__(self, options: Optional[CopilotClientOptions] = None):
185185
"auto_restart": opts.get("auto_restart", True),
186186
"use_logged_in_user": use_logged_in_user,
187187
}
188+
if opts.get("cli_args"):
189+
self.options["cli_args"] = opts["cli_args"]
188190
if opts.get("cli_url"):
189191
self.options["cli_url"] = opts["cli_url"]
190192
if opts.get("env"):
@@ -292,8 +294,21 @@ async def start(self) -> None:
292294
await self._verify_protocol_version()
293295

294296
self._state = "connected"
295-
except Exception:
297+
except ProcessExitedError as e:
298+
# Process exited with error - reraise as RuntimeError with stderr
296299
self._state = "error"
300+
raise RuntimeError(str(e)) from None
301+
except Exception as e:
302+
self._state = "error"
303+
# Check if process exited and capture any remaining stderr
304+
if self._process and hasattr(self._process, "poll"):
305+
return_code = self._process.poll()
306+
if return_code is not None and self._client:
307+
stderr_output = self._client.get_stderr_output()
308+
if stderr_output:
309+
raise RuntimeError(
310+
f"CLI process exited with code {return_code}\nstderr: {stderr_output}"
311+
) from e
297312
raise
298313

299314
async def stop(self) -> list["StopError"]:
@@ -1141,7 +1156,14 @@ async def _start_cli_server(self) -> None:
11411156
if not os.path.exists(cli_path):
11421157
raise RuntimeError(f"Copilot CLI not found at {cli_path}")
11431158

1144-
args = ["--headless", "--no-auto-update", "--log-level", self.options["log_level"]]
1159+
# Start with user-provided cli_args, then add SDK-managed args
1160+
cli_args = self.options.get("cli_args") or []
1161+
args = list(cli_args) + [
1162+
"--headless",
1163+
"--no-auto-update",
1164+
"--log-level",
1165+
self.options["log_level"],
1166+
]
11451167

11461168
# Add auth-related flags
11471169
if self.options.get("github_token"):

0 commit comments

Comments
 (0)