From 4bfde4fcb7b26414f6a64854b98502e3fe5f8f37 Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Tue, 15 Sep 2026 01:12:56 -0700 Subject: [PATCH 1/2] Preserve structured JSON-RPC error data Keep JSON-RPC error data on copilot::Error and expose it through rpc_data() while retaining existing code, message, display, and source behavior. Add framed transport regressions for responses with and without data. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- rust/src/errors.rs | 25 +++++++++++ rust/src/lib.rs | 5 +-- rust/tests/prepared_session_test.rs | 66 ++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 5 deletions(-) diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 3bf5becbda..95bb34d80f 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -5,6 +5,8 @@ use std::borrow::{Borrow, Cow}; use std::fmt; use std::time::Duration; +use serde_json::Value; + use crate::types::SessionId; /// Crate-specific [`Result`](std::result::Result). @@ -252,6 +254,7 @@ impl fmt::Display for ErrorKind { /// Errors returned by the SDK. pub struct Error { repr: Repr, + rpc_data: Option>, // Only `Some` when `RUST_BACKTRACE` is set; boxed so the `Some` variant // doesn't inflate `Error` beyond `clippy::result_large_err` limits. backtrace: Option>, @@ -268,6 +271,7 @@ impl Error { kind, error: error.into(), }), + rpc_data: None, backtrace: capture_backtrace(), } } @@ -297,6 +301,18 @@ impl Error { { Self { repr: Repr::SimpleMessage(kind, message.into()), + rpc_data: None, + backtrace: capture_backtrace(), + } + } + + pub(crate) fn from_rpc(code: i32, message: C, data: Option) -> Self + where + C: Into>, + { + Self { + repr: Repr::SimpleMessage(ErrorKind::Rpc { code }, message.into()), + rpc_data: data.map(Box::new), backtrace: capture_backtrace(), } } @@ -319,6 +335,11 @@ impl Error { _ => None, } } + + /// Returns the structured JSON-RPC error data provided by the CLI, if any. + pub fn rpc_data(&self) -> Option<&Value> { + self.rpc_data.as_deref() + } } impl fmt::Display for Error { @@ -341,6 +362,9 @@ impl fmt::Debug for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut dbg = f.debug_struct("Error"); dbg.field("context", &self.repr); + if let Some(rpc_data) = &self.rpc_data { + dbg.field("rpc_data", rpc_data); + } if let Some(backtrace) = &self.backtrace { return dbg.field("backtrace", backtrace).finish(); } @@ -361,6 +385,7 @@ impl From for Error { fn from(kind: ErrorKind) -> Self { Self { repr: Repr::Simple(kind), + rpc_data: None, backtrace: capture_backtrace(), } } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index c95ed2087a..d02a349df8 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -2321,10 +2321,7 @@ impl Client { )) .into()); } - return Err(Error::with_message( - ErrorKind::Rpc { code: err.code }, - err.message, - )); + return Err(Error::from_rpc(err.code, err.message, err.data)); } Ok(response.result.unwrap_or(serde_json::Value::Null)) } diff --git a/rust/tests/prepared_session_test.rs b/rust/tests/prepared_session_test.rs index 3fefccabb6..190d8270b1 100644 --- a/rust/tests/prepared_session_test.rs +++ b/rust/tests/prepared_session_test.rs @@ -92,11 +92,22 @@ impl FakeServer { } async fn respond_error(&mut self, request: &Value, code: i64, message: &str) { + self.respond_error_with_data(request, code, message, None) + .await; + } + + async fn respond_error_with_data( + &mut self, + request: &Value, + code: i64, + message: &str, + data: Option, + ) { let id = request["id"].as_u64().unwrap(); let response = json!({ "jsonrpc": "2.0", "id": id, - "error": { "code": code, "message": message }, + "error": { "code": code, "message": message, "data": data }, }); write_framed(&mut self.write, &serde_json::to_vec(&response).unwrap()).await; } @@ -178,6 +189,59 @@ fn cloud_options() -> CloudSessionOptions { CloudSessionOptions::with_repository(CloudSessionRepository::new("octocat", "hello-world")) } +#[tokio::test] +async fn client_call_preserves_structured_rpc_error_data() { + let (client, mut server) = make_client(); + let call = tokio::spawn({ + let client = client.clone(); + async move { client.call("session.raw", None).await } + }); + + let request = server.read_request().await; + let data = json!({ + "code": "managed_policy_blocked", + "setting": "extensions", + "message": "Extensions are disabled by policy", + }); + server + .respond_error_with_data( + &request, + -32001, + "managed policy blocked", + Some(data.clone()), + ) + .await; + + let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); + assert_eq!(error.rpc_code(), Some(-32001)); + assert_eq!(error.message(), Some("managed policy blocked")); + assert_eq!(error.rpc_data(), Some(&data)); + assert_eq!( + error.to_string(), + "RPC error -32001: managed policy blocked" + ); +} + +#[tokio::test] +async fn client_call_handles_rpc_error_without_data() { + let (client, mut server) = make_client(); + let call = tokio::spawn({ + let client = client.clone(); + async move { client.call("session.raw", None).await } + }); + + let request = server.read_request().await; + server + .respond_error(&request, -32002, "request failed") + .await; + + let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); + assert_eq!(error.rpc_code(), Some(-32002)); + assert_eq!(error.message(), Some("request failed")); + assert_eq!(error.rpc_data(), None); + assert_eq!(error.to_string(), "RPC error -32002: request failed"); +} + fn create_result(session_id: &str) -> Value { json!({ "sessionId": session_id, "workspacePath": "/tmp/workspace" }) } From 515bd38742543471daa88388146e6043714c4fc9 Mon Sep 17 00:00:00 2001 From: Hunter Sadler Date: Fri, 18 Sep 2026 13:58:29 -0600 Subject: [PATCH 2/2] Preserve RPC error Debug compatibility Keep structured data accessible through rpc_data without changing formatted error output. Cover non-object payloads and distinguish omitted data from explicit null in framed RPC tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- rust/src/errors.rs | 9 ++-- rust/tests/prepared_session_test.rs | 73 +++++++++++++++++++++++------ 2 files changed, 63 insertions(+), 19 deletions(-) diff --git a/rust/src/errors.rs b/rust/src/errors.rs index 95bb34d80f..b17fd03d0c 100644 --- a/rust/src/errors.rs +++ b/rust/src/errors.rs @@ -336,7 +336,11 @@ impl Error { } } - /// Returns the structured JSON-RPC error data provided by the CLI, if any. + /// Returns the non-null JSON-RPC error data for an [`ErrorKind::Rpc`] error, if any. + /// + /// The value may be an object, array, or scalar. Missing and `null` data + /// return `None`. Data is not included in this error's `Display` or `Debug` + /// output. pub fn rpc_data(&self) -> Option<&Value> { self.rpc_data.as_deref() } @@ -362,9 +366,6 @@ impl fmt::Debug for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut dbg = f.debug_struct("Error"); dbg.field("context", &self.repr); - if let Some(rpc_data) = &self.rpc_data { - dbg.field("rpc_data", rpc_data); - } if let Some(backtrace) = &self.backtrace { return dbg.field("backtrace", backtrace).finish(); } diff --git a/rust/tests/prepared_session_test.rs b/rust/tests/prepared_session_test.rs index 190d8270b1..56698bac92 100644 --- a/rust/tests/prepared_session_test.rs +++ b/rust/tests/prepared_session_test.rs @@ -104,11 +104,14 @@ impl FakeServer { data: Option, ) { let id = request["id"].as_u64().unwrap(); - let response = json!({ + let mut response = json!({ "jsonrpc": "2.0", "id": id, - "error": { "code": code, "message": message, "data": data }, + "error": { "code": code, "message": message }, }); + if let Some(data) = data { + response["error"]["data"] = data; + } write_framed(&mut self.write, &serde_json::to_vec(&response).unwrap()).await; } @@ -213,33 +216,73 @@ async fn client_call_preserves_structured_rpc_error_data() { .await; let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); + assert_eq!(error.kind(), &ErrorKind::Rpc { code: -32001 }); assert_eq!(error.rpc_code(), Some(-32001)); assert_eq!(error.message(), Some("managed policy blocked")); assert_eq!(error.rpc_data(), Some(&data)); + assert!(std::error::Error::source(&error).is_none()); assert_eq!( error.to_string(), "RPC error -32001: managed policy blocked" ); + for debug in [format!("{error:?}"), format!("{error:#?}")] { + assert!(debug.contains("context")); + assert!(debug.contains("Rpc")); + assert!(debug.contains("managed policy blocked")); + assert!(!debug.contains("rpc_data")); + assert!( + !debug.contains("managed_policy_blocked"), + "RPC payloads must be accessed explicitly, not included in Debug" + ); + } +} + +#[tokio::test] +async fn client_call_preserves_non_object_rpc_error_data() { + let (client, mut server) = make_client(); + for data in [ + json!([{"detail": "example"}, null, false, 42]), + json!("detail"), + json!(0), + json!(false), + ] { + let call = tokio::spawn({ + let client = client.clone(); + async move { client.call("session.raw", None).await } + }); + + let request = server.read_request().await; + server + .respond_error_with_data(&request, -32001, "request failed", Some(data.clone())) + .await; + + let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); + assert_eq!(error.rpc_data(), Some(&data)); + } } #[tokio::test] async fn client_call_handles_rpc_error_without_data() { let (client, mut server) = make_client(); - let call = tokio::spawn({ - let client = client.clone(); - async move { client.call("session.raw", None).await } - }); + for data in [None, Some(Value::Null)] { + let call = tokio::spawn({ + let client = client.clone(); + async move { client.call("session.raw", None).await } + }); - let request = server.read_request().await; - server - .respond_error(&request, -32002, "request failed") - .await; + let request = server.read_request().await; + server + .respond_error_with_data(&request, -32002, "request failed", data) + .await; - let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); - assert_eq!(error.rpc_code(), Some(-32002)); - assert_eq!(error.message(), Some("request failed")); - assert_eq!(error.rpc_data(), None); - assert_eq!(error.to_string(), "RPC error -32002: request failed"); + let error = timeout(TIMEOUT, call).await.unwrap().unwrap().unwrap_err(); + assert_eq!(error.kind(), &ErrorKind::Rpc { code: -32002 }); + assert_eq!(error.rpc_code(), Some(-32002)); + assert_eq!(error.message(), Some("request failed")); + assert_eq!(error.rpc_data(), None); + assert!(std::error::Error::source(&error).is_none()); + assert_eq!(error.to_string(), "RPC error -32002: request failed"); + } } fn create_result(session_id: &str) -> Value {