Introduce raise_on_error_status flag

[robertschweizer]

Closes #491

This behaves similarly to raise_on_unexpected_status introduced in #593.

Partially implements and inspired by the "Ability to raise exception for failed responses" from #722, but I decided to go for a much simpler implementation without an x-response-type property. I'm not sure it is useful to go against the standard that error codes are 4xx and 5xx.

This will conflict with #729. To combine both, we need to introduce a common base class for the different exception types, e.g. ApiException, that handles the extra arguments.

[robertschweizer]

I realized that setting this option in the Client will never allow us to get the request functions' return types right.

What do you think about adding both the existing raise_on_unexpected_status and the new raise_on_error_status to the generation config file instead? Then we can get rid of None in the return type annotations if both flags are set. We can additionally allow setting raise_on_unexpected_status in the Client for backwards-compatibility.

[aletor123]

We also need this feature(in generator config) in our team, it would be great if you add this

[aletor123]

We can also think about adding this as boolean parameter to _parse_response( and it's users) and overload for these functions that would return expected type if this parameter is true.

Also we can add raise_on_error status as generic for Client and add overloads for this

[robertschweizer]

@dbanty which one do you like better: A runtime flag with overloads to get type annotations working or a flag in the generation config file?

I think for most APIs it's OK if the person generating the client makes the decision on how error response codes are handled. This does not need to be done differently by every user of the generated client. It would save us a lot of typing complexity in the generated code.

[dbanty]

The fact that exceptions bypass the type system is the entire reason I resisted adding any exceptions for so long 😅. For the folks who want exceptions instead of statically-typed errors (and I know a lot of you exist), do you care about static types at all? I feel like mixing and matching the error handling is just going to be annoying, and that a generated client should either throw exceptions for all errors or return types which represent those errors. That means the documented error types from OpenAPI should also become generated exceptions rather than attrs classes.

As a side note, rather than maintaining this code ourselves, we should expose the httpx client and let folks set up event hooks (there's an example that raises exceptions on 4xx/5xx here). I know there are a lot of people using custom templates to set up hooks for other things already, so this would also enable a bunch of other features that folks are requesting without adding a bunch more code for us to maintain. That's not to say that this PR can't land, just that it will cause future breaking changes/tech debt that needs to be considered.

[robertschweizer]

Thanks for the response!

I definitely prefer getting errors as exceptions. I want to use the sync function's response value without first checking its type: Ideally it would have only one possible return type.

I don't personally care so much about this, but I think the exceptions can be typed: The generated client could provide a different exception class for every combination of status code and response schema and then raise the correct exception types. I would make the decoded response body a member of the custom exception types though, to be able to inherit from httpx.HTTPStatusError:

class CustomResponseType422(httpx.HTTPStatusError):
    def __init__(self, *args, decoded: CustomResponseType, **kwargs):
        super().__init__(*args, **kwargs)
        self.decoded = decoded
...
try:
    my_request.sync(client)
except CustomResponseType422 as err:
    handle_422(err.decoded)

Using what httpx offers is a good idea. However, if we expose the client to allow adding a raise_on_4xx_5xx hook, sync methods would still have unions as return type annotations. I would like to introduce the raise_on_unexpected_status and raise_on_unexpected_status as config flags at generation time to get rid of the union return types. I will open a new PR for that.