Conversation
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>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
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.
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>
|
The Rust parity suggestion in the consistency review is already addressed by merged #2664 (commit 8045fb7). Current main exposes |
Cross-SDK Consistency ReviewThis PR exposes structured JSON-RPC error data (code, message, and raw However, this creates a parity gap with the other three SDKs:
Suggestion: Consider follow-up work to (a) publicly export
|
Summary
Expose machine-readable JSON-RPC error data through supported public APIs, following the nonblocking review follow-up on #2664.
copilot.RPCErroras an alias of the existing transport error; callers useerrors.Asand readCode,Message, andData(json.RawMessage).RemoteRpcExceptionpublic, exposingErrorCodeandErrorData(JsonElement?) through the existingIOException.InnerException. The exact outer exception and wrapping remain unchanged.JsonRpcExceptionpublic, retain error data in the transport, and expose it throughJsonNode 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 returnsNonefor 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
net8.0with SDK 10.0.401; executed on runtime 10.0.12 usingDOTNET_ROLL_FORWARD=Major. Execution on an actual .NET 8 runtime remains unverified.finalrestriction.git diff --checkpassed; 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