Skip to content

Expose structured JSON-RPC error data in Go, .NET, and Java - #2732

Open
aurokin wants to merge 2 commits into
mainfrom
aurokin-public-rpc-error-data
Open

aurokin wants to merge 2 commits into
mainfrom
aurokin-public-rpc-error-data

Conversation

@aurokin

@aurokin aurokin commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Expose machine-readable JSON-RPC error data through supported public APIs, following the nonblocking review follow-up on #2664.

  • Go: export copilot.RPCError as an alias of the existing transport error; callers use errors.As and read Code, Message, and Data (json.RawMessage).
  • .NET: make the existing RemoteRpcException public, exposing ErrorCode and ErrorData (JsonElement?) through the existing IOException.InnerException. The exact outer exception and wrapping remain unchanged.
  • Java: make the existing final JsonRpcException public, retain error data in the transport, and expose it through JsonNode getData(). Existing code/message and future wrapping remain unchanged.

Add public usage examples and framed transport-to-public-API regressions for objects, arrays, scalars, empty values, omitted data, explicit null, formatting, and catch/unwrap compatibility.

Payload compatibility

Omitted data and explicit JSON null remain distinct in all three APIs. This preserves existing Go/.NET transport semantics and extends them to Java; Rust's rpc_data() instead returns None for both. Existing error messages and ordinary error formatting do not newly include payloads. No domain-specific exception classification or runtime changes are introduced.

Related: #2726 (the completed Rust counterpart, implemented in #2664). This PR does not reopen or duplicate that Rust work.

Validation

  • Go: focused public-error and transport tests passed with the race detector and offline dependencies.
  • .NET: 19 focused public-error/transport tests passed. Compiled for net8.0 with SDK 10.0.401; executed on runtime 10.0.12 using DOTNET_ROLL_FORWARD=Major. Execution on an actual .NET 8 runtime remains unverified.
  • Java: 43 focused tests passed using offline Maven verification with JDK 25; Spotless and Checkstyle passed. Rerun after retaining the exception's original final restriction.
  • git diff --check passed; fresh review after the compatibility fix found no remaining issues.

No broad/native matrix or real-CLI E2E was run. Controlled framed peers exercise the error-envelope boundary without CLI/runtime downloads or inference dependencies.

Fixes: github/copilot-sdk-internal#291

Preserve existing error identities and wrapping while exposing raw JSON data through supported public APIs. Add framed transport regressions and usage documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aurokin
aurokin requested a review from a team as a code owner September 19, 2026 16:54
Copilot AI balanced review requested due to automatic review settings September 19, 2026 16:54
@github-actions

This comment has been minimized.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The APIs preserve compatibility and include focused coverage for payload shapes, null semantics, formatting, and unwrapping.

Review effort: Balanced
Findings: None

What changed in this PR

Exposes structured JSON-RPC error payloads through public Go, .NET, and Java SDK APIs while preserving existing wrapping and formatting behavior.

Changes:

  • Adds public access to raw error codes, messages, and JSON data.
  • Documents omitted versus explicit-null payload semantics.
  • Adds framed transport regression tests across payload types and failure paths.
File Description
java/​sdk/​src/​test/​java/​com/​github/​copilot/​consumer/​JsonRpcErrorDataTest.java Tests Java payload preservation and wrapping.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​JsonRpcException.java Publicly exposes optional JSON error data.
java/​sdk/​src/​main/​java/​com/​github/​copilot/​JsonRpcClient.java Retains error data from responses.
java/​README.md Documents Java error handling.
go/​README.md Documents Go RPC error inspection.
go/​errors.go Exports the transport error alias.
go/​errors_test.go Tests Go payload and wrapping semantics.
dotnet/​test/​Unit/​RpcErrorDataTests.cs Tests .NET payload lifetime and wrapping.
dotnet/​src/​JsonRpc.cs Publicly exposes remote error details.
dotnet/​README.md Documents .NET error unwrapping.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread dotnet/test/Unit/RpcErrorDataTests.cs Fixed
Comment thread dotnet/test/Unit/RpcErrorDataTests.cs Fixed
Surface malformed frame lengths as IO failures with regression coverage. Use scoped cancellation disposal and only suppress the expected server shutdown cancellation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aurokin

aurokin commented Sep 19, 2026

Copy link
Copy Markdown
Contributor Author

The Rust parity suggestion in the consistency review is already addressed by merged #2664 (commit 8045fb7). Current main exposes Error::rpc_data() and forwards err.data through Error::from_rpc in the shared call path. That implementation intentionally returns None for both omitted and explicit-null data; this PR preserves the existing Go/.NET distinction and documents the Java equivalent. No additional Rust change is needed here.

@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review

This PR exposes structured JSON-RPC error data (code, message, and raw data payload) as public API in Go (copilot.RPCError), .NET (RemoteRpcException made public, ErrorData property), and Java (JsonRpcException made public, getData() added). Each includes README docs and tests. Nice, consistent design across these three (naming/behavior for omitted vs. explicit-null data is well aligned).

However, this creates a parity gap with the other three SDKs:

  • Node.js: The underlying ResponseError (from vscode-jsonrpc) already carries a .data property, but it is not re-exported from nodejs/src/index.ts. Consumers can only access it by importing vscode-jsonrpc directly rather than through the blessed @github/copilot-sdk surface — unlike Go/.NET/Java, which now expose this natively.
  • Python: JsonRpcError (python/copilot/_jsonrpc.py) already stores code, message, and data, but the class lives in a private (_-prefixed) module and is not listed in copilot/__init__.py's __all__, so it isn't part of the documented public API either.
  • Rust: This is a real behavioral gap, not just a documentation/export gap. Error::rpc_code() exposes the JSON-RPC error code, but when an RPC error response is converted into the crate's Error type (call_with_inline_callback in rust/src/lib.rs), only err.code and err.message are used — the data field on the internal JsonRpcError (rust/src/jsonrpc.rs) is silently dropped and has no equivalent accessor (e.g., an Error::rpc_data()).

Suggestion: Consider follow-up work to (a) publicly export ResponseError/its data field from the Node.js package surface, (b) promote Python's JsonRpcError to a public, documented API, and (c) thread the data payload through to Rust's Error type with an accessor analogous to rpc_code(), so all six SDKs offer equivalent structured error-data access. None of this blocks the current PR, which is internally consistent for the three languages it touches.

Generated by SDK Consistency Review Agent for #2732 · copilot · sonnet50 · 84.9 AIC · ⌖ 11.5 AIC · ⊞ 7.8K ·

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.

3 participants