Hide StreamJsonRpc implementation detail behind IOException #202

Copilot · 2026-01-26T15:06:44Z

This PR ensures that StreamJsonRpc.RemoteInvocationException is not exposed through the public API by wrapping all RPC communication errors with standard IOException.

Changes Made

Wrapper Methods: Created InvokeRpcAsync helper methods in both Session.cs and Client.cs to centralize exception handling
Exception Handling: All RPC calls now use the wrapper methods which catch StreamJsonRpc.RemoteInvocationException and throw IOException instead
Tests Updated: Changed test assertions from StreamJsonRpc.RemoteInvocationException to IOException
Documentation Updated: Updated README.md error handling examples to use IOException

Implementation Details

The wrapper methods provide a single point of exception translation:

Session.cs: Instance method that uses the session's _rpc field
Client.cs: Static method that accepts a JsonRpc parameter

This approach eliminates ~115 lines of repetitive try-catch blocks while ensuring StreamJsonRpc remains an implementation detail that doesn't leak through the public API surface.

Testing

✅ Code builds successfully with 0 warnings
✅ Code formatting passes validation
✅ No security vulnerabilities detected
✅ All tests updated to use IOException

Original prompt

This section details on the original issue you should resolve

<issue_title>StreamJsonRpc use is being surfaced via exceptions</issue_title>
<issue_description>StreamJsonRpc is supposed to be an implementation detail of the .NET implementation. But its use is surfacing via StreamJsonRpc.RemoteInvocationException exceptions that emerge, and these are being documented and tested as part of its contract. Such exceptions should be caught and replaced either by an IOException, or if it's really important to call these out as JSON-RPC issues, a custom GH copilot exception type that surfaces the relevant information.

cc: @SteveSandersonMS </issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes StreamJsonRpc use is being surfaced via exceptions #201

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

SteveSandersonMS · 2026-01-26T15:31:26Z

dotnet/src/Session.cs

+        catch (StreamJsonRpc.RemoteInvocationException ex)
+        {
+            throw new CopilotRpcException($"Failed to get messages: {ex.Message}", ex);
+        }


@copilot Don't wrap more lines in this try/catch than you have to. It's enough only to wrap the InvokeWithCancellationAsync call.

Fixed in 81d8dec. Now only the InvokeWithCancellationAsync calls are wrapped in try-catch blocks, with processing moved outside.

SteveSandersonMS · 2026-01-26T15:32:30Z

dotnet/src/Types.cs

+/// <remarks>
+/// This exception wraps underlying communication errors to provide a stable API surface
+/// that doesn't expose implementation details of the JSON-RPC protocol.
+/// </remarks>


Make these XML docs less specific about implementation details. No need to mention JSON-RPC specifically. Don't need to say why the exception is here (i.e. the stable API reason).

cc @copilot

Simplified in f46b3d5. Removed JSON-RPC mentions and the remarks section explaining the API design rationale.

SteveSandersonMS · 2026-01-26T15:33:19Z

@stephentoub This looks reasonable to me. Is it the sort of thing you had in mind?

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

stephentoub · 2026-01-26T15:35:44Z

dotnet/src/Session.cs

+        catch (StreamJsonRpc.RemoteInvocationException ex)
+        {
+            throw new CopilotRpcException($"Failed to destroy session: {ex.Message}", ex);
+        }


Would it be cleaner / more maintainable to have a custom InvokeWithCancellationAsync wrapper that does the catch/throw and then all the call sites would just use that wrapper?

@copilot Please do this. Also we can just use IOException - no need for a custom CopilotRpcException type.

stephentoub · 2026-01-26T15:37:01Z

Is it the sort of thing you had in mind?

If the ability to differentiate these exceptions by type is important (or if you want to expose additional strongly-typed data on the exception), yes.

Otherwise, it could just use the base IOException and not have a custom exception type.

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

SteveSandersonMS · 2026-01-26T17:00:49Z

@copilot Address this feedback:

It would be cleaner / more maintainable to have a custom InvokeWithCancellationAsync wrapper that does the catch/throw and then all the call sites would just use that wrapper

do this. Also we can just use IOException - no need for a custom CopilotRpcException type.

github-actions · 2026-01-26T17:03:29Z

Cross-SDK Consistency Review

I've analyzed error handling patterns across all four SDK implementations and found partial consistency issues with this PR.

Current State

✅ Good: Custom Exception Type Alignment

All SDKs except Node.js now use custom exception types for RPC errors:

Python: JsonRpcError
Go: JSONRPCError
.NET: CopilotRpcException (added in this PR)
Node.js: Uses generic Error class

This PR brings .NET into alignment with Python and Go.

⚠️ Concern: Structured Error Data Loss

The Python and Go SDKs preserve structured error information from JSON-RPC error responses:

Python (JsonRpcError):

class JsonRpcError(Exception):
    def __init__(self, code: int, message: str, data: Any = None):
        self.code = code      # RPC error code
        self.message = message
        self.data = data      # Additional error context

Go (JSONRPCError):

type JSONRPCError struct {
    Code    int                    `json:"code"`
    Message string                 `json:"message"`
    Data    map[string]interface{} `json:"data,omitempty"`
}

However, the .NET implementation only preserves the message string:

catch (StreamJsonRpc.RemoteInvocationException ex)
{
    throw new CopilotRpcException($"Failed to send message: {ex.Message}", ex);
}

Recommendation

Consider enhancing CopilotRpcException to preserve structured error information like Python/Go:

public class CopilotRpcException : IOException
{
    public int? ErrorCode { get; }
    public object? ErrorData { get; }
    
    public CopilotRpcException(string message, int? errorCode = null, object? errorData = null) : base(message)
    {
        ErrorCode = errorCode;
        ErrorData = errorData;
    }
}

This would allow .NET consumers to programmatically handle specific error types (e.g., authentication failures, session not found) consistently with Python and Go SDKs.

Impact

Current implementation: Functional, but consumers lose access to error codes/data that Python and Go users can access
If unaddressed: Cross-SDK applications will need different error handling strategies per language
Node.js: Should eventually adopt a similar custom exception pattern for full consistency

Verdict: This PR is a good step forward. The suggestion above is for future enhancement to achieve full parity with Python/Go error handling.

AI generated by SDK Consistency Review Agent

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

Copilot · 2026-01-26T17:07:20Z

@copilot Address this feedback:

It would be cleaner / more maintainable to have a custom InvokeWithCancellationAsync wrapper that does the catch/throw and then all the call sites would just use tha...

Implemented in 63b974a. Created InvokeRpcAsync wrapper methods in both Session and Client classes to centralize exception handling. Replaced CopilotRpcException with standard IOException. All RPC call sites now use the wrapper instead of manual try-catch blocks.

github-actions · 2026-01-26T17:16:31Z

SDK Consistency Review: Error Handling Patterns

I've reviewed this PR for cross-SDK consistency. The changes hide the StreamJsonRpc.RemoteInvocationException implementation detail behind standard IOException, which improves the .NET SDK's public API surface.

Consistency Analysis

Current Error Handling Across SDKs:

SDK	RPC Error Type	Implementation Exposed?
Node.js	Generic `Error`	✗ Hidden (aligned with this PR)
Python	`JsonRpcError`	✓ Exposed (custom exception with `code`, `message`, `data` fields)
Go	`JSONRPCError`	✓ Exposed (custom struct with `Code`, `Message`, `Data` fields)
.NET (after this PR)	`IOException`	✗ Hidden (aligned with this PR)

Observations

✅ Good: This PR aligns the .NET SDK with Node.js by hiding JSON-RPC implementation details and using standard library exception types.

⚠️ Divergence: Python and Go SDKs currently expose JSON-RPC-specific error types that provide detailed error codes and messages. This creates two distinct error handling patterns:

Abstracted approach (Node.js, .NET): Generic error types, simpler API, less diagnostic information
Explicit approach (Python, Go): JSON-RPC-specific types, more detailed error information for debugging

Recommendation

This is not a blocking issue - the divergence reflects different language idioms:

JavaScript/C# developers expect generic error types
Python/Go developers often expect explicit error types with structured data

However, for future consideration: The team may want to document this intentional design divergence in the SDK design guidelines, or consider whether all SDKs should converge on one approach:

Option A: All SDKs hide RPC details (requires Python/Go to wrap errors like this PR)
Option B: All SDKs expose RPC details (would require .NET/Node.js to expose custom types)

No action required for this PR - the changes maintain .NET SDK quality and follow reasonable C# conventions. ✅

AI generated by SDK Consistency Review Agent

Copilot

Pull request overview

This PR addresses issue #201 by hiding the StreamJsonRpc implementation detail from the public API surface. Previously, StreamJsonRpc.RemoteInvocationException was leaking through to API consumers. The PR wraps all RPC communication errors with standard IOException to maintain abstraction boundaries.

Changes:

Created wrapper methods InvokeRpcAsync in both Session.cs and Client.cs to centralize exception handling
Updated all RPC invocation call sites to use the wrapper methods
Updated test assertions and documentation examples to expect IOException instead of StreamJsonRpc.RemoteInvocationException

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
dotnet/test/SessionTests.cs	Updated test assertions to expect IOException instead of RemoteInvocationException
dotnet/src/Types.cs	Added System.IO using statement (unnecessary - handled by implicit usings)
dotnet/src/Session.cs	Added InvokeRpcAsync wrapper method and updated all RPC calls to use it
dotnet/src/Client.cs	Added static InvokeRpcAsync wrapper method and updated all RPC calls to use it
dotnet/README.md	Updated error handling documentation example to show IOException

Comments suppressed due to low confidence (1)

dotnet/src/Session.cs:101

The XML documentation should include an exception tag for IOException to inform API consumers that this method can throw IOException when communication errors occur with the Copilot CLI. Add: /// <exception cref="IOException">Thrown when communication with the Copilot CLI fails.</exception>

    /// <summary>
    /// Sends a message to the Copilot session and waits for the response.
    /// </summary>
    /// <param name="options">Options for the message to be sent, including the prompt and optional attachments.</param>
    /// <param name="cancellationToken">A <see cref="CancellationToken"/> that can be used to cancel the operation.</param>
    /// <returns>A task that resolves with the ID of the response message, which can be used to correlate events.</returns>
    /// <exception cref="InvalidOperationException">Thrown if the session has been disposed.</exception>

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T17:18:16Z

dotnet/src/Session.cs

+        {
+            return await _rpc.InvokeWithCancellationAsync<T>(method, args, cancellationToken);
+        }
+        catch (StreamJsonRpc.RemoteInvocationException ex)


Consider catching a broader set of StreamJsonRpc exceptions beyond just RemoteInvocationException. Other exceptions like ConnectionLostException, RemoteMethodNotFoundException, or the base RemoteRpcException could also be thrown during RPC operations and would currently leak through the abstraction. Consider catching the base RemoteRpcException type or handling these additional exception types to fully hide the StreamJsonRpc implementation details.

Suggested change

catch (StreamJsonRpc.RemoteInvocationException ex)

catch (StreamJsonRpc.RemoteRpcException ex)

Copilot · 2026-01-26T17:18:17Z

dotnet/src/Client.cs

+        {
+            return await rpc.InvokeWithCancellationAsync<T>(method, args, cancellationToken);
+        }
+        catch (StreamJsonRpc.RemoteInvocationException ex)


Consider catching a broader set of StreamJsonRpc exceptions beyond just RemoteInvocationException. Other exceptions like ConnectionLostException, RemoteMethodNotFoundException, or the base RemoteRpcException could also be thrown during RPC operations and would currently leak through the abstraction. Consider catching the base RemoteRpcException type or handling these additional exception types to fully hide the StreamJsonRpc implementation details.

Suggested change

catch (StreamJsonRpc.RemoteInvocationException ex)

catch (StreamJsonRpc.RemoteRpcException ex)

Copilot · 2026-01-26T17:18:17Z

dotnet/src/Types.cs

 *  Copyright (c) Microsoft Corporation. All rights reserved.
 *--------------------------------------------------------------------------------------------*/

+using System.IO;


The using System.IO; directive is unnecessary in this file since IOException is not used anywhere in Types.cs. This import should be removed. Note that IOException is used in Session.cs and Client.cs, but those files have implicit usings enabled via the project's ImplicitUsings setting, making System.IO automatically available.

Suggested change

using System.IO;

stephentoub · 2026-01-26T17:18:56Z

dotnet/src/Session.cs

        WorkspacePath = workspacePath;
    }

+    private async Task<T> InvokeRpcAsync<T>(string method, object?[]? args, CancellationToken cancellationToken)


nit: this method is identical to Client.InvokeRpcAsync, except that Client.InvokeRpcAsync accepts a JsonRpc parameter and this method bakes in the _rpc reference. If Client.InvokeRpcAsync were internal instead, this method could just be:

private Task<T> InvokeRpcAsync<T>(string method, object?[]? args, CancellationToken cancellationToken) => Client.InvokeRpcAsync(_rpc, method, args, cancellationToken);

and avoid the duplication.

Initial plan

81c05d7

Copilot AI assigned Copilot and SteveSandersonMS Jan 26, 2026

Copilot started work on behalf of SteveSandersonMS January 26, 2026 15:07 View session

Copilot AI and others added 2 commits January 26, 2026 15:12

Wrap StreamJsonRpc exceptions with CopilotRpcException

7ec76c8

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

Add explicit using System.IO for IOException

8598f71

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix StreamJsonRpc exceptions in implementation~~ Hide StreamJsonRpc implementation detail behind CopilotRpcException Jan 26, 2026

Copilot AI requested a review from SteveSandersonMS January 26, 2026 15:18

Copilot finished work on behalf of SteveSandersonMS January 26, 2026 15:18

SteveSandersonMS reviewed Jan 26, 2026

View reviewed changes

Copilot started work on behalf of SteveSandersonMS January 26, 2026 15:31 View session

SteveSandersonMS reviewed Jan 26, 2026

View reviewed changes

Only wrap RPC invocation calls in try-catch blocks

81d8dec

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

stephentoub reviewed Jan 26, 2026

View reviewed changes

Copilot AI requested a review from SteveSandersonMS January 26, 2026 15:36

Copilot finished work on behalf of SteveSandersonMS January 26, 2026 15:36

Copilot started work on behalf of SteveSandersonMS January 26, 2026 15:37 View session

Remove implementation details from CopilotRpcException docs

f46b3d5

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

Copilot finished work on behalf of SteveSandersonMS January 26, 2026 15:41

Copilot started work on behalf of SteveSandersonMS January 26, 2026 15:57 View session

Copilot finished work on behalf of SteveSandersonMS January 26, 2026 15:58

Copilot started work on behalf of SteveSandersonMS January 26, 2026 17:01 View session

Replace CopilotRpcException with IOException and add RPC wrapper methods

63b974a

Co-authored-by: SteveSandersonMS <1101362+SteveSandersonMS@users.noreply.github.com>

Copilot AI changed the title ~~Hide StreamJsonRpc implementation detail behind CopilotRpcException~~ Hide StreamJsonRpc implementation detail behind IOException Jan 26, 2026

Copilot finished work on behalf of SteveSandersonMS January 26, 2026 17:09

SteveSandersonMS marked this pull request as ready for review January 26, 2026 17:13

SteveSandersonMS requested a review from a team as a code owner January 26, 2026 17:13

Copilot AI review requested due to automatic review settings January 26, 2026 17:13

Copilot started reviewing on behalf of SteveSandersonMS January 26, 2026 17:13 View session

SteveSandersonMS approved these changes Jan 26, 2026

View reviewed changes

Copilot AI reviewed Jan 26, 2026

View reviewed changes

friggeri approved these changes Jan 26, 2026

View reviewed changes

stephentoub reviewed Jan 26, 2026

View reviewed changes

	catch (StreamJsonRpc.RemoteInvocationException ex)
	catch (StreamJsonRpc.RemoteRpcException ex)

Hide StreamJsonRpc implementation detail behind IOException #202

Are you sure you want to change the base?

Hide StreamJsonRpc implementation detail behind IOException #202

Conversation

Copilot AI commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes Made

Implementation Details

Testing

Comments on the Issue (you are @copilot in this section)

Uh oh!

SteveSandersonMS Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS commented Jan 26, 2026

Uh oh!

stephentoub Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

SteveSandersonMS Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

stephentoub commented Jan 26, 2026

Uh oh!

SteveSandersonMS commented Jan 26, 2026

Uh oh!

github-actions bot commented Jan 26, 2026

Cross-SDK Consistency Review

Current State

✅ Good: Custom Exception Type Alignment

⚠️ Concern: Structured Error Data Loss

Recommendation

Impact

Uh oh!

Copilot AI commented Jan 26, 2026

Uh oh!

github-actions bot commented Jan 26, 2026

SDK Consistency Review: Error Handling Patterns

Consistency Analysis

Observations

Recommendation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

stephentoub Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Jan 26, 2026 •

edited

Loading