diff --git a/Cargo.lock b/Cargo.lock index b595233..a59f204 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -936,7 +936,7 @@ checksum = "384b8ab6d37215f3c5301a95a4accb5d64aa607f1fcb26a11b5303878451b4fe" [[package]] name = "openapi-to-rust" -version = "0.1.15" +version = "0.2.0" dependencies = [ "clap", "heck", diff --git a/Cargo.toml b/Cargo.toml index 4920935..83e5eef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openapi-to-rust" -version = "0.1.15" +version = "0.2.0" edition = "2024" rust-version = "1.85.0" authors = ["James Lal"] diff --git a/examples/generated/client.rs b/examples/generated/client.rs index 9c3a7b0..245e0fd 100644 --- a/examples/generated/client.rs +++ b/examples/generated/client.rs @@ -2,9 +2,17 @@ //! //! This file contains the HTTP client implementation for GET, POST, etc. //! Do not edit manually - regenerate using the appropriate script. +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] use super::types::*; use thiserror::Error; -/// HTTP client errors that can occur during API requests +/// Transport-level errors: failures where we never received an +/// inspectable HTTP response from the server. +/// +/// HTTP responses with non-2xx status codes are surfaced as +/// [`ApiError`] inside [`ApiOpError::Api`], not here, so callers can +/// always inspect status, headers, and the raw body when the server +/// actually responded. #[derive(Error, Debug)] pub enum HttpError { /// Network or connection error (from reqwest) @@ -16,12 +24,6 @@ pub enum HttpError { /// Request serialization error #[error("Failed to serialize request: {0}")] Serialization(String), - /// Response deserialization error - #[error("Failed to deserialize response: {0}")] - Deserialization(String), - /// HTTP error response (4xx, 5xx) - #[error("HTTP error {status}: {message}")] - Http { status: u16, message: String, body: Option }, /// Authentication error #[error("Authentication error: {0}")] Auth(String), @@ -36,46 +38,92 @@ pub enum HttpError { Other(String), } impl HttpError { - /// Create an HTTP error from a status code and message - pub fn from_status( - status: u16, - message: impl Into, - body: Option, - ) -> Self { - Self::Http { - status, - message: message.into(), - body, - } - } /// Create a serialization error pub fn serialization_error(error: impl std::fmt::Display) -> Self { Self::Serialization(error.to_string()) } - /// Create a deserialization error - pub fn deserialization_error(error: impl std::fmt::Display) -> Self { - Self::Deserialization(error.to_string()) + /// Check if this transport error is retryable + pub fn is_retryable(&self) -> bool { + matches!(self, Self::Network(_) | Self::Middleware(_) | Self::Timeout) } - /// Check if this is a client error (4xx) +} +/// Envelope returned for any HTTP response that we received but +/// couldn't (or didn't) treat as a successful typed result. +/// +/// Includes both non-2xx responses and 2xx responses whose body +/// failed to deserialize into the expected success type. `status`, +/// `headers`, and `body` are always populated so callers can +/// inspect what the server sent without modifying the generated +/// code. `typed` carries the parsed per-operation error variant +/// when the body matched a declared schema. +#[derive(Debug, Clone)] +pub struct ApiError { + pub status: u16, + pub headers: reqwest::header::HeaderMap, + pub body: String, + pub typed: Option, + pub parse_error: Option, +} +impl ApiError { pub fn is_client_error(&self) -> bool { - matches!(self, Self::Http { status, .. } if * status >= 400 && * status < 500) + (400..500).contains(&self.status) } - /// Check if this is a server error (5xx) pub fn is_server_error(&self) -> bool { - matches!(self, Self::Http { status, .. } if * status >= 500 && * status < 600) + (500..600).contains(&self.status) } - /// Check if this error is retryable + /// Retry guidance for the response. Mirrors the previous + /// HttpError logic for backwards-compatible retry middleware. pub fn is_retryable(&self) -> bool { + matches!(self.status, 429 | 500 | 502 | 503 | 504) + } +} +impl std::fmt::Display for ApiError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "API error {}: {}", self.status, self.body) + } +} +impl std::error::Error for ApiError {} +/// Result error type returned by every generated operation method. +/// +/// `Transport` covers failures where we never got an inspectable +/// response (network, timeout, middleware, request-side +/// serialization). `Api` covers any case where the server *did* +/// respond — the envelope always carries status + headers + raw +/// body even when the typed deserialize fails. +#[derive(Debug, Error)] +pub enum ApiOpError { + #[error(transparent)] + Transport(#[from] HttpError), + #[error(transparent)] + Api(ApiError), +} +impl ApiOpError { + /// Returns the API envelope when this is an `Api` variant. + pub fn api(&self) -> Option<&ApiError> { match self { - Self::Network(_) => true, - Self::Middleware(_) => true, - Self::Timeout => true, - Self::Http { status, .. } => matches!(status, 429 | 500 | 502 | 503 | 504), - _ => false, + Self::Api(e) => Some(e), + Self::Transport(_) => None, } } + /// True when the underlying error came from the server (i.e. + /// any `Api` variant) rather than the transport layer. + pub fn is_api_error(&self) -> bool { + matches!(self, Self::Api(_)) + } } -/// Result type for HTTP operations +impl From for ApiOpError { + fn from(e: reqwest::Error) -> Self { + Self::Transport(HttpError::Network(e)) + } +} +impl From for ApiOpError { + fn from(e: reqwest_middleware::Error) -> Self { + Self::Transport(HttpError::Middleware(e)) + } +} +/// Result alias for transport-only error paths (e.g. helpers that +/// don't have a per-operation error type). Generated operation +/// methods use [`ApiOpError`] directly. pub type HttpResult = Result; /// Retry configuration for HTTP requests #[derive(Debug, Clone)] @@ -162,17 +210,22 @@ impl HttpClient { self } } +impl Default for HttpClient { + fn default() -> Self { + Self::new() + } +} impl HttpClient { ///POST /todos - pub async fn create_todo(&self, request: CreateTodoRequest) -> HttpResult { - let url = format!("{}{}", self.base_url, "/todos"); + pub async fn create_todo( + &self, + request: CreateTodoRequest, + ) -> Result> { + let request_url = format!("{}{}", self.base_url, "/todos"); let mut req = self .http_client - .post(url) - .body( - serde_json::to_vec(&request) - .map_err(|e| HttpError::serialization_error(e))?, - ) + .post(request_url) + .body(serde_json::to_vec(&request).map_err(HttpError::serialization_error)?) .header("content-type", "application/json"); if let Some(api_key) = &self.api_key { req = req.bearer_auth(api_key); @@ -182,23 +235,66 @@ impl HttpClient { } let response = req.send().await?; let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response + .text() + .await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { - let body = response - .json() - .await - .map_err(|e| HttpError::deserialization_error(e))?; - Ok(body) + match serde_json::from_str(&body_text) { + Ok(body) => Ok(body), + Err(e) => { + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers: headers, + body: body_text, + typed: None, + parse_error: Some( + format!("failed to deserialize 2xx response body: {}", e), + ), + }), + ) + } + } } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option; + let parse_error: Option; + match status_code { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + }), + ) } } ///DELETE /todos/{id} - pub async fn delete_todo(&self) -> HttpResult<()> { - let url = format!("{}{}", self.base_url, "/todos/{id}"); - let mut req = self.http_client.delete(url); + pub async fn delete_todo( + &self, + id: impl AsRef, + ) -> Result<(), ApiOpError> { + let request_url = format!( + "{}{}", self.base_url, format!("/todos/{}", id.as_ref()) + ); + let mut req = self.http_client.delete(request_url); if let Some(api_key) = &self.api_key { req = req.bearer_auth(api_key); } @@ -207,23 +303,53 @@ impl HttpClient { } let response = req.send().await?; let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response + .text() + .await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { - let body = response - .json() - .await - .map_err(|e| HttpError::deserialization_error(e))?; - Ok(body) + let _ = body_text; + let _ = headers; + Ok(()) } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option; + let parse_error: Option; + match status_code { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + }), + ) } } ///GET /todos/{id} - pub async fn get_todo(&self) -> HttpResult { - let url = format!("{}{}", self.base_url, "/todos/{id}"); - let mut req = self.http_client.get(url); + pub async fn get_todo( + &self, + id: impl AsRef, + ) -> Result> { + let request_url = format!( + "{}{}", self.base_url, format!("/todos/{}", id.as_ref()) + ); + let mut req = self.http_client.get(request_url); if let Some(api_key) = &self.api_key { req = req.bearer_auth(api_key); } @@ -232,23 +358,63 @@ impl HttpClient { } let response = req.send().await?; let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response + .text() + .await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { - let body = response - .json() - .await - .map_err(|e| HttpError::deserialization_error(e))?; - Ok(body) + match serde_json::from_str(&body_text) { + Ok(body) => Ok(body), + Err(e) => { + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers: headers, + body: body_text, + typed: None, + parse_error: Some( + format!("failed to deserialize 2xx response body: {}", e), + ), + }), + ) + } + } } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option; + let parse_error: Option; + match status_code { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + }), + ) } } ///GET /todos - pub async fn list_todos(&self) -> HttpResult<()> { - let url = format!("{}{}", self.base_url, "/todos"); - let mut req = self.http_client.get(url); + pub async fn list_todos( + &self, + ) -> Result> { + let request_url = format!("{}{}", self.base_url, "/todos"); + let mut req = self.http_client.get(request_url); if let Some(api_key) = &self.api_key { req = req.bearer_auth(api_key); } @@ -257,29 +423,70 @@ impl HttpClient { } let response = req.send().await?; let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response + .text() + .await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { - let body = response - .json() - .await - .map_err(|e| HttpError::deserialization_error(e))?; - Ok(body) + match serde_json::from_str(&body_text) { + Ok(body) => Ok(body), + Err(e) => { + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers: headers, + body: body_text, + typed: None, + parse_error: Some( + format!("failed to deserialize 2xx response body: {}", e), + ), + }), + ) + } + } } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option; + let parse_error: Option; + match status_code { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + }), + ) } } ///PUT /todos/{id} - pub async fn update_todo(&self, request: UpdateTodoRequest) -> HttpResult { - let url = format!("{}{}", self.base_url, "/todos/{id}"); + pub async fn update_todo( + &self, + id: impl AsRef, + request: UpdateTodoRequest, + ) -> Result> { + let request_url = format!( + "{}{}", self.base_url, format!("/todos/{}", id.as_ref()) + ); let mut req = self .http_client - .put(url) - .body( - serde_json::to_vec(&request) - .map_err(|e| HttpError::serialization_error(e))?, - ) + .put(request_url) + .body(serde_json::to_vec(&request).map_err(HttpError::serialization_error)?) .header("content-type", "application/json"); if let Some(api_key) = &self.api_key { req = req.bearer_auth(api_key); @@ -289,17 +496,55 @@ impl HttpClient { } let response = req.send().await?; let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response + .text() + .await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { - let body = response - .json() - .await - .map_err(|e| HttpError::deserialization_error(e))?; - Ok(body) + match serde_json::from_str(&body_text) { + Ok(body) => Ok(body), + Err(e) => { + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers: headers, + body: body_text, + typed: None, + parse_error: Some( + format!("failed to deserialize 2xx response body: {}", e), + ), + }), + ) + } + } } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option; + let parse_error: Option; + match status_code { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + Err( + ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + }), + ) } } } diff --git a/examples/generated/mod.rs b/examples/generated/mod.rs index eab4700..113f3af 100644 --- a/examples/generated/mod.rs +++ b/examples/generated/mod.rs @@ -1,8 +1,10 @@ //! Generated API modules -//! +//! //! This module exports all generated API types and clients. //! Do not edit manually - regenerate using the appropriate script. +#![allow(unused_imports)] + pub mod types; pub mod client; diff --git a/examples/generated/types.rs b/examples/generated/types.rs index 915eec5..abaf315 100644 --- a/examples/generated/types.rs +++ b/examples/generated/types.rs @@ -3,26 +3,29 @@ //! This file contains all the generated types for the API. //! Do not edit manually - regenerate using the appropriate script. #![allow(clippy::large_enum_variant)] +#![allow(clippy::format_in_format_args)] +#![allow(clippy::let_unit_value)] #![allow(unreachable_patterns)] use serde::{Deserialize, Serialize}; +pub type ListTodosResponse = Vec; #[derive(Debug, Clone, Deserialize, Serialize)] -pub struct CreateTodoRequest { +pub struct Todo { + ///Whether the todo is completed + #[serde(default)] + pub completed: bool, ///Optional description #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, + ///Unique identifier for the todo + pub id: String, ///The todo item title pub title: String, } #[derive(Debug, Clone, Deserialize, Serialize)] -pub struct Todo { - ///Whether the todo is completed - #[serde(default)] - pub completed: bool, +pub struct CreateTodoRequest { ///Optional description #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, - ///Unique identifier for the todo - pub id: String, ///The todo item title pub title: String, } diff --git a/src/analysis.rs b/src/analysis.rs index 282b76e..5b2c5e9 100644 --- a/src/analysis.rs +++ b/src/analysis.rs @@ -3662,14 +3662,32 @@ impl SchemaAnalyzer { Ok(op_info) } - /// Generate a type name for an inline response schema - fn generate_inline_response_type_name(&self, operation_id: &str, _status_code: &str) -> String { + /// Generate a type name for an inline response schema. + /// + /// 200 (the canonical success status) keeps the unsuffixed `{Op}Response` + /// name so simple specs and existing snapshots are unchanged. Every other + /// status code is disambiguated by suffix so that multi-response operations + /// (e.g. 200 + 400) don't collide in the schema registry — see issue #8. + fn generate_inline_response_type_name(&self, operation_id: &str, status_code: &str) -> String { use heck::ToPascalCase; - // Convert operation_id to PascalCase and append Response - // e.g., "app.skills" -> "AppSkillsResponse" - // e.g., "getUser" + "200" -> "GetUserResponse" let base_name = operation_id.replace('.', "_").to_pascal_case(); - format!("{}Response", base_name) + let suffix = Self::status_code_suffix(status_code); + format!("{}Response{}", base_name, suffix) + } + + /// Map an OpenAPI status code key to a suffix for generated type names. + /// + /// "200" → "" (unchanged, the dominant case) + /// "201", "400", "404" → "201", "400", "404" + /// "default" → "Default" + /// "4XX" / "4xx" → "4xx" (lowercased range form) + fn status_code_suffix(status_code: &str) -> String { + match status_code { + "" | "200" => String::new(), + "default" | "Default" => "Default".to_string(), + other if other.chars().all(|c| c.is_ascii_digit()) => other.to_string(), + other => other.to_ascii_lowercase(), + } } /// Generate a type name for an inline request body schema diff --git a/src/client_generator.rs b/src/client_generator.rs index b23a6b6..949b157 100644 --- a/src/client_generator.rs +++ b/src/client_generator.rs @@ -368,8 +368,18 @@ impl CodeGenerator { } } - /// Generate HTTP operation methods for the client + /// Generate HTTP operation methods for the client. + /// + /// Emits per-operation typed error enums (one variant per declared non-2xx + /// response with a body schema) BEFORE the `impl HttpClient` block so the + /// generated method signatures can reference them. pub fn generate_operation_methods(&self, analysis: &SchemaAnalysis) -> TokenStream { + let op_error_enums: Vec = analysis + .operations + .values() + .filter_map(|op| self.generate_op_error_enum(op)) + .collect(); + let methods: Vec = analysis .operations .values() @@ -377,12 +387,92 @@ impl CodeGenerator { .collect(); quote! { + #(#op_error_enums)* + impl HttpClient { #(#methods)* } } } + /// Generate the per-operation typed error enum, if the op has any non-2xx + /// responses with a body schema. Returns None when the op has no declared + /// error bodies — those operations use `ApiOpError` so + /// the raw response body is still inspectable. + fn generate_op_error_enum(&self, op: &OperationInfo) -> Option { + let variants: Vec<(String, String)> = op + .response_schemas + .iter() + .filter(|(code, _)| !code.starts_with('2')) + .map(|(code, schema)| (code.clone(), schema.clone())) + .collect(); + + if variants.is_empty() { + return None; + } + + let enum_ident = self.op_error_enum_ident(op); + let variant_decls: Vec = variants + .iter() + .map(|(code, schema)| { + let variant_ident = Self::op_error_variant_ident(code); + let payload_ty_name = self.to_rust_type_name(schema); + let payload_ty = syn::Ident::new(&payload_ty_name, proc_macro2::Span::call_site()); + quote! { #variant_ident(#payload_ty) } + }) + .collect(); + + let doc = format!( + "Typed error responses for `{}`. One variant per declared non-2xx response.", + op.operation_id + ); + + Some(quote! { + #[doc = #doc] + #[derive(Debug, Clone)] + pub enum #enum_ident { + #(#variant_decls,)* + } + }) + } + + /// Type name (Ident) for the per-op error enum, e.g. `ListTodosApiError`. + fn op_error_enum_ident(&self, op: &OperationInfo) -> syn::Ident { + use heck::ToPascalCase; + let name = format!( + "{}ApiError", + op.operation_id.replace('.', "_").to_pascal_case() + ); + syn::Ident::new(&name, proc_macro2::Span::call_site()) + } + + /// Variant name for a status code: "400" → Status400, "default" → Default, + /// "4XX" → Status4xx. + fn op_error_variant_ident(status_code: &str) -> syn::Ident { + let raw = match status_code { + "default" | "Default" => "Default".to_string(), + other if other.chars().all(|c| c.is_ascii_digit()) => format!("Status{other}"), + other => format!("Status{}", other.to_ascii_lowercase()), + }; + syn::Ident::new(&raw, proc_macro2::Span::call_site()) + } + + /// Token stream for the type plugged into `ApiOpError` for an op: + /// either the per-op enum, or `serde_json::Value` for ops with no + /// declared error body schemas. + fn op_error_type_token(&self, op: &OperationInfo) -> TokenStream { + if op + .response_schemas + .iter() + .any(|(code, _)| !code.starts_with('2')) + { + let ident = self.op_error_enum_ident(op); + quote! { #ident } + } else { + quote! { serde_json::Value } + } + } + /// Generate a single operation method fn generate_single_operation_method(&self, op: &OperationInfo) -> TokenStream { let method_name = self.get_method_name(op); @@ -393,7 +483,8 @@ impl CodeGenerator { let query_params = self.generate_query_params(op); let response_type = self.get_response_type(op); let has_response_body = self.get_success_response_schema(op).is_some(); - let error_handling = self.generate_error_handling(has_response_body); + let op_error_type = self.op_error_type_token(op); + let error_handling = self.generate_error_handling(op, has_response_body); let url_construction = self.generate_url_construction(path, op); let doc_comment = self.generate_operation_doc_comment(op); @@ -402,7 +493,7 @@ impl CodeGenerator { pub async fn #method_name( &self, #request_param - ) -> HttpResult<#response_type> { + ) -> Result<#response_type, ApiOpError<#op_error_type>> { #url_construction let mut req = self.http_client @@ -679,31 +770,151 @@ impl CodeGenerator { /// Generate error handling. /// - /// When `has_response_body` is false the endpoint returns no JSON body - /// (e.g. 204 No Content) and we skip deserialization entirely. - fn generate_error_handling(&self, has_response_body: bool) -> TokenStream { + /// Always reads the response body to a string before attempting any typed + /// deserialization, so the raw body and headers are preserved on the error + /// path even when JSON parsing fails. On 2xx the body is parsed into the + /// success type; on non-2xx the body is parsed into the matching variant + /// of the per-operation error enum (when one is declared) and wrapped in + /// `ApiError`. + fn generate_error_handling(&self, op: &OperationInfo, has_response_body: bool) -> TokenStream { + let op_error_type = self.op_error_type_token(op); + let success_branch = if has_response_body { quote! { - let body = response.json().await - .map_err(HttpError::deserialization_error)?; - Ok(body) + match serde_json::from_str(&body_text) { + Ok(body) => Ok(body), + Err(e) => Err(ApiOpError::Api(ApiError { + status: status_code, + headers: headers, + body: body_text, + typed: None, + parse_error: Some(format!( + "failed to deserialize 2xx response body: {}", + e + )), + })), + } } } else { quote! { + let _ = body_text; + let _ = headers; Ok(()) } }; + let error_match_arms = self.generate_error_match_arms(op); + quote! { let status = response.status(); + let status_code = status.as_u16(); + let headers = response.headers().clone(); + let body_text = response.text().await + .map_err(|e| ApiOpError::Transport(HttpError::Network(e)))?; if status.is_success() { #success_branch } else { - let status_code = status.as_u16(); - let message = status.canonical_reason().unwrap_or("Unknown error"); - let body = response.text().await.ok(); - Err(HttpError::from_status(status_code, message, body)) + let typed: Option<#op_error_type>; + let parse_error: Option; + #error_match_arms + Err(ApiOpError::Api(ApiError { + status: status_code, + headers, + body: body_text, + typed, + parse_error, + })) + } + } + } + + /// Generate the match arms that select which per-op error variant to + /// deserialize the response body into based on the runtime status code. + fn generate_error_match_arms(&self, op: &OperationInfo) -> TokenStream { + let arms: Vec = op + .response_schemas + .iter() + .filter(|(code, _)| !code.starts_with('2')) + .filter_map(|(code, schema)| { + let variant_ident = Self::op_error_variant_ident(code); + let payload_ty_name = self.to_rust_type_name(schema); + let payload_ty = syn::Ident::new(&payload_ty_name, proc_macro2::Span::call_site()); + let enum_ident = self.op_error_enum_ident(op); + + let pattern = match code.as_str() { + "default" | "Default" => return None, // handled in fallback + other if other.chars().all(|c| c.is_ascii_digit()) => { + let n: u16 = other.parse().ok()?; + quote! { #n } + } + // Range like "4XX" / "5XX" — fall through to generic + // for now; declared-range handling is a follow-up. + _ => return None, + }; + + Some(quote! { + #pattern => { + match serde_json::from_str::<#payload_ty>(&body_text) { + Ok(v) => { + typed = Some(#enum_ident::#variant_ident(v)); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + }) + }) + .collect(); + + // Fallback for "default" or undeclared status codes: try to parse + // as `serde_json::Value` for inspectability when the op's error + // type is generic, otherwise leave typed = None. + let has_typed_enum = op.response_schemas.iter().any(|(code, _)| { + !code.starts_with('2') && !matches!(code.as_str(), "default" | "Default") + }); + + let default_arm = if has_typed_enum { + quote! { + _ => { + typed = None; + parse_error = None; + } + } + } else { + // No typed enum — op_error_type is serde_json::Value. + quote! { + _ => { + match serde_json::from_str::(&body_text) { + Ok(v) => { + typed = Some(v); + parse_error = None; + } + Err(e) => { + typed = None; + parse_error = Some(e.to_string()); + } + } + } + } + }; + + if arms.is_empty() { + // No declared status arms — just the fallback. + quote! { + match status_code { + #default_arm + } + } + } else { + quote! { + match status_code { + #(#arms)* + #default_arm + } } } } diff --git a/src/generator.rs b/src/generator.rs index 987b0c6..572a7c4 100644 --- a/src/generator.rs +++ b/src/generator.rs @@ -381,7 +381,13 @@ impl CodeGenerator { quote! { use thiserror::Error; - /// HTTP client errors that can occur during API requests + /// Transport-level errors: failures where we never received an + /// inspectable HTTP response from the server. + /// + /// HTTP responses with non-2xx status codes are surfaced as + /// [`ApiError`] inside [`ApiOpError::Api`], not here, so callers can + /// always inspect status, headers, and the raw body when the server + /// actually responded. #[derive(Error, Debug)] pub enum HttpError { /// Network or connection error (from reqwest) @@ -396,18 +402,6 @@ impl CodeGenerator { #[error("Failed to serialize request: {0}")] Serialization(String), - /// Response deserialization error - #[error("Failed to deserialize response: {0}")] - Deserialization(String), - - /// HTTP error response (4xx, 5xx) - #[error("HTTP error {status}: {message}")] - Http { - status: u16, - message: String, - body: Option, - }, - /// Authentication error #[error("Authentication error: {0}")] Auth(String), @@ -426,51 +420,108 @@ impl CodeGenerator { } impl HttpError { - /// Create an HTTP error from a status code and message - pub fn from_status(status: u16, message: impl Into, body: Option) -> Self { - Self::Http { - status, - message: message.into(), - body, - } - } - /// Create a serialization error pub fn serialization_error(error: impl std::fmt::Display) -> Self { Self::Serialization(error.to_string()) } - /// Create a deserialization error - pub fn deserialization_error(error: impl std::fmt::Display) -> Self { - Self::Deserialization(error.to_string()) + /// Check if this transport error is retryable + pub fn is_retryable(&self) -> bool { + matches!(self, Self::Network(_) | Self::Middleware(_) | Self::Timeout) } + } - /// Check if this is a client error (4xx) + /// Envelope returned for any HTTP response that we received but + /// couldn't (or didn't) treat as a successful typed result. + /// + /// Includes both non-2xx responses and 2xx responses whose body + /// failed to deserialize into the expected success type. `status`, + /// `headers`, and `body` are always populated so callers can + /// inspect what the server sent without modifying the generated + /// code. `typed` carries the parsed per-operation error variant + /// when the body matched a declared schema. + #[derive(Debug, Clone)] + pub struct ApiError { + pub status: u16, + pub headers: reqwest::header::HeaderMap, + pub body: String, + pub typed: Option, + pub parse_error: Option, + } + + impl ApiError { pub fn is_client_error(&self) -> bool { - matches!(self, Self::Http { status, .. } if *status >= 400 && *status < 500) + (400..500).contains(&self.status) } - /// Check if this is a server error (5xx) pub fn is_server_error(&self) -> bool { - matches!(self, Self::Http { status, .. } if *status >= 500 && *status < 600) + (500..600).contains(&self.status) } - /// Check if this error is retryable + /// Retry guidance for the response. Mirrors the previous + /// HttpError logic for backwards-compatible retry middleware. pub fn is_retryable(&self) -> bool { + matches!(self.status, 429 | 500 | 502 | 503 | 504) + } + } + + impl std::fmt::Display for ApiError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "API error {}: {}", self.status, self.body) + } + } + + impl std::error::Error for ApiError {} + + /// Result error type returned by every generated operation method. + /// + /// `Transport` covers failures where we never got an inspectable + /// response (network, timeout, middleware, request-side + /// serialization). `Api` covers any case where the server *did* + /// respond — the envelope always carries status + headers + raw + /// body even when the typed deserialize fails. + #[derive(Debug, Error)] + pub enum ApiOpError { + #[error(transparent)] + Transport(#[from] HttpError), + + #[error(transparent)] + Api(ApiError), + } + + impl ApiOpError { + /// Returns the API envelope when this is an `Api` variant. + pub fn api(&self) -> Option<&ApiError> { match self { - Self::Network(_) => true, - Self::Middleware(_) => true, - Self::Timeout => true, - Self::Http { status, .. } => { - // Retry on 429 (rate limit), 500, 502, 503, 504 - matches!(status, 429 | 500 | 502 | 503 | 504) - } - _ => false, + Self::Api(e) => Some(e), + Self::Transport(_) => None, } } + + /// True when the underlying error came from the server (i.e. + /// any `Api` variant) rather than the transport layer. + pub fn is_api_error(&self) -> bool { + matches!(self, Self::Api(_)) + } + } + + // Direct From impls so `?` works without going through HttpError + // first. Rust's `?` only chains a single `From` conversion. + impl From for ApiOpError { + fn from(e: reqwest::Error) -> Self { + Self::Transport(HttpError::Network(e)) + } + } + + impl From for ApiOpError { + fn from(e: reqwest_middleware::Error) -> Self { + Self::Transport(HttpError::Middleware(e)) + } } - /// Result type for HTTP operations + /// Result alias for transport-only error paths (e.g. helpers that + /// don't have a per-operation error type). Generated operation + /// methods use [`ApiOpError`] directly. pub type HttpResult = Result; } } diff --git a/src/http_error.rs b/src/http_error.rs index ef6f470..f4afaa8 100644 --- a/src/http_error.rs +++ b/src/http_error.rs @@ -203,3 +203,63 @@ impl HttpError { /// Result type for HTTP operations pub type HttpResult = Result; + +/// Envelope for an API response we received but couldn't (or didn't) treat as success. +/// +/// `ApiError` is returned whenever the server actually responded — whether the +/// status was non-2xx, or the 2xx body failed to deserialize into the expected +/// type. `status`, `headers`, and `body` are always populated so callers can +/// inspect what the server actually sent without having to hack the generated +/// client. `typed` is `Some(_)` when the raw body was successfully parsed into a +/// per-operation error type; `parse_error` records why parsing failed when not. +#[derive(Debug, Clone)] +pub struct ApiError { + pub status: u16, + pub headers: reqwest::header::HeaderMap, + pub body: String, + pub typed: Option, + pub parse_error: Option, +} + +impl ApiError { + pub fn is_client_error(&self) -> bool { + (400..500).contains(&self.status) + } + + pub fn is_server_error(&self) -> bool { + (500..600).contains(&self.status) + } +} + +impl std::fmt::Display for ApiError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "API error {}: {}", self.status, self.body) + } +} + +impl std::error::Error for ApiError {} + +/// Result error type for generated operation methods. +/// +/// `Transport` covers failures where the request never produced a response we +/// can inspect (network, timeout, middleware, request-side serialization). +/// `Api` covers any case where the server *did* respond — the envelope always +/// carries status + headers + raw body even when the typed deserialize fails. +#[derive(Debug, thiserror::Error)] +pub enum ApiOpError { + #[error(transparent)] + Transport(#[from] HttpError), + + #[error(transparent)] + Api(ApiError), +} + +impl ApiOpError { + /// Convenience accessor: if this is an Api variant, return the envelope. + pub fn api(&self) -> Option<&ApiError> { + match self { + Self::Api(e) => Some(e), + Self::Transport(_) => None, + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 5f30292..6970263 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -18,7 +18,7 @@ pub use config::ConfigFile; pub use error::GeneratorError; pub use generator::{CodeGenerator, GeneratedFile, GenerationResult, GeneratorConfig}; pub use http_config::{AuthConfig, HttpClientConfig, RetryConfig}; -pub use http_error::{HttpError, HttpResult}; +pub use http_error::{ApiError, ApiOpError, HttpError, HttpResult}; pub use openapi::{OpenApiSpec, Schema, SchemaType}; pub type Result = std::result::Result; diff --git a/tests/full_client_generation_test.rs b/tests/full_client_generation_test.rs index 9b8acb3..3a0dab5 100644 --- a/tests/full_client_generation_test.rs +++ b/tests/full_client_generation_test.rs @@ -402,7 +402,7 @@ fn test_full_client_includes_errors() { .generate_http_client(&analysis) .expect("Failed to generate HTTP client"); - // Verify all error types are present + // Verify transport-error machinery is present assert!( client_code.contains("pub enum HttpError"), "Should include HttpError enum" @@ -416,22 +416,34 @@ fn test_full_client_includes_errors() { "Should include Serialization error variant" ); assert!( - client_code.contains("Deserialization(String)"), - "Should include Deserialization error variant" + client_code.contains("pub type HttpResult"), + "Should include HttpResult type alias" ); + + // Verify the new typed-response error envelope and operation result + // wrapper are emitted (replaces the old `HttpError::Http { ... }` + // variant — see issue #8 for the design). assert!( - client_code.contains("Http {"), - "Should include Http error variant with status and message" + client_code.contains("pub struct ApiError"), + "Should include ApiError envelope struct" ); assert!( - client_code.contains("pub type HttpResult"), - "Should include HttpResult type alias" + client_code.contains("pub enum ApiOpError"), + "Should include ApiOpError enum" + ); + assert!( + client_code.contains("Transport(#[from] HttpError)"), + "ApiOpError should have Transport variant carrying HttpError" ); - // Verify helper methods + // Verify helper methods on the new envelope + assert!( + client_code.contains("pub fn is_client_error"), + "Should include is_client_error helper" + ); assert!( - client_code.contains("pub fn from_status"), - "Should include from_status helper" + client_code.contains("pub fn is_server_error"), + "Should include is_server_error helper" ); assert!( client_code.contains("pub fn is_retryable"), diff --git a/tests/http_error_test.rs b/tests/http_error_test.rs index 44bd7d0..b5b659f 100644 --- a/tests/http_error_test.rs +++ b/tests/http_error_test.rs @@ -247,13 +247,11 @@ fn test_generated_error_code() { let client_content = &client_file.expect("client file").content; - // Verify the generated code contains HttpError enum + // Transport-level error machinery assert!( client_content.contains("pub enum HttpError"), "Generated code should contain HttpError enum" ); - - // Verify error variants assert!( client_content.contains("Network(#[from] reqwest::Error)"), "Generated code should contain Network variant" @@ -262,26 +260,6 @@ fn test_generated_error_code() { client_content.contains("Serialization(String)"), "Generated code should contain Serialization variant" ); - assert!( - client_content.contains("Deserialization(String)"), - "Generated code should contain Deserialization variant" - ); - assert!( - client_content.contains("Http {"), - "Generated code should contain Http variant" - ); - assert!( - client_content.contains("status: u16"), - "Generated code should contain status field in Http variant" - ); - assert!( - client_content.contains("message: String"), - "Generated code should contain message field in Http variant" - ); - assert!( - client_content.contains("body: Option"), - "Generated code should contain body field in Http variant" - ); assert!( client_content.contains("Auth(String)"), "Generated code should contain Auth variant" @@ -299,18 +277,25 @@ fn test_generated_error_code() { "Generated code should contain Other variant" ); - // Verify helper methods + // New typed-response envelope (replaces the old HttpError::Http variant + // and from_status helper — see issue #8). assert!( - client_content.contains("pub fn from_status"), - "Generated code should contain from_status method" + client_content.contains("pub struct ApiError"), + "Generated code should contain ApiError struct" ); assert!( - client_content.contains("pub fn serialization_error"), - "Generated code should contain serialization_error method" + client_content.contains("pub enum ApiOpError"), + "Generated code should contain ApiOpError enum" ); assert!( - client_content.contains("pub fn deserialization_error"), - "Generated code should contain deserialization_error method" + client_content.contains("Transport(#[from] HttpError)"), + "ApiOpError should have Transport variant carrying HttpError" + ); + + // Helpers on the new envelope + assert!( + client_content.contains("pub fn serialization_error"), + "Generated code should contain serialization_error method" ); assert!( client_content.contains("pub fn is_client_error"), diff --git a/tests/inline_response_schema_test.rs b/tests/inline_response_schema_test.rs index f00f4b6..b91d809 100644 --- a/tests/inline_response_schema_test.rs +++ b/tests/inline_response_schema_test.rs @@ -120,6 +120,113 @@ fn test_inline_object_response_analyzed() { insta::assert_debug_snapshot!(op.response_schemas); } +#[test] +fn test_multiple_inline_responses_have_distinct_schemas() { + // Regression test for issue #8: when an operation declares multiple responses + // with inline schemas (e.g. 200 success + 400 error), each must produce a + // distinct registered schema. Previously the inline naming function ignored + // the status code, so the 400 schema overwrote the 200 in the schema registry + // and the generated client tried to deserialize success bodies as the error + // shape. + let spec = serde_json::json!({ + "openapi": "3.1.0", + "info": { "title": "Test API", "version": "1.0.0" }, + "paths": { + "/todos": { + "get": { + "operationId": "listTodos", + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { "type": "string" }, + "title": { "type": "string" } + }, + "required": ["id", "title"] + } + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "error": { "type": "string" } + }, + "required": ["error"] + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Placeholder": { + "type": "object", + "properties": { "id": { "type": "string" } } + } + } + } + }); + + let mut analyzer = SchemaAnalyzer::new(spec).expect("Failed to create analyzer"); + let analysis = analyzer.analyze().expect("Failed to analyze"); + + let op = analysis + .operations + .get("listTodos") + .expect("Operation not found"); + + // Both status codes must be present. + let s200 = op + .response_schemas + .get("200") + .expect("200 response missing from response_schemas"); + let s400 = op + .response_schemas + .get("400") + .expect("400 response missing from response_schemas"); + + // The two responses must map to *distinct* registered schemas. + assert_ne!( + s200, s400, + "200 and 400 inline response schemas collided into the same name: {s200}" + ); + + // Both names must actually exist in the schema registry. + assert!( + analysis.schemas.contains_key(s200), + "200 schema {s200} not registered" + ); + assert!( + analysis.schemas.contains_key(s400), + "400 schema {s400} not registered" + ); + + // Sanity: the registered schemas should differ structurally — the 200 is an + // array wrapper / object with id+title, the 400 is an object with `error`. + let analyzed_200 = &analysis.schemas[s200]; + let analyzed_400 = &analysis.schemas[s400]; + assert_ne!( + format!("{:?}", analyzed_200.schema_type), + format!("{:?}", analyzed_400.schema_type), + "200 and 400 schemas resolved to the same SchemaType — collision not actually fixed" + ); +} + #[test] fn test_ref_response_still_works() { // Ensure we don't break the existing $ref handling diff --git a/tests/multi_response_client_test.rs b/tests/multi_response_client_test.rs new file mode 100644 index 0000000..84d11f0 --- /dev/null +++ b/tests/multi_response_client_test.rs @@ -0,0 +1,215 @@ +//! End-to-end test for the multi-response error envelope work in issue #8. +//! +//! These tests generate a complete client (types + http_client) for a spec +//! that exercises the new code paths — multiple responses with different +//! schemas, declared error bodies, and ops with no error bodies — and then +//! invoke `cargo check` on the generated crate to verify the emitted code is +//! not just syntactically valid Rust but actually compiles against the +//! reqwest / serde / thiserror runtime it depends on. + +use openapi_to_rust::{CodeGenerator, GeneratorConfig, SchemaAnalyzer}; +use std::fs; +use std::path::Path; +use std::process::Command; +use tempfile::TempDir; + +const CLIENT_CARGO_TOML: &str = r#" +[package] +name = "{name}" +version = "0.1.0" +edition = "2021" + +[dependencies] +serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" +reqwest = { version = "0.12", features = ["json", "multipart"] } +reqwest-middleware = { version = "0.4", features = ["multipart"] } +thiserror = "1.0" +tokio = { version = "1.0", features = ["full"] } +validator = { version = "0.20", features = ["derive"] } +"#; + +/// Generate types + http_client for a spec, write a compilable crate to a +/// temp dir, run `cargo check`, and panic if it fails. Returns the temp dir +/// so callers can keep it alive long enough for inspection on failure. +fn assert_generates_compiling_client(name: &str, spec: serde_json::Value) -> TempDir { + let mut analyzer = SchemaAnalyzer::new(spec).expect("analyzer"); + let mut analysis = analyzer.analyze().expect("analyze"); + + let config = GeneratorConfig { + module_name: name.to_string(), + enable_async_client: true, + tracing_enabled: false, + ..Default::default() + }; + let generator = CodeGenerator::new(config); + let types_code = generator.generate(&mut analysis).expect("generate types"); + let client_code = generator + .generate_http_client(&analysis) + .expect("generate http client"); + + let temp = TempDir::new().expect("temp dir"); + let temp_path = temp.path(); + + fs::write( + temp_path.join("Cargo.toml"), + CLIENT_CARGO_TOML.replace("{name}", name), + ) + .expect("write Cargo.toml"); + + let src = temp_path.join("src"); + fs::create_dir_all(&src).expect("create src"); + fs::write(src.join("types.rs"), types_code).expect("write types.rs"); + fs::write(src.join("client.rs"), client_code).expect("write client.rs"); + fs::write(src.join("lib.rs"), "pub mod types;\npub mod client;\n").expect("write lib.rs"); + + let output = Command::new("cargo") + .arg("check") + .arg("--quiet") + .current_dir(temp_path) + .output() + .expect("run cargo check"); + + if !output.status.success() { + eprintln!( + "STDOUT:\n{}\n\nSTDERR:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + panic!("Generated client for `{name}` failed to compile"); + } + temp +} + +fn multi_response_spec() -> serde_json::Value { + serde_json::json!({ + "openapi": "3.1.0", + "info": { "title": "Multi-Response Test", "version": "1.0.0" }, + "paths": { + "/todos": { + "get": { + "operationId": "listTodos", + "responses": { + "200": { + "description": "Success", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { "$ref": "#/components/schemas/Todo" } + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "application/json": { + "schema": { "$ref": "#/components/schemas/BadRequest" } + } + } + } + } + } + }, + "/ping": { + "get": { + "operationId": "ping", + "responses": { + "200": { + "description": "OK", + "content": { + "application/json": { + "schema": { "type": "string" } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "Todo": { + "type": "object", + "properties": { + "id": { "type": "string" }, + "title": { "type": "string" } + }, + "required": ["id", "title"] + }, + "BadRequest": { + "type": "object", + "properties": { + "error": { "type": "string" } + }, + "required": ["error"] + } + } + } + }) +} + +#[test] +fn test_generated_multi_response_client_compiles() { + let _temp = assert_generates_compiling_client("multi_response_client", multi_response_spec()); +} + +/// The anthropic and openai fixtures are realistic-shape API specs whose +/// generated clients must compile end-to-end. These regression tests catch +/// codegen breakage in non-trivial schema territory (oneOfs, discriminated +/// unions, etc) that the toy multi_response_spec doesn't exercise. +#[test] +fn test_generated_anthropic_client_compiles() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/anthropic.yml"); + let raw = fs::read_to_string(&path).expect("read anthropic fixture"); + let spec: serde_json::Value = serde_yaml::from_str(&raw).expect("parse anthropic fixture"); + let _temp = assert_generates_compiling_client("anthropic_client", spec); +} + +#[test] +fn test_generated_openai_client_compiles() { + let path = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/fixtures/openai-responses.json"); + let raw = fs::read_to_string(&path).expect("read openai fixture"); + let spec: serde_json::Value = serde_json::from_str(&raw).expect("parse openai fixture"); + let _temp = assert_generates_compiling_client("openai_client", spec); +} + +#[test] +fn test_generated_client_emits_per_op_error_enum() { + let spec = multi_response_spec(); + let mut analyzer = SchemaAnalyzer::new(spec).expect("analyzer"); + let analysis = analyzer.analyze().expect("analyze"); + + let config = GeneratorConfig { + module_name: "multi_response_client".to_string(), + enable_async_client: true, + ..Default::default() + }; + let generator = CodeGenerator::new(config); + let client_code = generator + .generate_http_client(&analysis) + .expect("generate http client"); + + // The op with a 400 schema should get its own typed error enum. + assert!( + client_code.contains("pub enum ListTodosApiError"), + "Expected ListTodosApiError enum in generated client. Code:\n{client_code}" + ); + assert!( + client_code.contains("Status400(BadRequest)"), + "Expected Status400(BadRequest) variant. Code:\n{client_code}" + ); + + // The signature should plug the per-op error type into ApiOpError. + assert!( + client_code.contains("ApiOpError"), + "list_todos signature should use ApiOpError. Code:\n{client_code}" + ); + + // The op without any non-2xx schemas should fall back to serde_json::Value. + assert!( + client_code.contains("ApiOpError"), + "ping signature should use ApiOpError. Code:\n{client_code}" + ); +} diff --git a/tests/operation_generation_test.rs b/tests/operation_generation_test.rs index d751cba..8f7d779 100644 --- a/tests/operation_generation_test.rs +++ b/tests/operation_generation_test.rs @@ -60,8 +60,8 @@ fn test_generate_get_operation() { assert!(result_str.contains("get_user")); // Verify HTTP method is GET assert!(result_str.contains(". get (request_url)")); - // Verify response type - assert!(result_str.contains("HttpResult < User >")); + // Verify response type — see issue #8 for the ApiOpError envelope. + assert!(result_str.contains("Result < User , ApiOpError <")); // Verify no request body parameter assert!(!result_str.contains("request :")); } @@ -103,8 +103,8 @@ fn test_generate_post_operation() { // Verify request body serialization (middleware-compatible) assert!(result_str.contains("serde_json :: to_vec (& request)")); assert!(result_str.contains("application/json")); - // Verify response type - assert!(result_str.contains("HttpResult < User >")); + // Verify response type — see issue #8 for the ApiOpError envelope. + assert!(result_str.contains("Result < User , ApiOpError <")); } #[test] @@ -271,8 +271,9 @@ fn test_method_with_response_type() { let result = generator.generate_operation_methods(&analysis); let result_str = result.to_string(); - // Verify response type is correctly mapped - assert!(result_str.contains("HttpResult < DataResponse >")); + // Verify response type is correctly mapped — see issue #8 for the + // ApiOpError envelope. + assert!(result_str.contains("Result < DataResponse , ApiOpError <")); } #[test] @@ -299,12 +300,13 @@ fn test_method_without_response_type() { let result = generator.generate_operation_methods(&analysis); let result_str = result.to_string(); - // Verify unit type for no response - assert!(result_str.contains("HttpResult < () >")); + // Verify unit type for no response — see issue #8 for the ApiOpError + // envelope. + assert!(result_str.contains("Result < () , ApiOpError <")); // Verify no deserialization for empty response (should use Ok(()) directly) assert!( - !result_str.contains("deserialization_error"), + !result_str.contains("failed to deserialize 2xx response body"), "Empty response type should not attempt JSON deserialization" ); assert!( @@ -339,11 +341,13 @@ fn test_error_handling_generation() { let result = generator.generate_operation_methods(&analysis); let result_str = result.to_string(); - // Verify error handling logic + // Verify error handling logic — the new envelope reads the body to a + // string before deserializing and wraps non-2xx (and 2xx parse failures) + // in `ApiOpError::Api(ApiError { ... })`. See issue #8. assert!(result_str.contains("let status = response . status ()")); assert!(result_str.contains("if status . is_success ()")); - assert!(result_str.contains("HttpError :: deserialization_error")); - assert!(result_str.contains("HttpError :: from_status")); + assert!(result_str.contains("response . text ()")); + assert!(result_str.contains("ApiOpError :: Api (ApiError")); } #[test]