Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 45 additions & 9 deletions dotnet/src/Client.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,20 @@ async Task<Connection> StartCoreAsync(CancellationToken ct)

var connection = await result;

// Verify protocol version compatibility
await VerifyProtocolVersionAsync(connection, ct);
await ConfigureSessionFsAsync(ct);
try
{
// Verify protocol version compatibility
await VerifyProtocolVersionAsync(connection, ct);
await ConfigureSessionFsAsync(ct);

_logger.LogInformation("Copilot client connected");
return connection;
_logger.LogInformation("Copilot client connected");
return connection;
}
catch
{
await CleanupConnectionAsync(connection, errors: null);
throw;
}
}
}

Expand Down Expand Up @@ -353,11 +361,27 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
return;
}

var ctx = await _connectionTask;
var connectionTask = _connectionTask;
_connectionTask = null;

Connection ctx;
try
{
ctx = await connectionTask;
}
catch (Exception ex)
{
_logger.LogDebug(ex, "Ignoring failed Copilot client startup during cleanup");
return;
}
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
Comment thread
stephentoub marked this conversation as resolved.

await CleanupConnectionAsync(ctx, errors);
Comment thread
stephentoub marked this conversation as resolved.
}

private async Task CleanupConnectionAsync(Connection ctx, List<Exception>? errors)
{
try { ctx.Rpc.Dispose(); }
catch (Exception ex) { errors?.Add(ex); }
catch (Exception ex) { AddCleanupError(errors, ex); }
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed

// Clear RPC and models cache
_serverRpc = null;
Expand All @@ -366,7 +390,7 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
if (ctx.NetworkStream is not null)
{
try { await ctx.NetworkStream.DisposeAsync(); }
catch (Exception ex) { errors?.Add(ex); }
catch (Exception ex) { AddCleanupError(errors, ex); }
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}

if (ctx.CliProcess is { } childProcess)
Expand All @@ -380,7 +404,19 @@ private async Task CleanupConnectionAsync(List<Exception>? errors)
}
childProcess.Dispose();
}
catch (Exception ex) { errors?.Add(ex); }
catch (Exception ex) { AddCleanupError(errors, ex); }
Comment thread
github-code-quality[bot] marked this conversation as resolved.
Fixed
}
Comment thread
stephentoub marked this conversation as resolved.
}

private void AddCleanupError(List<Exception>? errors, Exception ex)
{
if (errors is not null)
{
errors.Add(ex);
}
else
{
_logger.LogDebug(ex, "Error while cleaning up Copilot CLI connection");
}
}

Expand Down
40 changes: 40 additions & 0 deletions dotnet/test/E2E/ClientOptionsE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,28 @@ public async Task Should_Propagate_Activity_TraceContext_To_Session_Create_And_S
await session.DisposeAsync();
}

[Fact]
public async Task ForceStop_Does_Not_Rethrow_When_Tcp_Cli_Drops_During_Startup()
{
var cliPath = Path.Join(Ctx.WorkDir, $"fake-tcp-drop-cli-{Guid.NewGuid():N}.js");
await File.WriteAllTextAsync(cliPath, FakeTcpDropDuringStartupCliScript);

await using var client = Ctx.CreateClient(
useStdio: false,
options: new CopilotClientOptions
{
AutoStart = false,
CliPath = cliPath,
UseLoggedInUser = false,
});

var ex = await Assert.ThrowsAsync<IOException>(() => client.StartAsync());
Assert.Contains("Communication error", ex.Message, StringComparison.Ordinal);

await client.ForceStopAsync();
Assert.Equal(ConnectionState.Disconnected, client.State);
}

[Fact]
public async Task Should_Propagate_Activity_TraceContext_To_Session_Resume()
{
Expand Down Expand Up @@ -362,6 +384,24 @@ private static JsonElement GetCapturedRequestParams(JsonElement captureRoot, str
.GetProperty("params");
}

private const string FakeTcpDropDuringStartupCliScript = """
const net = require("net");

const server = net.createServer(socket => {
socket.on("data", () => {
socket.destroy();
server.close(() => process.exit(0));
});
});

server.listen(0, "localhost", () => {
const address = server.address();
console.log(`listening on port ${address.port}`);
});

setTimeout(() => process.exit(2), 30000).unref();
""";

private const string FakeStdioCliScript = """
const fs = require("fs");

Expand Down
Loading