Multiple Operation Responses Appear to Not Be Handled

[atom-mfeldkamp]

Found another interesting issue: it looks like when an endpoint/operation has multiple response entries, only one (the last one?) gets included in the endpoint's Response type.

For example, in the generate_full_client.rs example, I added this to "paths"."/todos"."get"."responses" :

"responses": {
    "200": {
        "description": "Success",
        "content": {
            "application/json": {
                "schema": {
                    "type": "array",
                    "items": {
                        "$ref": "#/components/schemas/Todo"
                    }
                }
            }
        }
    },
    "400": {
        "description": "Bad Request",
        "content": {
            "application/json": {
                "schema": {
                    "type": "object",
                    "properties": {
                        "error": {
                            "type": "string"
                        }
                    }
                }
            }
        }
    }
}

Before the change it generates:

pub type ListTodosResponse = Vec<Todo>;

And after adding the 400 response, it generates the following rust type:

#[derive(Debug, Clone, Deserialize, Serialize)]
pub struct ListTodosResponse {
    #[serde(skip_serializing_if = "Option::is_none")]
    pub error: Option<String>,
}
// No indication of the 200 version anywhere to be seen

The Todo struct for the 200 response is still generated, but there's no Response type containing it. In practice, this leads to a somewhat surprising (de)serialization error when making calls to the API, because the generated client code tries to deserialize the normal response body in the shape of the error response body type (which obviously won't work).

Minor side tangent: What would you think of attaching the raw response body text / headers / whatever else to the client error so it can be inspected? It would be very useful to see what the API responded with when there's a deserialization error like this. Could help determine if there's a problem with the API itself or how I'm calling into it. I was able to get the raw response body text by hacking the generated code a little bit, but I'd rather not do that.

[lightsofapollo]

Minor side tangent: What would you think of attaching the raw response body text / headers / whatever else to the client error so it can be inspected? It would be very useful to see what the API responded with when there's a deserialization error like this. Could help determine if there's a problem with the API itself or how I'm calling into it. I was able to get the raw response body text by hacking the generated code a little bit, but I'd rather not do that.

This is good idea

SO I imagine the expected behavior around multiple responses would be an enum?

[lightsofapollo]

Thought about this more. I floated an enum of all responses earlier but I want to walk that back — collapsing success and errors into one variant set forces every caller to match on every variant even when they just want the happy path, and we lose Result ergonomics.

The shape I'm leaning toward instead (similar to what progenitor / openapi-generator-rust settled on):

pub fn list_todos(...) -> Result<Vec<Todo>, ApiError<ListTodosError>>

pub enum ListTodosError {
    Status400(BadRequestBody),
    Status404(NotFoundBody),
    // one variant per declared non-2xx schema
}

pub struct ApiError<E> {
    pub status: u16,
    pub headers: HeaderMap,
    pub body: String,            // always populated
    pub typed: Option<E>,        // Some if a declared schema matched
    pub parse_error: Option<String>,
}

Rules:

1. 2xx: if there's one 2xx schema (or several that are structurally identical), return `T` directly. Only generate a success enum if there are genuinely different 2xx shapes (rare — e.g. 200 vs 202 with different bodies). No gratuitous one-variant enums.
2. Declared non-2xx: per-operation error enum, body typed per status code.
3. Undeclared status codes: still surface via `ApiError` with `typed = None` and the raw body — never drop the response.

This also folds in your side tangent cleanly: `ApiError` always carries `status + headers + body` whether deserialization succeeded or not, so you never have to hack the generated code to see what the server actually sent. Today `HttpError::Http` carries `body: Option` but it isn't populated on deserialization failures and there are no headers — that's the gap.

For reference, the immediate root cause of the symptom you hit: in `analysis.rs` the inline-response type-naming function ignores the status code, so 200 and 400 both get registered as `ListTodosResponse` and the second one overwrites the first. The schema collision is why the 200 path tries to deserialize as the 400 shape. That'll get fixed as part of this work.

I'll prototype it on a branch and link a draft PR.

[atom-mfeldkamp]

Yeah this direction looks a lot more reasonable and straightforward to me than big enum. I like it quite a lot.

The one thing I think could get possibly missed is distinguishing issues with getting error codes from the API and reporting those with the body, versus getting an expected code with a payload shaped in an unexpected way. That is, I think there's maybe a fourth case: a declared status code that doesn't get deserialized correctly - I'm not entirely positive if ApiError semantically covers that?

Either way, big improvement. Thanks!

[atom-mfeldkamp]

Good news: getting good responses from our pathological internal API now! Thanks again for all the support!

[lightsofapollo]

Nice! Let me know if anything else comes up :D