Is it really necessary to handle cancellations?

[karpetrosyan]

#726 introduced the async cancellations support in httpcore, but do we really support them?

There are several issues that can approve that our connection pool cannot handle all the cancellation cases.
See: #830, #785, #757

I think we hurried up saying that we support async cancellations.

Async cancellation support at least means that we have looked at all the awaits, and we are sure that whenever our program exits during await something(), we are not breaking the connection pool state.

Possibly, we can drop the AsyncShieldCancellation class at all and instead shield the whole connection pool operation to be sure that the connection pool will never be broken.

So I suggest documenting this behavior so that we do not handle cancellations, and if you want, you can run the handle_async_request in a cancellation-isolated environment.

This solution has several benefits, including:

• Our connection pool becomes more stable. We always know that it doesn't matter when the program is cancelled, so we can assume that we always have a connection pool in the correct state.
• We avoid reviewing tons of await statements that can cause the connection pool to be in a broken state. You can look at Handle asynchronous cancellations for HTTPConnection instances #802 to figure out how hard it is to detect such issues.
• We can drop the AsyncShieldCancellation class, which never made sense to me, especially when using it in synchronous code.

And finally, after making this change, we should shield the pool.handle_async_request in httpx and also be sure that we always run our code with timeouts, so the application cannot hang with unlimited time even if it is cancelled.

[zanieb]

I think cancellation support is a very important use-case for an asynchronous HTTP client. I think we must support this and adding arbitrary timeouts will not result in a good user experience.

[konstantin-baidin-y42]

Hello, I'm experiencing pool poisoning in my application because my tasks can be canceled due to a timeout. So after some number of such cancelations, the pool is full of unusable connections. Here is how to reproduce it:

I experimented with versions 0.16.3, 0.17.1 and 0.18.0.

Having some long-responding server on localhost:5522 the following code fails with PoolTImeout. You can get and run such a long-responding server from this comment: encode/httpx#1171 (comment)

I expect that the following code will not raise PoolTimeout, especially when I canceled all the tasks and waited for an additional 5 seconds for cancelation to complete.

import asyncio
import httpcore


async def main() -> None:
    async with httpcore.AsyncConnectionPool(max_connections=3) as pool:
        async def do_one_request():
            return await pool.request("GET", "http://localhost:5522/", extensions={"timeout": {"pool": 1}})

        # First, create many requests, then cancel while they are in progress.
        tasks = []
        for i in range(5):
            tasks.append(asyncio.create_task(do_one_request()))
            await asyncio.sleep(0.0001)
            tasks[-1].cancel()

        print("Wait reasonable amount of time")
        await asyncio.sleep(5)
        print("Starting another request will now fail with a `PoolTimeout`")
        await do_one_request()


asyncio.run(main())

Does this case belongs to this discussion, or should I create an issue for it?