Skip to content

fix(agent): classify ACP and provider errors #518

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
kaizhou-lab merged 1 commit into from
Jun 25, 2026
Merged

fix(agent): classify ACP and provider errors #518

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
108 changes: 91 additions & 17 deletions crates/aionui-ai-agent/src/protocol/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,20 @@ pub enum AcpError {
/// Agent requires authentication first.
AuthRequired,

/// Agent returned JSON that the protocol layer could not parse.
ProtocolParseError { message: String },

/// Agent rejected a malformed JSON-RPC request object.
InvalidRequest { message: String },

/// Agent-side session not found.
SessionNotFound { session_id: String },

/// Agent-side resource not found. This is distinct from stale ACP session
/// IDs; `session-not-found` payloads are normalized to `SessionNotFound`
/// before this variant is constructed.
ResourceNotFound { resource: Option<String>, message: String },

/// Agent does not support the requested method.
MethodNotFound { method: String },

Expand All @@ -131,6 +142,15 @@ pub enum AcpError {
data: Option<serde_json::Value>,
},

/// Agent returned a custom JSON-RPC/ACP error code outside the standard
/// set. Keep it structured so UI can show a protocol error instead of an
/// unknown upstream failure.
OtherProtocolError {
code: i32,
message: String,
data: Option<serde_json::Value>,
},

// ── Local errors ──────────────────────────────────────────────
/// Protocol not connected (used before connect or after disconnect).
NotConnected,
Expand Down Expand Up @@ -182,9 +202,19 @@ impl std::fmt::Display for AcpError {
write!(f, "Agent process disconnected{detail}")
}
AcpError::AuthRequired => f.write_str("Authentication required"),
AcpError::ProtocolParseError { message } => {
write!(f, "Agent protocol parse error: {message}")
}
AcpError::InvalidRequest { message } => {
write!(f, "Agent rejected an invalid protocol request: {message}")
}
AcpError::SessionNotFound { session_id } => {
write!(f, "Session not found: {session_id}")
}
AcpError::ResourceNotFound { resource, message } => match resource {
Some(resource) => write!(f, "Agent resource not found: {resource} ({message})"),
None => write!(f, "Agent resource not found: {message}"),
},
AcpError::MethodNotFound { method } => {
write!(f, "Method not supported: {method}")
}
Expand All @@ -208,6 +238,14 @@ impl std::fmt::Display for AcpError {
}
Ok(())
}
AcpError::OtherProtocolError { code, message, data } => {
write!(f, "Agent protocol error (code {code}): {message}")?;
if let Some(data) = data {
let compact = serde_json::to_string(data).unwrap_or_else(|_| "<unserializable data>".to_owned());
write!(f, " ({compact})")?;
}
Ok(())
}
AcpError::NotConnected => f.write_str("ACP protocol not connected"),
AcpError::InitTimeout { timeout_secs } => {
write!(f, "Initialize handshake timed out after {timeout_secs}s")
Expand Down Expand Up @@ -243,9 +281,18 @@ impl AcpError {
pub fn from_sdk(err: SdkError, context: &str) -> Self {
match err.code {
ErrorCode::AuthRequired => AcpError::AuthRequired,
ErrorCode::ResourceNotFound => AcpError::SessionNotFound {
session_id: context.to_owned(),
},
ErrorCode::ParseError => AcpError::ProtocolParseError { message: err.message },
ErrorCode::InvalidRequest => AcpError::InvalidRequest { message: err.message },
ErrorCode::ResourceNotFound => {
if let Some(sid) = extract_session_not_found(err.data.as_ref()) {
AcpError::SessionNotFound { session_id: sid }
} else {
AcpError::ResourceNotFound {
resource: extract_resource_not_found(err.data.as_ref()),
message: err.message,
}
}
}
ErrorCode::MethodNotFound => AcpError::MethodNotFound {
method: context.to_owned(),
},
Expand All @@ -256,7 +303,7 @@ impl AcpError {
AcpError::InvalidParams { message: err.message }
}
}
ErrorCode::ParseError | ErrorCode::InvalidRequest | ErrorCode::InternalError => {
ErrorCode::InternalError => {
if let Some(sid) = extract_session_not_found(err.data.as_ref()) {
AcpError::SessionNotFound { session_id: sid }
} else {
Expand All @@ -269,17 +316,18 @@ impl AcpError {
}
_ => {
let code = i32::from(err.code);
// -32001, -32002: additional session-not-found codes used by some agents
if code == -32001 || code == -32002 {
// -32001: additional session-not-found code used by some agents.
// -32002 is ACP ResourceNotFound and is handled above by ErrorCode.
if code == -32001 {
AcpError::SessionNotFound {
session_id: context.to_owned(),
}
} else if let Some(sid) = extract_session_not_found(err.data.as_ref()) {
AcpError::SessionNotFound { session_id: sid }
} else {
AcpError::AgentInternal {
message: err.message,
AcpError::OtherProtocolError {
code,
message: err.message,
data: err.data,
}
}
Expand All @@ -306,6 +354,16 @@ fn extract_session_not_found(data: Option<&serde_json::Value>) -> Option<String>
if sid.is_empty() { None } else { Some(sid.to_owned()) }
}

fn extract_resource_not_found(data: Option<&serde_json::Value>) -> Option<String> {
let value = data?;
let obj = match value {
serde_json::Value::Object(_) => value.clone(),
serde_json::Value::String(s) => serde_json::from_str(s).ok()?,
_ => return None,
};
obj.get("uri").and_then(|uri| uri.as_str()).map(str::to_owned)
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -457,13 +515,29 @@ mod tests {
assert!(matches!(acp, AcpError::AuthRequired));
}

#[test]
fn from_sdk_parse_error_preserves_protocol_error() {
let sdk_err = SdkError::parse_error();
let acp = AcpError::from_sdk(sdk_err, "initialize");
assert!(matches!(acp, AcpError::ProtocolParseError { .. }));
}

#[test]
fn from_sdk_invalid_request_preserves_protocol_error() {
let sdk_err = SdkError::invalid_request();
let acp = AcpError::from_sdk(sdk_err, "session/new");
assert!(matches!(acp, AcpError::InvalidRequest { .. }));
}

#[test]
fn from_sdk_resource_not_found() {
let sdk_err = SdkError::resource_not_found(None);
let acp = AcpError::from_sdk(sdk_err, "sess-42");
let sdk_err = SdkError::resource_not_found(Some("file:///missing.txt".to_owned()));
let acp = AcpError::from_sdk(sdk_err, "session/new");
match acp {
AcpError::SessionNotFound { session_id } => assert_eq!(session_id, "sess-42"),
other => panic!("Expected SessionNotFound, got {other:?}"),
AcpError::ResourceNotFound { resource, .. } => {
assert_eq!(resource.as_deref(), Some("file:///missing.txt"));
}
other => panic!("Expected ResourceNotFound, got {other:?}"),
}
}

Expand Down Expand Up @@ -565,11 +639,11 @@ mod tests {
let sdk_err = SdkError::new(-32099, "custom error");
let acp = AcpError::from_sdk(sdk_err, "ctx");
match acp {
AcpError::AgentInternal { code, message, .. } => {
AcpError::OtherProtocolError { code, message, .. } => {
assert_eq!(code, -32099);
assert_eq!(message, "custom error");
}
other => panic!("Expected AgentInternal, got {other:?}"),
other => panic!("Expected OtherProtocolError, got {other:?}"),
}
}

Expand Down Expand Up @@ -642,19 +716,19 @@ mod tests {
}

#[test]
fn agent_internal_preserves_code_and_data() {
fn other_protocol_error_preserves_code_and_data() {
let sdk_err = SdkError::new(-32099, "custom upstream error")
.data(serde_json::json!({"reason": "rate_limited", "retry_after": 30}));
let acp = AcpError::from_sdk(sdk_err, "context");
match acp {
AcpError::AgentInternal { code, message, data } => {
AcpError::OtherProtocolError { code, message, data } => {
assert_eq!(code, -32099);
assert_eq!(message, "custom upstream error");
let data = data.expect("data must be preserved");
assert_eq!(data["reason"], "rate_limited");
assert_eq!(data["retry_after"], 30);
}
other => panic!("Expected AgentInternal, got {other:?}"),
other => panic!("Expected OtherProtocolError, got {other:?}"),
}
}

Expand Down
Loading
Loading