Skip to content

Conversation

akx
Copy link

@akx akx commented Oct 10, 2025

Summary

try: except: blocks are free on modern Pythons when exceptions aren't raised.

Entering a @contextmanager'd with block is much less free (see e.g. this Django discussion about contextlib.suppress), and there are hot paths (e.g. reading from a sync socket) where it's worth avoiding that overhead.

For consistency, this retires the use of map_exceptions everywhere.

Checklist

  • I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
    • No test changes needed.
  • I've updated the documentation accordingly.
    • No changes needed; internal change.

Performance impact

Using pytest-benchmark, and Caddy locally serving a 10-megabyte file,

from httpcore import ConnectionPool


def test_bench(benchmark):
    with ConnectionPool() as pool:
        def b():
            resp = pool.request(
                method="GET",
                url="http://127.0.0.1:8080/bigfile2",
            )
            assert len(resp.content) == 10 * 1024 * 1024
        benchmark(b)

is shown to be a bit faster:

Name (time in ms)               Mean                 OPS
------------------------------------------------------------------
test_bench (0002_2655597)     3.8573 (1.0)      259.2509 (1.0)
test_bench (0001_5974b03)     4.2206 (1.09)     236.9331 (0.91)
------------------------------------------------------------------

`try: except:` blocks are free on modern Pythons when exceptions
aren't raised. Entering a `@contextmanager`'d block is much less free,
and there are hot paths (e.g. reading from a sync socket) where it's
worth avoiding that overhead.

For consistency, this retires the use of `map_exceptions` everywhere.
@akx
Copy link
Author

akx commented Oct 10, 2025

The new coverage misses are a bit 🤷, because evidently those cases weren't tested previously either, they were just conveniently covered by the exception mapping 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant