Skip to content

Preserve JSON-RPC error data in .NET#1425

Open
stephentoub wants to merge 1 commit into
mainfrom
stephentoub/investigate-todo
Open

Preserve JSON-RPC error data in .NET#1425
stephentoub wants to merge 1 commit into
mainfrom
stephentoub/investigate-todo

Conversation

@stephentoub
Copy link
Copy Markdown
Collaborator

Summary

  • Preserve JSON-RPC error.data in the .NET client transport by carrying it on the internal remote RPC exception.
  • Keep the existing public exception behavior unchanged.

Validation

  • dotnet build .\src\GitHub.Copilot.SDK.csproj --no-restore -f net8.0 /m:1 /nodeReuse:false

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 26, 2026 03:58
@stephentoub stephentoub requested a review from a team as a code owner May 26, 2026 03:58
Copy link
Copy Markdown
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 updates the .NET JSON-RPC transport to retain the JSON-RPC error.data payload from error responses by storing it on the internal RemoteRpcException, while leaving the existing public-facing exception behavior (wrapping into IOException) unchanged.

Changes:

  • Parse and capture error.data from JSON-RPC error responses in the response handler.
  • Extend RemoteRpcException to carry an ErrorData JsonElement? payload.
  • Attach the captured error data when completing pending RPC requests with an exception.
Show a summary per file
File Description
dotnet/src/JsonRpc.cs Captures error.data from JSON-RPC responses and stores it on RemoteRpcException for internal propagation.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread dotnet/src/JsonRpc.cs
: 0;
pending.TrySetException(new RemoteRpcException(errorMessage, errorCode));
var errorData = errorProp.TryGetProperty("data", out var dataProp)
? dataProp.Clone()
Comment thread dotnet/src/JsonRpc.cs
Comment on lines +443 to +446
var errorData = errorProp.TryGetProperty("data", out var dataProp)
? dataProp.Clone()
: (JsonElement?)null;
pending.TrySetException(new RemoteRpcException(errorMessage, errorCode, errorData));
@github-actions
Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR improves cross-SDK consistency by adding ErrorData to the .NET RemoteRpcException. Here's how the other SDKs compare:

SDK Error class Has data field?
Python JsonRpcError ✅ Yes (self.data)
Go jsonrpc2.Error ✅ Yes (Data json.RawMessage)
Node.js vscode-jsonrpc ResponseError ✅ Yes (via library)
.NET (this PR) RemoteRpcException ✅ Now yes (ErrorData)

All four SDKs now preserve error.data from JSON-RPC error responses. No further changes needed in other SDKs.

Generated by SDK Consistency Review Agent for issue #1425 · ● 3.3M ·

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