aio-libs / aiohttp

Asynchronous HTTP client/server framework for asyncio and Python
https://docs.aiohttp.org
Other
14.81k stars 1.98k forks source link

Feature: make `HTTPException` non-abstract #4034

Open atemate opened 4 years ago

atemate commented 4 years ago

Long story short

Use-case: a server's handler uses a client to make a request to another server. I would like to re-use the response status code in the first server's response (the code below does not work as HTTPException's constructor does not accept parameter status_code):

    try:
        ...
    except ClientResponseError as e:
        status = e.status or 500
        raise web.HTTPException(text=e.message, status_code=status)

Of course I can use return web.Response(text=e.message, status=status), but web.Response cannot be throwed, however I need it in my applicatoin.

Expected behaviour

I would expect web.HTTPException not to be abstract so that one can construct and raise it with a dynamic status code.

Actual behaviour

one cannot create an instance of web.HTTPException

Steps to reproduce

(not a bug so no steps to reproduce)

Your environment

(not a bug so no environment description)

asvetlov commented 4 years ago

Thanks for the interesting question. Just raising HTTPException for status code 400 is not the best idea, the expected exception type, in this case, is HTTPBadRequest.

Python's OSError has a similar problem. OSError(errno.ENOENT, 'file zzz.txt not found') actually creates FileNotFound(errno.ENOENT, 'file zzz.txt not found'). It works but confusing, there are edge cases when exception type downcasting is not performed.

I prefer a factory function, e.g. make_exception(status_code, *args, **kwargs) that finds a proper exception type based on passed status_code and instantiate the exception with args / kwargs.

Feel free to make a pull request. I have a feeling that the feature priority is low; don't want to work on it personally but happy to help with the review.

webknjaz commented 4 years ago

I prefer a factory function

I'd even say a @classmethod...

asvetlov commented 4 years ago

Earlier I used to apply class methods often enough but now I prefer just a function usually.

It gives better decomposability but provides the same functionality. The taste question though. In aiohttp we have several factories and 3 public class methods, all of them are creators of self class and never are used for polymorphic type downcasting.

serhiy-storchaka commented 4 years ago

Some HTTPException subclasses require additional arguments. For example HTTPRequestEntityTooLarge requires max_size and actual_size. Passing them to constructors of other exceptions is an error. How to use the factory function in such case?

asvetlov commented 4 years ago

I thought that the factory accepts *args and **kwargs and passes them into the selected class constructor. Not ideal, we miss type checking here...

As an alternative, we can do nothing. The issue is useful only for writing some kind of proxy that passes HTTP codes from internal HTTP calls to its own caller as is. Otherwise, the proxy already has some transformation code, it can build the factory on its own.

aiohttp is used for building microservices/proxies/aggregators often enough, maybe the idea has own fellows. I have no strong preference here (and don't want to invest my own time in the implementation but open for review).