From 9c5cac1ccef82913a049725162250b20ba74d0fb Mon Sep 17 00:00:00 2001 From: "Syed A. Hashmi" Date: Thu, 19 Feb 2026 03:57:09 -0800 Subject: [PATCH] [ISSUE 706]: Standardize returned errors from Plano --- crates/Cargo.lock | 3 + crates/brightstaff/src/handlers/llm.rs | 70 +++++++--------- crates/common/Cargo.toml | 6 ++ crates/common/src/errors.rs | 107 ++++++++++++++++++++++++- 4 files changed, 141 insertions(+), 45 deletions(-) diff --git a/crates/Cargo.lock b/crates/Cargo.lock index ebe5b881..fbf817e7 100644 --- a/crates/Cargo.lock +++ b/crates/Cargo.lock @@ -436,11 +436,14 @@ name = "common" version = "0.1.0" dependencies = [ "axum", + "bytes", "derivative", "duration-string", "governor", "hermesllm", "hex", + "http-body-util", + "hyper 1.6.0", "log", "pretty_assertions", "proxy-wasm", diff --git a/crates/brightstaff/src/handlers/llm.rs b/crates/brightstaff/src/handlers/llm.rs index 435fb6f5..8e8f9661 100644 --- a/crates/brightstaff/src/handlers/llm.rs +++ b/crates/brightstaff/src/handlers/llm.rs @@ -8,9 +8,9 @@ use hermesllm::apis::openai_responses::InputParam; use hermesllm::clients::{SupportedAPIsFromClient, SupportedUpstreamAPIs}; use hermesllm::{ProviderRequest, ProviderRequestType}; use http_body_util::combinators::BoxBody; -use http_body_util::{BodyExt, Full}; +use http_body_util::BodyExt; use hyper::header::{self}; -use hyper::{Request, Response, StatusCode}; +use hyper::{Request, Response}; use opentelemetry::global; use opentelemetry::trace::get_active_span; use opentelemetry_http::HeaderInjector; @@ -30,11 +30,7 @@ use crate::state::{ }; use crate::tracing::{llm as tracing_llm, operation_component, set_service_name}; -fn full>(chunk: T) -> BoxBody { - Full::new(chunk.into()) - .map_err(|never| match never {}) - .boxed() -} +use common::errors::BrightStaffError; pub async fn llm_chat( request: Request, @@ -135,10 +131,11 @@ async fn llm_chat_inner( error = %err, "failed to parse request as ProviderRequestType" ); - let err_msg = format!("Failed to parse request: {}", err); - let mut bad_request = Response::new(full(err_msg)); - *bad_request.status_mut() = StatusCode::BAD_REQUEST; - return Ok(bad_request); + return Ok(BrightStaffError::InvalidRequest(format!( + "Failed to parse request: {}", + err + )) + .into_response()); } }; @@ -163,9 +160,7 @@ async fn llm_chat_inner( None => { let err_msg = "No model specified in request and no default provider configured"; warn!("{}", err_msg); - let mut bad_request = Response::new(full(err_msg.to_string())); - *bad_request.status_mut() = StatusCode::BAD_REQUEST; - return Ok(bad_request); + return Ok(BrightStaffError::NoModelSpecified.into_response()); } } } else { @@ -186,14 +181,8 @@ async fn llm_chat_inner( .get(&alias_resolved_model) .is_none() { - let err_msg = format!( - "Model '{}' not found in configured providers", - alias_resolved_model - ); warn!(model = %alias_resolved_model, "model not found in configured providers"); - let mut bad_request = Response::new(full(err_msg)); - *bad_request.status_mut() = StatusCode::BAD_REQUEST; - return Ok(bad_request); + return Ok(BrightStaffError::ModelNotFound(alias_resolved_model).into_response()); } // Handle provider/model slug format (e.g., "openai/gpt-4") @@ -288,13 +277,10 @@ async fn llm_chat_inner( Err(StateStorageError::NotFound(_)) => { // Return 409 Conflict when previous_response_id not found warn!(previous_response_id = %prev_resp_id, "previous response_id not found"); - let err_msg = format!( - "Conversation state not found for previous_response_id: {}", - prev_resp_id - ); - let mut conflict_response = Response::new(full(err_msg)); - *conflict_response.status_mut() = StatusCode::CONFLICT; - return Ok(conflict_response); + return Ok(BrightStaffError::ConversationStateNotFound( + prev_resp_id.to_string(), + ) + .into_response()); } Err(e) => { // Log warning but continue on other storage errors @@ -345,9 +331,11 @@ async fn llm_chat_inner( { Ok(result) => result, Err(err) => { - let mut internal_error = Response::new(full(err.message)); - *internal_error.status_mut() = err.status_code; - return Ok(internal_error); + return Ok(BrightStaffError::ForwardedError { + status_code: err.status_code, + message: err.message, + } + .into_response()); } }; @@ -413,10 +401,11 @@ async fn llm_chat_inner( { Ok(res) => res, Err(err) => { - let err_msg = format!("Failed to send request: {}", err); - let mut internal_error = Response::new(full(err_msg)); - *internal_error.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - return Ok(internal_error); + return Ok(BrightStaffError::InternalServerError(format!( + "Failed to send request: {}", + err + )) + .into_response()); } }; @@ -473,12 +462,11 @@ async fn llm_chat_inner( match response.body(streaming_response.body) { Ok(response) => Ok(response), - Err(err) => { - let err_msg = format!("Failed to create response: {}", err); - let mut internal_error = Response::new(full(err_msg)); - *internal_error.status_mut() = StatusCode::INTERNAL_SERVER_ERROR; - Ok(internal_error) - } + Err(err) => Ok(BrightStaffError::InternalServerError(format!( + "Failed to create response: {}", + err + )) + .into_response()), } } diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index cb471bd6..dd2cba15 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -20,6 +20,9 @@ urlencoding = "2.1.3" url = "2.5.4" hermesllm = { version = "0.1.0", path = "../hermesllm" } serde_with = "3.13.0" +hyper = "1.0" +bytes = "1.0" +http-body-util = "0.1" [features] default = [] @@ -30,3 +33,6 @@ serde_json = "1.0.64" serial_test = "3.2" axum = "0.7" tokio = { version = "1.44", features = ["sync", "time", "macros", "rt"] } +hyper = { version = "1.0", features = ["full"] } +bytes = "1.0" +http-body-util = "0.1" diff --git a/crates/common/src/errors.rs b/crates/common/src/errors.rs index 21af3c94..6e1f022d 100644 --- a/crates/common/src/errors.rs +++ b/crates/common/src/errors.rs @@ -1,9 +1,13 @@ -use proxy_wasm::types::Status; - use crate::{api::open_ai::ChatCompletionChunkResponseError, ratelimit}; +use bytes::Bytes; use hermesllm::apis::openai::OpenAIError; +use http_body_util::{combinators::BoxBody, BodyExt, Full}; +use hyper::{Error as HyperError, Response, StatusCode}; +use proxy_wasm::types::Status; +use serde_json::json; +use thiserror::Error; -#[derive(thiserror::Error, Debug)] +#[derive(Error, Debug)] pub enum ClientError { #[error("Error dispatching HTTP call to `{upstream_name}/{path}`, error: {internal_status:?}")] DispatchError { @@ -13,7 +17,7 @@ pub enum ClientError { }, } -#[derive(thiserror::Error, Debug)] +#[derive(Error, Debug)] pub enum ServerError { #[error(transparent)] HttpDispatch(ClientError), @@ -43,3 +47,98 @@ pub enum ServerError { #[error("error parsing openai message: {0}")] OpenAIPError(#[from] OpenAIError), } +// ----------------------------------------------------------------------------- +// BrightStaff Errors (Standardized) +// ----------------------------------------------------------------------------- +#[derive(Debug, Error)] +pub enum BrightStaffError { + #[error("The requested model '{0}' does not exist")] + ModelNotFound(String), + + #[error("No model specified in request and no default provider configured")] + NoModelSpecified, + + #[error("Conversation state not found for previous_response_id: {0}")] + ConversationStateNotFound(String), + + #[error("Internal server error: {0}")] + InternalServerError(String), + + #[error("Invalid request: {0}")] + InvalidRequest(String), + + #[error("{message}")] + ForwardedError { + status_code: StatusCode, + message: String, + }, +} + +impl BrightStaffError { + pub fn into_response(self) -> Response> { + let (status, code, details) = match &self { + BrightStaffError::ModelNotFound(model_name) => ( + StatusCode::NOT_FOUND, + "ModelNotFound", + json!({ "rejected_model_id": model_name }), + ), + + BrightStaffError::NoModelSpecified => { + (StatusCode::BAD_REQUEST, "NoModelSpecified", json!({})) + } + + BrightStaffError::ConversationStateNotFound(prev_resp_id) => ( + StatusCode::CONFLICT, + "ConversationStateNotFound", + json!({ "previous_response_id": prev_resp_id }), + ), + + BrightStaffError::InternalServerError(reason) => ( + StatusCode::INTERNAL_SERVER_ERROR, + "InternalServerError", + // Passing the reason into details for easier debugging + json!({ "reason": reason }), + ), + + BrightStaffError::InvalidRequest(reason) => ( + StatusCode::BAD_REQUEST, + "InvalidRequest", + json!({ "reason": reason }), + ), + + BrightStaffError::ForwardedError { + status_code, + message, + } => (*status_code, "ForwardedError", json!({ "reason": message })), + }; + + let body_json = json!({ + "error": { + "code": code, + "message": self.to_string(), + "details": details + } + }); + + // 1. Create the concrete body + let full_body = Full::new(Bytes::from(body_json.to_string())); + + // 2. Convert it to BoxBody + // We map_err because Full never fails, but BoxBody expects a HyperError + let boxed_body = full_body + .map_err(|never| match never {}) // This handles the "Infallible" error type + .boxed(); + + Response::builder() + .status(status) + .header("content-type", "application/json") + .body(boxed_body) + .unwrap_or_else(|_| { + Response::new( + Full::new(Bytes::from("Internal Error")) + .map_err(|never| match never {}) + .boxed(), + ) + }) + } +}