assertion failed: response_types.len() <= 1

[clarkmcc]

Here's the reproducible project:

https://github.com/clarkmcc/esi-rs

When building I get

thread 'main' panicked at /Users/.../.cargo/git/checkouts/progenitor-639bd64206ac5e43/2f32cde/progenitor-impl/src/method.rs:1264:9:
  assertion failed: response_types.len() <= 1

[clarkmcc]

Bumping, any ideas on this?

[scottbot95]

I hit this too. From what I can tell this issue is that progenitor can actually only deserializing to exactly one type. This means if you model the different response codes and they return differently shaped data, then progenitor is unable to generate a client for that model. This is pretty lame in cases where you actually want to read some data from error conditions (eg a time to wait before retrying on a 429 response). In my case it was acceptable to just not model the error conditions and sum them all up as "something went wrong", so I just removed all non-200 responses from the model I'm using.

[clarkmcc]

I figured that was the cause, but I'm not sure how that is even remotely acceptable behavior. First of all, you may not always control the model you want to do codegen for. Second, every well-designed API is going to support multiple response types, so generating APIs for some of the most popular or biggest APIs is impossible with this library. And third, why can't the codegen use an enum to represent the distinct error responses?

[ahl]

This is pretty lame in cases where you actually want to read some data from error conditions

Agreed! And your analysis of the situation is spot on. Here's the comment on the offending assert:

        // TODO to deal with multiple response types, we'll need to create an
        // enum type with variants for each of the response types.

So in this case, I think we'd like to generate something like this:

enum GetAlliancesErrorResponses {
    /// Not modified
    Response304(serde_json::Value),
    /// Bad request
    BadRequest(BadRequest),
    /// Error limited
    ErrorLimited(ErrorLimited),
    /// Internal server error
    InternalServerError(InternalServerError),
    /// Service unavailable
    ServiceUnavailable(ServiceUnavailable),
    /// Gateway timeout
    GatewayTimeout(GatewayTimeout),
}

Note that the first case -- 304 -- we'd probably need to invent a variant name since I'm not sure we have something better.

[ahl]

I'm not sure how that is even remotely acceptable behavior

Call it prioritization. The APIs for which we use progenitor haven't required multiple error (or success) response types though certainly I know others might.

you may not always control the model you want to do codegen for

Tell me about it! OpenAPI (and JSON Schema) permits all kinds of surprising constructions!

Second, every well-designed API is going to support multiple response types

That seems more debatable. But certainly, there are many opinions about what constitutes a well-designed API.

generating APIs for some of the most popular or biggest APIs is impossible with this library

True! Certainly we'd like to improve that, but--as I said--it's a matter of priority and support e.g. for the most popular or biggest APIs hasn't been an area where we're investing.

And third, why can't the codegen use an enum to represent the distinct error responses?

It could!

[scottbot95]

Second, every well-designed API is going to support multiple response types

That seems more debatable. But certainly, there are many opinions about what constitutes a well-designed API.

Sure there can be debate on what a well-designed API looks like, but something I hope we all would agree on is that a well-made OpenAPI specification should model the possible error conditions. However progenitor is incapable of handling even the most simple error condition (404 with no body).

I'm not sure how that is even remotely acceptable behavior

Call it prioritization. The APIs for which we use progenitor haven't required multiple error (or success) response types though certainly I know others might.

Prioritization is always a tough problem; you have limited resources and a near limitless field of possible features to implement. That said, not being able to gracefully handle any error conditions seems like a VERY high ROI opportunity feature, especially since the path forward is already well-know ("just" make it use an enum). I'm sure this is a somewhat naive take, but it seems like the only actual "hard" part about implementing this feature is coming up with names for the enum variants/corresponding structs (although probably those names are either the same or related)

[clarkmcc]

Makes sense on the prioritization issue.

Also agree with @scottbot95 that good APIs model their errors. I suppose that some APIs may choose to model their errors outside of the OpenAPI framework with a single response type that contains the error details, code, etc (the Google API error type I believe is like this). So @ahl maybe your point is that good APIs will model their errors but not necessarily within the OpenAPI framework. If so, that's fair. It does significantly restrict that number of public APIs where I can use this unfortunately.