asyncio connection_pool: catch CanceledError exception fix connection…

[aviramha]

… leak in case of cacncellations
closes#149
closeshttps://github.com/encode/httpx/issues/1461

[florimondmanca]

Thanks @aviramha

It seems we'd need to fit this into our async backends layer, so that it also works with eg trio (though I'm not sure if we can reproduce the issue there?).

Essentially I think we want the usage to be except self.backend.cancelled_exc_class instead, with that exception being None in the sync case (?).

[aviramha]

I'm not sure what's the functional/design benefit from such behavior - this is the async implementation of connection pool, and catching "CanceledError" in trio won't really do anything anyway so why does it matter?

[lovelydinosaur]

So, Trio's Cancelled exception also inherits from BaseException, yup... https://github.com/python-trio/trio/blob/4e8e19cff8fd5c1b5875d36c77564a6ec021330a/trio/_core/_exceptions.py#L31

I don't think it'd be a terrible idea for us to just catch BaseException here. We are always re-raising the exception.

Either...

exceptNewConnectionRequired:
connection=NoneexceptBaseException: # noqa: PIE786# Note that we catch BaseException here in order to also support# removing connections from the pool on task cancellations.logger.trace("remove from pool connection=%r", connection)
awaitself._remove_from_pool(connection)
raise

Or perhaps...

exceptNewConnectionRequired:
connection=NoneexceptBaseException: # noqa: PIE786try:
logger.trace("remove from pool connection=%r", connection)
awaitself._remove_from_pool(connection)
finally:
raise

[aviramha]

@tomchristie I tend to agree. I updated the PR.

[lovelydinosaur]

Thanks @aviramha!

[aviramha]

Thank you for this well maintained library!