Avoid handle BaseException just for cover async task cancellation.

[T-256]

Actually, I noticed by https://github.com/encode/httpcore/pull/802/files#r1326095576.

In #802, he tries to handle all of BaseExecption errors that we don't handle currently (e.g. KeyboardInterrupt).
What's need here for error handling is just async-backend specified cancellation error class.

Actually, not only here, also where #726 implemented:

• First, Before: no error handling, After: BaseException handling.
• Second, Before: Exception handling, After: BaseException handling.

[tomchristie]

Can you describe and demonstrate a behaviour that you want to see changed?

[T-256]

Also as said in https://stackoverflow.com/a/44655928, except BaseException: has same functionality as bare except:. linters avoid you to use bare except: (1, 2), so perhaps you now understand what actually I'm pointing to.

[karpetrosyan]

GeneratorExit, KeyboardInterrupt, SystemExit are builtin exceptions that directly inherited from BaseException, can you point why catching such exceptions is good in httpcore?

You can even use bare except: when writing your cleanup logic. It is always preferable to clean up even if a keyboardinterrupt has been raised, because the user may catch the keyboardinterrupt in his code and continue to use httpcore.ConnectionPool.

[T-256]

It is always preferable to clean up even if a keyboardinterrupt has been raised,

It's reasonable, but for that purpose, IMO you could use direct class name.

You can even use bare except: when writing your cleanup logic.

but for cleanup purposes you should be able to do cleanup from most top call layer, for an example please check:
https://github.com/T-256/httpcore/tree/cleanup-redesign

[tomchristie]

Okay, so I'm not against a proposal to switch cases of...

except BaseException as exc:

...to...

except (Exception, anyio.get_cancelled_exc_class()) as exc:

...what I would like to see is a proposal that demonstrates that change in a minimal way.

The cleanup-redesign branch you've linked above has a commit across 7 files with 272 additions and 236 deletions. That's not proportional to the change you're suggesting.

You're suggesting a potentially valid but not obviously necessary change here. That's okay, but if you could present it in a way that helps make it clear you've thought this through thoroughly, and that doesn't attention-churn the rest of the team, that would be much appreciated. 🙂

[T-256]

The cleanup-redesign branch you've linked above has a commit across 7 files with 272 additions and 236 deletions. That's not proportional to the change you're suggesting.

Correct, actually that branch is suggesting new cleanup structure, it exists to answer @karosis88 that we can omit except: for cleanup and use _cleanup method from parent caller.

if you could present it in a way that helps make it clear you've thought this through thoroughly, and that doesn't attention-churn the rest of the team, that would be much appreciated. 🙂

Sorry about that, I was trying to describe what I thought before open direct PR. it seems I didn't do well.

...what I would like to see is a proposal that demonstrates that change in a minimal way.

Yes, I'll try to do it.

[tomchristie]

I think the most graceful way to do what you're attempting here would be adding the following in _syncronization.py...

EXCEPTION_OR_CANCELLED = (Exception,)

try:
    import trio  # trio support is available to httpcore
    EXCEPTION_OR_CANCELLED += (trio.CancelledError,)
except ImportError:
    pass

try:
    import anyio  # asyncio support is available to httpcore
    import asyncio
    EXCEPTION_OR_CANCELLED += (asyncio.Cancelled,)
except ImportError:
    pass

And then import and use EXCEPTION_OR_CANCELLED in the existing BaseException cases...

try:
    ...
except EXCEPTION_OR_CANCELLED as exc:
    ...

The would seem to me to be the simplest approach that would work fine for both the _sync and _async codebases.

[T-256]

Thanks