fix: bound remote fetch connection pool acquire wait by fetch timeout

[cmyui]

Summary

Sibling to #77, independently mergeable. #77 makes the connection pool's size configurable; this PR bounds how long a fetch may wait for a connection from that pool.

Today the remote client acquires its pooled connection with no timeout:

# remote/client.py
conn = self._connection_pool.acquire()

With the pool's default timeout=None, the blocking branch is an unbounded while self.is_pool_empty(): self._lock.wait() — so when all pooled connections are busy (with the current pool of 1: whenever two fetches overlap during evaluation-API latency), concurrent callers queue indefinitely. fetch_timeout_millis never applies to this wait; it only bounds the socket read after acquisition. In production we observed request handlers pinned in acquire() for the entire duration of an upstream latency event, and the queued backlog continued failing requests for minutes after the API recovered (details and magnitude in #77's description).

Change

The pool already implements a deadline path (acquire(timeout=...) raising EmptyPoolError) — it just was never used. This PR:

• passes fetch_timeout_millis as the acquire timeout at the single call site, and
• maps EmptyPoolError to a builtin TimeoutError with a descriptive message.

TimeoutError gets the same retry classification as the socket read timeout (socket.timeout is the same type on Python ≥ 3.10), so both ways of violating fetch_timeout_millis — waiting for a connection, or waiting on the read — are indistinguishable to callers: with fetch_retries=0 (default) fetch_v2 returns the usual empty variants result; with retries configured it backs off and raises after exhaustion (each attempt's wait now bounded); async callbacks receive the error.

Tests: pool-exhausted fetch raises TimeoutError promptly instead of hanging; the timeout is classified retryable like a read timeout; end-to-end fetch_v2 parity (empty dict at fetch_retries=0, not None); existing fetch-options test updated for the new acquire(timeout=...) signature.

⚠️ Behavior change — maintainer input wanted

This is an intentional behavior change, and we'd like your opinion on it before merge:

• Before: a fetch whose connection-pool wait exceeds any duration simply blocks — indefinitely — with no error, no retry, and nothing recorded. With default config, worker threads can hang for the full length of an upstream event (and beyond, while the backlog drains).
• After: that fetch surfaces a TimeoutError after fetch_timeout_millis (default 10s), handled exactly like a socket read timeout — empty variants at fetch_retries=0, bounded retries then raise when retries are configured. Worst case per attempt becomes fetch_timeout_millis (queue wait) + fetch_timeout_millis (socket read) = 2× the configured value, instead of unbounded.

We believe bounding by default is the right call — an indefinite silent hang is strictly worse than a timely, handleable error, and users who set fetch_timeout_millis almost certainly believe it already bounds their fetch latency. But if you'd prefer no default-behavior change, we're happy to switch this to an opt-in fetch_pool_acquire_timeout_millis config (default None = current infinite wait) — it's a three-line delta. Let us know which shape you want.

Checklist

• Does your PR title have the correct title format?
• Does your PR have a breaking change?: behavioral — no API/signature changes, but previously-infinite pool waits now fail after fetch_timeout_millis (see section above).

---

Note

Medium Risk
Behavioral change under concurrent load: overlapping fetches may now time out waiting for a pool connection rather than hanging unbounded, affecting latency and error/retry outcomes.

Overview
Remote variant fetches no longer block indefinitely when the HTTP connection pool is exhausted (default max_size=1). __do_fetch now calls acquire(timeout=fetch_timeout_millis / 1000) and turns EmptyPoolError into a TimeoutError that names the configured wait and pool size.

RemoteEvaluationConfig docs clarify that fetch_timeout_millis also caps pool-queue wait, not only the socket read after a connection is obtained. With fetch_retries=0, a pool wait timeout follows the same path as other retryable timeouts (e.g. fetch returns {}); fetch_v2 can still propagate errors depending on retry settings.

Tests cover prompt failure under a held connection, TimeoutError retry classification, parity with read-timeout behavior on fetch_v2, and the mocked acquire(**kwargs) signature.

^{Reviewed by Cursor Bugbot for commit 207ae5a. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.}

^{Reviewed by Cursor Bugbot for commit ac735dc. Configure here.}

[cursor]

Pool timeout not surfaced publicly

Medium Severity

FetchException(408) from a pool acquire timeout is non-retryable, so __fetch_internal catches it and returns without re-raising. fetch_v2 then returns None instead of propagating the error, and async paths invoke callbacks with None variants and no error.

Additional Locations (1)

• src/amplitude_experiment/remote/client.py#L113-L117

 

^{Reviewed by Cursor Bugbot for commit ac735dc. Configure here.}

[cmyui]

Good catch, fixed in 207ae5a; the pool-acquire timeout now raises a builtin TimeoutError, which takes the same retry classification as the socket read timeout (socket.timeout = TimeoutError on 3.10+). Both ways of violating fetch_timeout_millis now behave identically.

[cmyui]

Also! I noticed that __fetch_internal returns None for any non-retryable FetchException instead of re-raising, despite fetch_v2's docstring saying it throws — that predates this PR and still applies to real 4xx responses. Happy to file that separately if maintainers agree it's a bug.

From what I can tell, the matrix is now:

Failure	Retry classification	fetch_retries=0 (default)	Retries configured, all fail
Socket read timeout	retryable	returns {}	raises after exhaustion
Pool-acquire timeout (this PR)	retryable	returns {}	raises after exhaustion
5xx / 429 response	retryable	returns {}	raises after exhaustion
Non-retryable 4xx response	not retried	returns None, silently	returns None, silently

[cmyui]

alternativly, add a new parameter (e.g. fetch_pool_acquire_timeout_millis) to avoid behavior changes to existing clients with their existing configuration