Skip to content

refactor(jsonrpc): carry typed inspection errors #2244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
johntmyers merged 1 commit into from
Jul 13, 2026
Merged

refactor(jsonrpc): carry typed inspection errors #2244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
204 changes: 166 additions & 38 deletions crates/openshell-supervisor-network/src/l7/jsonrpc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,70 @@ pub struct JsonRpcRequestInfo {
pub is_batch: bool,
pub receive_stream: bool,
pub has_response: bool,
pub error: Option<String>,
/// Typed inspection failure discovered before policy evaluation.
pub error: Option<JsonRpcInspectionError>,
}

/// Stable kind of failure found while inspecting a JSON-RPC-family message.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
#[non_exhaustive]
pub enum JsonRpcInspectionErrorKind {
/// The request body is not valid JSON.
InvalidJson,
/// The decoded JSON fails JSON-RPC or protocol-specific message validation.
InvalidMessage,
}

/// Typed JSON-RPC inspection failure with an inseparable kind and diagnostic.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct JsonRpcInspectionError {
kind: JsonRpcInspectionErrorKind,
detail: String,
}

impl JsonRpcInspectionError {
fn invalid_json() -> Self {
Self {
kind: JsonRpcInspectionErrorKind::InvalidJson,
detail: "invalid JSON".to_string(),
}
}

fn invalid_message(detail: impl Into<String>) -> Self {
Self {
kind: JsonRpcInspectionErrorKind::InvalidMessage,
detail: detail.into(),
}
}

/// Return the stable failure kind used by typed control flow.
#[must_use]
pub const fn kind(&self) -> JsonRpcInspectionErrorKind {
self.kind
}

/// Return the existing policy-visible and human-readable diagnostic.
///
/// Callers must use [`Self::kind`] instead of parsing this text for control flow.
#[must_use]
pub fn detail(&self) -> &str {
&self.detail
}

/// Render the existing proxy denial reason for an inspection failure.
pub(crate) fn rejection_reason(&self) -> String {
format!("JSON-RPC request rejected: {self}")
}
}

impl std::fmt::Display for JsonRpcInspectionError {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
formatter.write_str(&self.detail)
}
}

impl std::error::Error for JsonRpcInspectionError {}

#[derive(Debug, Clone, PartialEq, Eq)]
pub struct JsonRpcCallInfo {
/// JSON-RPC method, or the MCP method name after typed MCP parsing.
Expand All @@ -111,6 +172,16 @@ pub struct JsonRpcCallInfo {
}

impl JsonRpcRequestInfo {
fn rejected(is_batch: bool, error: JsonRpcInspectionError) -> Self {
Self {
calls: Vec::new(),
is_batch,
receive_stream: false,
has_response: false,
error: Some(error),
}
}

/// MCP streamable HTTP uses an empty GET to receive server messages. It has
/// no request body to inspect, but it must still pass through MCP endpoints.
pub(crate) fn receive_stream() -> Self {
Expand Down Expand Up @@ -167,24 +238,15 @@ pub fn parse_jsonrpc_body_with_options(
inspection_options: JsonRpcInspectionOptions,
) -> JsonRpcRequestInfo {
let Ok(value) = serde_json::from_slice::<serde_json::Value>(body) else {
return JsonRpcRequestInfo {
calls: Vec::new(),
is_batch: false,
receive_stream: false,
has_response: false,
error: Some("invalid JSON".to_string()),
};
return JsonRpcRequestInfo::rejected(false, JsonRpcInspectionError::invalid_json());
};

if let serde_json::Value::Array(items) = value {
if items.is_empty() {
return JsonRpcRequestInfo {
calls: Vec::new(),
is_batch: true,
receive_stream: false,
has_response: false,
error: Some("empty batch".to_string()),
};
return JsonRpcRequestInfo::rejected(
true,
JsonRpcInspectionError::invalid_message("empty batch"),
);
}
let mut calls = Vec::new();
let mut has_response = false;
Expand All @@ -193,13 +255,12 @@ pub fn parse_jsonrpc_body_with_options(
Ok(JsonRpcMessageInfo::Call(call)) => calls.push(call),
Ok(JsonRpcMessageInfo::Response) => has_response = true,
Err(error) => {
return JsonRpcRequestInfo {
calls: Vec::new(),
is_batch: true,
receive_stream: false,
has_response: false,
error: Some(format!("batch item invalid: {error}")),
};
return JsonRpcRequestInfo::rejected(
true,
JsonRpcInspectionError::invalid_message(format!(
"batch item invalid: {error}"
)),
);
}
}
}
Expand Down Expand Up @@ -227,13 +288,9 @@ pub fn parse_jsonrpc_body_with_options(
has_response: true,
error: None,
},
Err(error) => JsonRpcRequestInfo {
calls: Vec::new(),
is_batch: false,
receive_stream: false,
has_response: false,
error: Some(error),
},
Err(error) => {
JsonRpcRequestInfo::rejected(false, JsonRpcInspectionError::invalid_message(error))
}
}
}

Expand Down Expand Up @@ -444,6 +501,47 @@ mod tests {
assert!(info.error.is_none());
}

#[test]
fn inspection_error_kinds_distinguish_invalid_json_and_messages() {
fn assert_inspection_error(
body: &[u8],
mode: JsonRpcInspectionMode,
expected_kind: JsonRpcInspectionErrorKind,
expected_detail: &str,
) {
let info = parse_jsonrpc_body(body, mode);
let error = info.error.as_ref().expect("inspection failure");

assert_eq!(error.kind(), expected_kind);
assert_eq!(error.detail(), expected_detail);
}

assert_inspection_error(
b"{",
JsonRpcInspectionMode::JsonRpc,
JsonRpcInspectionErrorKind::InvalidJson,
"invalid JSON",
);
assert_inspection_error(
br"[]",
JsonRpcInspectionMode::JsonRpc,
JsonRpcInspectionErrorKind::InvalidMessage,
"empty batch",
);
assert_inspection_error(
br"null",
JsonRpcInspectionMode::JsonRpc,
JsonRpcInspectionErrorKind::InvalidMessage,
"missing or non-string 'jsonrpc' field",
);
assert_inspection_error(
br#"[{"jsonrpc":"2.0","id":1,"method":"reports.list"},{"id":2,"method":"reports.search"}]"#,
JsonRpcInspectionMode::JsonRpc,
JsonRpcInspectionErrorKind::InvalidMessage,
"batch item invalid: missing or non-string 'jsonrpc' field",
);
}

#[test]
fn ignores_params_when_extracting_method() {
let body = br#"{"jsonrpc":"2.0","id":1,"method":"reports.search","params":{"query":"quarterly","filters":{"scope":"workspace/main"}}}"#;
Expand Down Expand Up @@ -499,7 +597,8 @@ mod tests {
assert!(info.calls.is_empty());
assert!(
info.error
.as_deref()
.as_ref()
.map(JsonRpcInspectionError::detail)
.is_some_and(|error| error.contains("strict_tool_names")),
"expected strict tool-name error, got {info:?}"
);
Expand Down Expand Up @@ -534,9 +633,14 @@ mod tests {
let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::Mcp);

assert!(info.calls.is_empty());
assert_eq!(
info.error.as_ref().map(JsonRpcInspectionError::kind),
Some(JsonRpcInspectionErrorKind::InvalidMessage)
);
assert!(
info.error
.as_deref()
.as_ref()
.map(JsonRpcInspectionError::detail)
.is_some_and(|error| error.contains("invalid MCP request params")),
"expected MCP params validation error, got {info:?}"
);
Expand Down Expand Up @@ -636,7 +740,7 @@ mod tests {

assert!(info.calls.is_empty());
assert_eq!(
info.error.as_deref(),
info.error.as_ref().map(JsonRpcInspectionError::detail),
Some("missing or non-string 'jsonrpc' field")
);
}
Expand All @@ -652,7 +756,7 @@ mod tests {
assert!(info.calls.is_empty());
assert!(info.is_batch);
assert_eq!(
info.error.as_deref(),
info.error.as_ref().map(JsonRpcInspectionError::detail),
Some("batch item invalid: missing or non-string 'jsonrpc' field")
);
}
Expand All @@ -664,7 +768,7 @@ mod tests {

assert!(info.calls.is_empty());
assert_eq!(
info.error.as_deref(),
info.error.as_ref().map(JsonRpcInspectionError::detail),
Some("unsupported JSON-RPC version '1.0'")
);
}
Expand All @@ -684,6 +788,24 @@ mod tests {
assert_eq!(info.calls[1].method, "reports.search");
}

#[test]
fn preserves_current_mcp_batch_acceptance() {
let body = br#"[
{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"read_status","arguments":{}}},
{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"search_web","arguments":{"query":"openshell"}}}
]"#;
let info = parse_jsonrpc_body(body, JsonRpcInspectionMode::Mcp);

assert!(
info.error.is_none(),
"MCP batch should remain valid: {info:?}"
);
assert!(info.is_batch);
assert_eq!(info.calls.len(), 2);
assert_eq!(info.calls[0].tool.as_deref(), Some("read_status"));
assert_eq!(info.calls[1].tool.as_deref(), Some("search_web"));
}

#[test]
fn parses_batch_with_calls_and_responses() {
let body = br#"[
Expand All @@ -708,7 +830,7 @@ mod tests {
assert!(info.calls.is_empty());
assert!(!info.has_response);
assert_eq!(
info.error.as_deref(),
info.error.as_ref().map(JsonRpcInspectionError::detail),
Some("JSON-RPC response includes both result and error")
);
}
Expand All @@ -719,15 +841,21 @@ mod tests {
let result_info = parse_jsonrpc_body(result_body, JsonRpcInspectionMode::JsonRpc);
assert!(result_info.calls.is_empty());
assert_eq!(
result_info.error.as_deref(),
result_info
.error
.as_ref()
.map(JsonRpcInspectionError::detail),
Some("JSON-RPC message includes both method and result/error")
);

let error_body = br#"{"jsonrpc":"2.0","id":1,"method":"initialize","error":{"code":-32603,"message":"failed"}}"#;
let error_info = parse_jsonrpc_body(error_body, JsonRpcInspectionMode::JsonRpc);
assert!(error_info.calls.is_empty());
assert_eq!(
error_info.error.as_deref(),
error_info
.error
.as_ref()
.map(JsonRpcInspectionError::detail),
Some("JSON-RPC message includes both method and result/error")
);
}
Expand All @@ -743,7 +871,7 @@ mod tests {
assert!(info.calls.is_empty());
assert!(info.is_batch);
assert_eq!(
info.error.as_deref(),
info.error.as_ref().map(JsonRpcInspectionError::detail),
Some("batch item invalid: JSON-RPC message includes both method and result/error")
);
}
Expand Down
Loading
Loading