From 35e77385f090adbbee4253a9886a26ee425e992c Mon Sep 17 00:00:00 2001 From: Alexey Shalaev <75322386+AlexeyShalaev@users.noreply.github.com> Date: Fri, 16 Oct 2026 18:46:11 +0400 Subject: [PATCH] fix: count connect timeouts, not statement timeouts, in connection_timeouts_total --- docs/agents.md | 15 ++- docs/guide/advanced.md | 11 +- .../contrib/metrics/postgres.py | 17 ++- .../protocols/metrics.py | 20 ++- sqlalchemy_foundation_kit/session/manager.py | 41 +++++-- tests/integration/test_metrics_integration.py | 108 ++++++++++++++++ tests/unit/session/test_manager.py | 115 +++++++++++++----- 7 files changed, 271 insertions(+), 56 deletions(-) diff --git a/docs/agents.md b/docs/agents.md index caa7d3b..88101a3 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -213,8 +213,12 @@ engine. `instrument_pool_class(pool_class, metrics)` lives beside them and is the other half of the metrics wiring: it returns a subclass of `pool_class` that times `Pool.connect()`, which is -the only place the wait for a connection and the pool checkout timeout are visible — no -pool event fires for either. The manager applies it to whatever `resolve_pool_class` +the only place the wait for a connection, the pool checkout timeout and a failed connect +are visible — no pool event fires for any of them, and with asyncpg none reaches +`handle_error`. A builtin `TimeoutError` there (asyncpg's connect `timeout`) counts as a +timeout like the pool's own; any other exception is recorded as an error when `metrics` +implements `ErrorRecorder`, unless it is the driver's DBAPI error, which `handle_error` +reports. The manager applies it to whatever `resolve_pool_class` returned whenever `metrics` implements `CheckoutWaitRecorder`, so callers get it without asking. On an engine you build yourself, pass the result as `poolclass=` to `create_async_engine`; `attach_metrics` logs a warning if you did not, rather than leaving @@ -350,12 +354,12 @@ variables only through the parent `BaseSettings` that holds it. Under | `postgres_db_connection_checkout_wait_seconds` | histogram | how long a caller **waited** for a connection — the queue wait, plus the pre-ping and the connect handshake when the pool had to grow. Buckets run to 30 s, the default `pool_timeout` | | `postgres_db_connection_held_duration_seconds` | histogram | how long a caller **held** one, checkout to checkin — the query time seen from the pool | | `postgres_db_connection_checkout_duration_seconds` | histogram | **deprecated**, and never measured what its name says: it is the held duration under its old name. Kept for one more minor release, then dropped. Move dashboards to `…_held_duration_seconds`, or to `…_checkout_wait_seconds` if what you wanted was the wait | -| `postgres_db_connection_timeouts_total` | counter | pool checkout timeouts, plus `TimeoutError`s seen by `handle_error` | -| `postgres_db_connection_errors_total{error_type}` | counter | database errors by exception class name | +| `postgres_db_connection_timeouts_total` | counter | timeouts acquiring a connection: pool checkout timeouts and connect timeouts. A statement timeout (asyncpg `command_timeout`) is not one; it is `connection_errors_total{error_type="TimeoutError"}` | +| `postgres_db_connection_errors_total{error_type}` | counter | database errors by exception class name: everything `handle_error` sees, plus a connection the pool could not open | A `prefix` is prepended with an underscore and must match `^[a-zA-Z_][a-zA-Z0-9_]*$`. -The wait histogram and the pool-timeout half of the counter come from the instrumented pool +The wait histogram, the timeout counter and the connect errors come from the instrumented pool class, not from a listener, so they move only on an engine built by `AsyncSessionManager` or one whose pool went through `instrument_pool_class`. @@ -468,6 +472,7 @@ is what to do on the version the row names. | `create_async_session_manager(config)` through PgBouncer in transaction mode (0.2.1 and earlier) | `jit="off"` was the default and `db_schema` went as `search_path`, both as startup parameters: a default PgBouncer refuses every connection with `ProtocolViolationError: unsupported startup parameter: jit`; with `ignore_startup_parameters=jit,search_path` it connects and silently drops both, so every query lands in `public` | `jit=None`, `db_schema=None`, and `ALTER ROLE … SET search_path` on the server | | `postgres_db_connection_timeouts_total` (0.3.0 and earlier) | stayed at zero through every pool checkout timeout. The counter was fed only from the engine's `handle_error` listener, and a pool `TimeoutError` is raised by `pool.connect()` before any DBAPI call, so it never reaches that listener — the one timeout a pool actually produces under load was the one the counter did not count | count `sqlalchemy.exc.TimeoutError` around your own session calls | | `postgres_db_pool_overflow`, `postgres_db_pool_checked_out` (0.4.0 and earlier) | sampled from the `checkout` / `checkin` events: `overflow` exported `QueuePool.overflow()`, which starts at `-pool_size` (`-9` with one connection open of 10), and since `checkin` fires before the connection is back, an idle pool kept reporting its last burst as checked out | read `pool.checkedout()` and `max(pool.overflow(), 0)` yourself | +| `postgres_db_connection_timeouts_total` (0.4.0) | counted asyncpg statement timeouts — a slow query, on a healthy connection — and missed connect timeouts, which never reach `handle_error`. A database refusing connections moved no series at all | alert on `connection_errors_total{error_type="TimeoutError"}` for slow statements, and on the checkout wait for connections | | `postgres_db_connection_checkout_duration_seconds` (0.3.0 and earlier) | the only checkout histogram there was, and it measures the time between the `checkout` and `checkin` events — how long a connection was *held*, which is query duration seen from the pool. The time a caller waited for a connection, which is what the name suggests and what predicts a pool outage, was not exposed at all | none; the wait was not measurable from outside the pool | `IsolationLevel` itself was always fine — `READ_UNCOMMITTED`, `READ_COMMITTED`, diff --git a/docs/guide/advanced.md b/docs/guide/advanced.md index b653b1d..3c4e738 100644 --- a/docs/guide/advanced.md +++ b/docs/guide/advanced.md @@ -275,7 +275,7 @@ session_manager = create_async_session_manager( | `myapp_postgres_db_connection_held_duration_seconds` | Histogram | Time a caller **held** a connection, checkout to checkin | | `myapp_postgres_db_connection_checkout_duration_seconds` | Histogram | Deprecated alias of the held duration — see below | | `myapp_postgres_db_connection_errors_total` | Counter | Database errors by type (`error_type` label) | -| `myapp_postgres_db_connection_timeouts_total` | Counter | Pool checkout timeouts, and `TimeoutError`s during execution | +| `myapp_postgres_db_connection_timeouts_total` | Counter | Timeouts acquiring a connection: pool checkout timeouts and connect timeouts. A statement timeout is an error, not one of these | Without a `prefix` the names are `postgres_db_pool_size` and so on. @@ -300,9 +300,12 @@ keep working, and it will be removed in a future release. Point dashboards at `connection_held_duration_seconds` for the same numbers, or at `connection_checkout_wait_seconds` if what you actually wanted was the wait. -The wait histogram and the pool-timeout half of the counter are not event listeners — -SQLAlchemy fires no event when a checkout is *requested* — so they come from the pool class -the session manager builds. They are populated on any engine created by +The wait histogram and the timeout counter are not event listeners — +SQLAlchemy fires no event when a checkout is *requested*, and neither a pool timeout nor a +failed connect reaches `handle_error` — so they come from the pool class the session +manager builds. The same pool class counts a connection it could not open (refused, bad +password) in `connection_errors_total`. A statement that runs into asyncpg's +`command_timeout` is counted as `connection_errors_total{error_type="TimeoutError"}`. They are populated on any engine created by `AsyncSessionManager`, `AsyncSessionManagerBuilder` or `create_async_session_manager`. On an engine you build yourself, wrap the pool class: diff --git a/sqlalchemy_foundation_kit/contrib/metrics/postgres.py b/sqlalchemy_foundation_kit/contrib/metrics/postgres.py index 70c2f5b..3da5c2c 100644 --- a/sqlalchemy_foundation_kit/contrib/metrics/postgres.py +++ b/sqlalchemy_foundation_kit/contrib/metrics/postgres.py @@ -67,7 +67,8 @@ class PostgresMetrics: - postgres_db_connection_held_duration_seconds: Time a connection was held by its caller. - postgres_db_connection_checkout_duration_seconds: Deprecated alias of the held duration, published unchanged for one more minor release. - - postgres_db_connection_timeouts_total: Number of connection checkout timeouts. + - postgres_db_connection_timeouts_total: Number of times acquiring a connection timed out: + pool checkout timeouts and connect timeouts. Statement timeouts are not counted here. - postgres_db_connection_errors_total: Number of connection errors. Labels: @@ -122,7 +123,7 @@ def __init__(self, prefix: str | None = None) -> None: ) self.connection_timeouts_total = Counter( _make_metric_name("postgres_db_connection_timeouts_total", prefix), - "Number of connection checkout timeouts", + "Number of times acquiring a connection timed out: pool checkout timeouts and connect timeouts", ) self.connection_errors_total = Counter( _make_metric_name("postgres_db_connection_errors_total", prefix), @@ -170,10 +171,10 @@ def record_checkout_wait( duration: float, timed_out: bool = False, ) -> None: - """Record the wait for a connection from the pool, and count a pool timeout. + """Record the wait for a connection from the pool, and count a timeout acquiring it. - A pool checkout timeout is raised before any DBAPI call, so it never reaches the - engine's ``handle_error`` listener; this is the only place it is counted. + A pool checkout timeout and a connect timeout never reach the engine's + ``handle_error`` listener; this is the only place either is counted. """ self.connection_checkout_wait.observe(duration) if timed_out: @@ -184,7 +185,11 @@ def record_error( error_type: str, is_timeout: bool = False, ) -> None: - """Record a database connection or execution error.""" + """Record a database connection or execution error. + + ``is_timeout`` is kept for compatibility and still counts a timeout when a custom + caller sets it; the library itself no longer does. + """ self.connection_errors_total.labels(error_type=error_type).inc() if is_timeout: self.connection_timeouts_total.inc() diff --git a/sqlalchemy_foundation_kit/protocols/metrics.py b/sqlalchemy_foundation_kit/protocols/metrics.py index 166e4e7..8ea4182 100644 --- a/sqlalchemy_foundation_kit/protocols/metrics.py +++ b/sqlalchemy_foundation_kit/protocols/metrics.py @@ -102,22 +102,32 @@ def record_checkout_wait(self, duration: float, timed_out: bool = False) -> None Args: duration: Seconds spent inside ``pool.connect()`` — the queue wait, and the pre-ping round trip and connect handshake when the pool had to grow. - timed_out: True if the wait ended in ``sqlalchemy.exc.TimeoutError`` rather - than in a connection. This is the pool checkout timeout; it is raised - before any DBAPI call and so never reaches the engine's ``handle_error``. + timed_out: True if the wait ended in a timeout rather than in a connection: + the pool checkout timeout (``sqlalchemy.exc.TimeoutError``) or the + driver's connect timeout (a builtin ``TimeoutError`` while the pool opens + a connection). Neither reaches the engine's ``handle_error``. """ ... +@runtime_checkable class ErrorRecorder(Protocol): - """Capability protocol for recording database errors.""" + """Capability protocol for recording database errors. + + Called for every error the engine's ``handle_error`` sees -- errors on an established + connection, a statement timeout among them -- and, from the instrumented pool, for a + connection the pool could not open. ``runtime_checkable`` so the instrumented pool can + tell whether its recorder takes errors too. + """ def record_error(self, error_type: str, is_timeout: bool = False) -> None: """Record a database connection or execution error. Args: error_type: The type of error that occurred (e.g., "OperationalError"). - is_timeout: True if this error was specifically a connection checkout timeout. + is_timeout: Kept for compatibility; the library no longer sets it. Timeouts + acquiring a connection are reported through + :meth:`CheckoutWaitRecorder.record_checkout_wait`. """ ... diff --git a/sqlalchemy_foundation_kit/session/manager.py b/sqlalchemy_foundation_kit/session/manager.py index 3afafbf..a865f73 100644 --- a/sqlalchemy_foundation_kit/session/manager.py +++ b/sqlalchemy_foundation_kit/session/manager.py @@ -21,7 +21,7 @@ from .._typing import SessionT from ..base import build_engine_kwargs, resolve_pool_class -from ..protocols import CheckoutWaitRecorder, PoolStatsFunctionRecorder +from ..protocols import CheckoutWaitRecorder, ErrorRecorder, PoolStatsFunctionRecorder if TYPE_CHECKING: from sqlalchemy.engine import Connection @@ -75,7 +75,7 @@ def _pool_overflow(pool: Pool) -> int: def instrument_pool_class(pool_class: type, metrics: CheckoutWaitRecorder) -> type: - """Build a pool class that times the wait for a connection and counts pool timeouts. + """Build a pool class that times the wait for a connection and counts timeouts acquiring one. SQLAlchemy has no event for "a checkout was requested", only for one that succeeded, so the time a caller spends queued for a connection cannot be observed from a listener. @@ -86,6 +86,15 @@ def instrument_pool_class(pool_class: type, metrics: CheckoutWaitRecorder) -> ty ``await_only`` inside that call, so the timer spans the whole suspension and measures the time the caller really waited. + Every checkout records its wait, however it ends. Two endings count as a timeout: the + pool's own ``sqlalchemy.exc.TimeoutError`` (the queue outlasted ``pool_timeout``) and + a builtin ``TimeoutError`` from the driver while the pool opens a new connection + (asyncpg's connect ``timeout``). Any other exception is a connection the pool could + not open, and is also recorded as an error when ``metrics`` implements + ``ErrorRecorder`` -- unless it is the driver's DBAPI error, which the engine's + ``handle_error`` reports itself. asyncpg's connect errors are not: none of them reaches + ``handle_error``, so this is the only place a database refusing connections shows up. + ``_do_get`` is the wrong hook: ``QueuePool`` recurses into it when the non-blocking get comes back empty and the race for an overflow slot is lost, which is exactly the contended case worth measuring, and the wait would be observed twice. @@ -106,6 +115,7 @@ def instrument_pool_class(pool_class: type, metrics: CheckoutWaitRecorder) -> ty >>> poolclass = instrument_pool_class(resolve_pool_class("async_adapted_queue"), metrics) >>> engine = create_async_engine(url, poolclass=poolclass) """ + error_recorder = metrics if isinstance(metrics, ErrorRecorder) else None class InstrumentedPool(pool_class): # type: ignore[misc, valid-type] """Pool that reports how long each checkout waited.""" @@ -114,7 +124,7 @@ def connect(self) -> Any: started = time.perf_counter() try: connection = super().connect() - except SATimeoutError: + except (SATimeoutError, TimeoutError): _safe_metric_call( lambda: metrics.record_checkout_wait( duration=time.perf_counter() - started, @@ -123,12 +133,29 @@ def connect(self) -> Any: "Failed to record database checkout timeout", ) raise + except Exception as exc: + _safe_metric_call( + lambda: metrics.record_checkout_wait(duration=time.perf_counter() - started), + "Failed to record database checkout wait", + ) + if error_recorder is not None and not self._reported_by_handle_error(exc): + error_type = type(exc).__name__ + _safe_metric_call( + lambda: error_recorder.record_error(error_type=error_type), + "Failed to record database connect error", + ) + raise _safe_metric_call( lambda: metrics.record_checkout_wait(duration=time.perf_counter() - started), "Failed to record database checkout wait", ) return connection + def _reported_by_handle_error(self, exc: Exception) -> bool: + """Whether the engine turns ``exc`` into a ``handle_error`` event itself.""" + dbapi = getattr(self._dialect, "loaded_dbapi", None) + return dbapi is not None and isinstance(exc, dbapi.Error) + setattr(InstrumentedPool, _WAIT_RECORDER_ATTRIBUTE, metrics) InstrumentedPool.__name__ = f"Instrumented{pool_class.__name__}" InstrumentedPool.__qualname__ = InstrumentedPool.__name__ @@ -208,11 +235,11 @@ def on_checkin(dbapi_connection: Any, connection_record: Any) -> None: def on_error(exception_context: Any) -> None: """Handle database error event.""" - exc = exception_context.original_exception - error_type = type(exc).__name__ - is_timeout = isinstance(exc, (TimeoutError, SATimeoutError)) + # Only errors on an established connection get here -- a statement timeout among + # them. Timeouts acquiring a connection are counted by the instrumented pool. + error_type = type(exception_context.original_exception).__name__ _safe_metric_call( - lambda: metrics.record_error(error_type=error_type, is_timeout=is_timeout), + lambda: metrics.record_error(error_type=error_type), "Failed to record database error metric", ) diff --git a/tests/integration/test_metrics_integration.py b/tests/integration/test_metrics_integration.py index 72f390c..32b7c01 100644 --- a/tests/integration/test_metrics_integration.py +++ b/tests/integration/test_metrics_integration.py @@ -280,3 +280,111 @@ async def test__postgres_metrics__pool_gauges__describe_the_pool_while_busy_and_ assert _gauges(await _while_holding(manager, 12, prefix), prefix) == (10.0, 12.0, 2.0) # ...and after the burst the overflow connections are closed and nothing is in use assert _gauges(_exposed(prefix), prefix) == (10.0, 0.0, 0.0) + + +# ============================================================================ +# What Counts as a Connection Timeout Tests +# ============================================================================ + + +def _connection_series(prefix: str) -> tuple[float, float, float]: + """``(timeouts, errors over all labels, checkout waits)`` as scraped.""" + exposed = _exposed(prefix) + errors = sum(v for k, v in exposed.items() if k.startswith(f"{prefix}_postgres_db_connection_errors_total{{")) + return ( + exposed[f"{prefix}_postgres_db_connection_timeouts_total"], + errors, + exposed[f"{prefix}_postgres_db_connection_checkout_wait_seconds_count"], + ) + + +def _error_types(prefix: str) -> set[str]: + """The ``error_type`` labels ``connection_errors_total`` has seen.""" + marker = f'{prefix}_postgres_db_connection_errors_total{{error_type="' + return {k[len(marker) :].split('"', 1)[0] for k in _exposed(prefix) if k.startswith(marker)} + + +def _metrics_manager(url: str, prefix: str, connect_args: dict[str, object]) -> AsyncSessionManager[AsyncSession]: + from sqlalchemy_foundation_kit.contrib.metrics import PostgresMetrics + + return AsyncSessionManager( + url, + poolclass="async_adapted_queue", + metrics=PostgresMetrics(prefix=prefix), + pool_settings=_PoolSettings(size=2, timeout=5.0), + connect_args=connect_args, + ) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test__postgres_metrics__statement_timeout__is_not_a_connection_timeout( + postgres_container: PostgresContainer, +) -> None: + # Arrange: asyncpg's command_timeout fires on a running statement; the pool never waited + pytest.importorskip("prometheus_client") + prefix = f"it_{uuid.uuid4().hex[:8]}" + manager = _metrics_manager(asyncpg_url(postgres_container), prefix, {"command_timeout": 0.1}) + + # Act + async with manager: + with pytest.raises(TimeoutError): + async with manager.get_session() as session: + await session.execute(text("SELECT pg_sleep(1)")) + + # Assert + timeouts, errors, waits = _connection_series(prefix) + assert timeouts == 0.0 + assert _error_types(prefix) == {"TimeoutError"} + assert (errors, waits) == (1.0, 1.0) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test__postgres_metrics__connect_timeout__counts_a_connection_timeout_and_its_wait() -> None: + # Arrange: a server that accepts the TCP connection and never answers the startup packet + pytest.importorskip("prometheus_client") + + async def silent(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: + await reader.read() # until the client gives up and closes + writer.close() + + server = await asyncio.start_server(silent, "127.0.0.1", 0) + port = server.sockets[0].getsockname()[1] + prefix = f"it_{uuid.uuid4().hex[:8]}" + manager = _metrics_manager(f"postgresql+asyncpg://u:p@127.0.0.1:{port}/db", prefix, {"timeout": 0.3}) + + # Act + async with server, manager: + with pytest.raises(TimeoutError): + async with manager.get_session() as session: + await session.execute(text("SELECT 1")) + + # Assert + timeouts, errors, waits = _connection_series(prefix) + assert (timeouts, errors, waits) == (1.0, 0.0, 1.0) + assert _exposed(prefix)[f"{prefix}_postgres_db_connection_checkout_wait_seconds_sum"] >= 0.3 + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test__postgres_metrics__connection_refused__counts_an_error_and_its_wait() -> None: + # Arrange: a port nothing listens on + pytest.importorskip("prometheus_client") + server = await asyncio.start_server(lambda r, w: None, "127.0.0.1", 0) + port = server.sockets[0].getsockname()[1] + server.close() + await server.wait_closed() + prefix = f"it_{uuid.uuid4().hex[:8]}" + manager = _metrics_manager(f"postgresql+asyncpg://u:p@127.0.0.1:{port}/db", prefix, {}) + + # Act + async with manager: + with pytest.raises(OSError): + async with manager.get_session() as session: + await session.execute(text("SELECT 1")) + + # Assert + timeouts, errors, waits = _connection_series(prefix) + assert (timeouts, errors, waits) == (0.0, 1.0, 1.0) + assert _error_types(prefix) == {"ConnectionRefusedError"} diff --git a/tests/unit/session/test_manager.py b/tests/unit/session/test_manager.py index 7a52de7..8fda316 100644 --- a/tests/unit/session/test_manager.py +++ b/tests/unit/session/test_manager.py @@ -359,14 +359,11 @@ def test__attach_metrics__error_records_error_type() -> None: error_callback(exception_context) # Assert - mock_metrics.record_error.assert_called_once() - call_args = mock_metrics.record_error.call_args[1] - assert call_args["error_type"] == "ValueError" - assert call_args["is_timeout"] is False + mock_metrics.record_error.assert_called_once_with(error_type="ValueError") -def test__attach_metrics__timeout_error__marks_as_timeout() -> None: - # Arrange +def test__attach_metrics__statement_timeout__is_an_error_not_a_connection_timeout() -> None: + # Arrange: asyncpg's command_timeout on a running statement -- the connection was fine mock_engine = Mock() mock_metrics = Mock() @@ -377,32 +374,11 @@ def test__attach_metrics__timeout_error__marks_as_timeout() -> None: error_callback = [c[0][2] for c in mock_event.listen.call_args_list if c[0][1] == "handle_error"][0] exception_context = Mock() - exception_context.original_exception = TimeoutError("Connection timeout") + exception_context.original_exception = TimeoutError() error_callback(exception_context) # Assert - call_args = mock_metrics.record_error.call_args[1] - assert call_args["is_timeout"] is True - - -def test__attach_metrics__sqlalchemy_timeout__marks_as_timeout() -> None: - # Arrange - mock_engine = Mock() - mock_metrics = Mock() - - # Act - with patch("sqlalchemy_foundation_kit.session.manager.event") as mock_event: - attach_metrics(mock_engine, mock_metrics) - - error_callback = [c[0][2] for c in mock_event.listen.call_args_list if c[0][1] == "handle_error"][0] - - exception_context = Mock() - exception_context.original_exception = SATimeoutError("Pool timeout") - error_callback(exception_context) - - # Assert - call_args = mock_metrics.record_error.call_args[1] - assert call_args["is_timeout"] is True + mock_metrics.record_error.assert_called_once_with(error_type="TimeoutError") def test__attach_metrics__pool_without_size__uses_zero() -> None: @@ -500,6 +476,87 @@ def test__instrument_pool_class__exhausted_pool__counts_the_timeout_and_reraises held.close() +class _WaitAndErrorRecorder(_WaitRecorder): + """Also takes errors, as ``PostgresMetrics`` does.""" + + def __init__(self) -> None: + super().__init__() + self.errors: list[str] = [] + + def record_error(self, error_type: str, is_timeout: bool = False) -> None: + self.errors.append(error_type) + + +class _FakeDBAPIError(Exception): + """Stands in for the driver's DBAPI ``Error``.""" + + +def _failing_pool(recorder: _WaitRecorder, error: Exception) -> QueuePool: + """An instrumented QueuePool whose every attempt to open a connection raises ``error``.""" + + def creator() -> None: + raise error + + poolclass = instrument_pool_class(QueuePool, recorder) + return cast("QueuePool", poolclass(creator, pool_size=1, max_overflow=0, timeout=0.1)) + + +def test__instrument_pool_class__connect_timeout__counts_the_timeout_and_reraises() -> None: + # Arrange: asyncpg raises a builtin TimeoutError when the connect handshake runs out + recorder = _WaitAndErrorRecorder() + pool = _failing_pool(recorder, TimeoutError()) + + # Act + with pytest.raises(TimeoutError): + pool.connect() + + # Assert + assert recorder.timeouts == 1 + assert recorder.errors == [] + + +def test__instrument_pool_class__connection_refused__records_the_wait_and_the_error() -> None: + # Arrange + recorder = _WaitAndErrorRecorder() + pool = _failing_pool(recorder, ConnectionRefusedError()) + + # Act + with pytest.raises(ConnectionRefusedError): + pool.connect() + + # Assert + assert [timed_out for _duration, timed_out in recorder.waits] == [False] + assert recorder.errors == ["ConnectionRefusedError"] + + +def test__instrument_pool_class__driver_dbapi_error__left_to_handle_error() -> None: + # Arrange: a driver that translates connect errors reports them through handle_error + recorder = _WaitAndErrorRecorder() + pool = _failing_pool(recorder, _FakeDBAPIError()) + pool._dialect = Mock(loaded_dbapi=Mock(Error=_FakeDBAPIError)) + + # Act + with pytest.raises(_FakeDBAPIError): + pool.connect() + + # Assert + assert len(recorder.waits) == 1 + assert recorder.errors == [] + + +def test__instrument_pool_class__connect_error_with_a_wait_only_recorder__records_the_wait() -> None: + # Arrange + recorder = _WaitRecorder() + pool = _failing_pool(recorder, ConnectionRefusedError()) + + # Act + with pytest.raises(ConnectionRefusedError): + pool.connect() + + # Assert + assert [timed_out for _duration, timed_out in recorder.waits] == [False] + + def test__instrument_pool_class__measures_the_wait_not_the_time_held() -> None: # Arrange: the first caller holds its connection for HOLD seconds; the second one is # queued behind it the whole time and then gives the connection straight back. What