Option to raise exception on error codes
Discussed in https://github.com/openapi-generators/openapi-python-client/discussions/254
Originally posted by theFong November 28, 2020
Is your feature request related to a problem? Please describe.
In some cases, if an unexpected error code is returned (or even an expected error code), I would rather raise an exception than return None. This is because depending on the linter and adoption of type annotation in a code base I may not handle a None variable. In this case an exception raising status code, message or other meta data would be more useful to debug especially when using exception logging software like Sentry.
Describe the solution you'd like I see a couple of interface possibilities if this feature would make sense to implement.
Add new functions with a must prefix:
-
must_sync -
must_sync_detailed -
must_async -
must_async_detailedThis is inspired by a Golang's pattern of prefixing "must" to functions which may panic. The main downside is the addition of 4 new methods.
Another solution would be to provide a raise_for_status method similar to python requests library on the Response object. This interface would allow the passing of args to specify raising in case of any error code or just unexpected ones. The side effect here is that like remembering to handle None you would have to remember to raise_for_status. This side effect exists when using the requests library so is not a big deal.
Describe alternatives you've considered
Currently in cases where I would like to fail hard, I have a wrapper function which takes a ..._detailed function and builds an exception with meta data if parsed is None. This works fine.
Interested in thoughts/discussion!
Something related to this that I think would be really nice is a way for the user of the generated code to more easily inspect the HTTP status code and body on error. Currently, when the API returns, if it is None, the user knows it fails but has no context as to why it failed, and thus can't display this to the end user or do much to act open it.
Instead of returning Optional[T], perhaps we could devise a return type like:
@dataclass
class TResponse:
data: Optional[T]
status_code: int
raw_payload: Dict[str, Any]
The data member would be exactly what is returned today. status_code would allow the consumer of the API to check for different status codes (403 vs 500, etc) and the raw_payload would allow for, on error, displaying the payload returned by the server, e.g. "field": "field1 is a required field.". I'm most fuzzy on the raw_payload part. Perhaps just passing back up the full httpx.Response would be better? Not sure if that breaks some encapsulation though.
I came across this while working on writing a small consumer for my API. The requests were failing, but I couldn't tell why without breaking out Wireshark and looking at the raw server HTTP response messages.
That's what the *_detailed functions are for, they include additional info (like status code) and should be used when there's something you can do about the failure case (other than just "I didn't get the data").
I avoided returning the actual httpx response class just because I wasn't sure for a while whether I wanted to couple to it, but I don't see us switching to anything else so if adding that to _detailed functions would be useful we can do so.
That's what the *_detailed functions are for,
🤦 Totally missed those. Thanks for the heads up, you already have exactly what I was looking for.
They're mentioned in the generated READMEs but it's not super obvious. If there's a better way we could call those out so they're more discoverable, please let me know or make a PR with an improvement. Clear documentation is supposed to be a feature of this project 😅
Another alternative is creating the client with some extra configuration, in this case a raise_on_error_code flag
client = Client(
base_url="https://api.example.com",
raise_on_error_code=True,
)
This would be kind of similar to what OpenApi client generator does for other languages, for example Typescript:
export interface ConfigurationParameters {
basePath?: string; // override base path
fetchApi?: FetchAPI; // override for fetch implementation
middleware?: Middleware[]; // middleware to apply before/after fetch requests
queryParamsStringify?: (params: HTTPQuery) => string; // stringify function for query strings
username?: string; // parameter for basic security
password?: string; // parameter for basic security
apiKey?: string | ((name: string) => string); // parameter for apiKey security
accessToken?: string | ((name?: string, scopes?: string[]) => string); // parameter for oauth2 security
headers?: HTTPHeaders; //header params we want to use on every request
credentials?: RequestCredentials; //value for the credentials param we want to use on each request
}
This will allow also the creation of middlewares which can be useful too.
I know this solution is not ideal either as having a clean client object is pretty nice, but if you are ok with it I can contribute with a PR adding this feature.
After #775 raising exceptiosn for error codes can be done via event hooks—so I'm considering that as closing this.
@dbanty One downside of that approach is that error responses often don't have response types in an openapi schema, so any endpoint that enumerates its error codes ends up with a return type of Union[ActualResponseType, Any].
Closing this, will track built-in exceptions in #254