Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert ClientResponseError in HTTPException #3449

Closed
kornicameister opened this issue Dec 12, 2018 · 11 comments
Closed

Convert ClientResponseError in HTTPException #3449

kornicameister opened this issue Dec 12, 2018 · 11 comments

Comments

@kornicameister
Copy link
Contributor

kornicameister commented Dec 12, 2018

Long story short

We have the case, in our project (rest API), where we make some requests to the upstream services using the session. So long story short, we have sth like async with session.get(...). That may generates ClientResponseError. And that is fine, because at this point we act as a client. But we need a way to respond with HTTPException to the original caller of the API.

I just can't find a neat way other than getting the status from ClientResponseError and choosing one of the subclasses of HTTPException (that do set the appropriate code) other than if-ing by the status one by one maybe discarding ones that we do not care about as HTTPInternalServerError.

Expected behaviour

I am able to easily transform ClientResponseError into HTTPException that will share the HTTP code and carried body.

Actual behaviour

I cannot do the above.

None as this is most likely not a bug. It might be a feature or I have failed to locate existing solution.

Your environment

python --version ; pip freeze | grep aiohttp
Python 3.7.1
aiohttp==3.5.0a1
aiohttp-debugtoolbar==0.5.0
aiohttp-jinja2==1.1.0
@kornicameister
Copy link
Contributor Author

kornicameister commented Dec 12, 2018

cc @marrataj ( he was the guy from my project originally spotting that little inconvenience )

@aio-libs-bot
Copy link

GitMate.io thinks the contributor most likely able to help you is @asvetlov.

Possibly related issues are #662 (ClientResponseError wrong error propagation?), #2781 (Replace ClientResponseError.code with .status), #1754 (ClientResponseError after upgrading to latest version), #850 (How to handle ClientResponseError/ServerDisconnectedError properly), and #2980 (Drop deprecated code parameter from ClientResponseError).

@kornicameister
Copy link
Contributor Author

FYI: I don't think we should expect any improvement in the existing code as I am not really convinced this falls into automatic behavior. Perhaps it does not and those are the things that are best to be handled case-by-base.

Anyway, any recommendation is welcomed.

@asvetlov
Copy link
Member

You can collect a mapping of status_code -> web_exception_class easy.
The tricky thing is arguments list used for web exception construction.
Also, some processing is usually needed (conversion of json body payload with exception info is a good example).

Another question is: should you pass all exceptions as-is or wrap them somehow. Maybe you want to distinguish gateway timeout returned by client from timeouts to proxy server itself.

Taking all the above into account I doubt if automation transition is possible in a generic matter as a part of aiohttp.
But writing a translator for your particular case should be not very big challenge.

@stj
Copy link
Contributor

stj commented Dec 12, 2018

We do something like this with a custom response class

import aiohttp
from aiohttp import http_exceptions

class CustomResponse(aiohttp.ClientResponse):
    def raise_for_status(self) -> None:
        if 400 <= self.status:
            raise http_exceptions.HttpProcessingError(
                code=self.status,
                message=self.reason,
                headers=self.headers,
            )

session = aiohttp.ClientSession(..., raise_for_status=True, response_class=CustomResponse)

@kornicameister
Copy link
Contributor Author

kornicameister commented Dec 12, 2018

@stj so the trick is to always raise_for_status instead of parsing code after exiting request coroutine?

Ah my nad, there is a method with the same name. Slipped my mind.

I like this idea, seems simple and should play nicely in global context as well as in local use case

@asvetlov
Copy link
Member

Caution: custom response class is a sort of gray hidden feature of aiohttp.
I have a plan to remove it eventually

@kornicameister
Copy link
Contributor Author

kornicameister commented Dec 12, 2018

Ah...that does not sound right, I mean to implement and face the potential consequences later. So @asvetlov your recommendation is to basically do this case by case. Whatever status_code from ClientResponseError we want to convert - we do, any other we treat as, for example, HTTPInternalServerError ?

@asvetlov
Copy link
Member

Yes, I think it is a practical solution.

@kornicameister
Copy link
Contributor Author

Nice, I will close it on my own since the question has been answered.
Thx @asvetlov and @stj

@lock
Copy link

lock bot commented Dec 13, 2019

This thread has been automatically locked since there has not been
any recent activity after it was closed. Please open a new issue for
related bugs.

If you feel like there's important points made in this discussion,
please include those exceprts into that new issue.

@lock lock bot added the outdated label Dec 13, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants