Skip to content

fix: time out beacon POST requests, declare requests, update deps - #44

Merged
mateuszsokola merged 4 commits into
mainfrom
bugfix
Aug 18, 2026
Merged

fix: time out beacon POST requests, declare requests, update deps#44
mateuszsokola merged 4 commits into
mainfrom
bugfix

Conversation

@mateuszsokola

@mateuszsokola mateuszsokola commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

_make_post_request sends no request timeout, unlike the GET path directly above it:

def _make_get_request_with_params(self, endpoint, params):
    return self._request_session_manager.json_make_get_request(uri, timeout=self.request_timeout, params=params)

def _make_post_request(self, endpoint, json_data):
    response = session.post(uri, json=json_data)   # no timeout

Why it matters more than it looks

AsyncBeacon runs these through _run_as_async, i.e. in the default executor. A caller wrapping the coroutine in asyncio.wait_for gets no protection from that: wait_for cancels the waiter, not the thread. The worker stays blocked in requests.post for as long as the beacon node holds the socket open — indefinitely, with no timeout set.

So a stalling beacon node leaks a thread per call. Repeat it and the default executor is exhausted, at which point every executor-backed method here — get_validators, get_genesis, the pending-queue reads — stops making progress too. A caller that looks defensive, with a wait_for around each call, is not.

The POST path is also the one that carries the whole-validator-set requests (get_validators, get_attestations_rewards), which are exactly the calls most likely to be slow.

The fix

Pass self.request_timeout, matching the GET path. One line, and the timeout value is already configured per client.

The test asserting the exact post call now pins the timeout too, so the argument cannot be dropped again without a failure:

mocked_session.post.assert_called_once_with(f"http://127.0.0.1:8545{path}", json=["7", "8"], timeout=10.0)

Verification

23 tests pass. 0.12.1 in setup.cfg and .bumpversion.cfg together — patch, since this changes no interface.

Consumer note

A consumer pinning ==0.12.0 needs bumping to 0.12.1 for this to take effect; the timeout lives entirely inside the library.

Dependency updates folded in

Five of the six open dependabot PRs are absorbed here so they do not need merging separately. Every target version is at least 14 days old, per the min-release-age policy:

PR Change Action
#38 actions/upload-artifact 4 → 7 applied — sast.yml was the last v4 reference
#40 pkginfo >=1.10 → >=1.12.1.2 removed instead — nothing imports it; it is a twine dependency that was sitting in runtime requirements
#41 importlib-metadata 7.0.1 → 9.0.0 removed instead — nothing imports it
#42 twine 6.0.0 → 7.0.0 applied — used by twine check in the publish workflow
#43 requests >=2.32.0 → >=2.34.2 applied, and see below

#43 exposed a real packaging bug. web3_utils/async_beacon.py and web3_utils/retryable_eth_module.py both from requests import ConnectionError, HTTPError, but requests was never in install_requires — it only resolved because web3 and python-gitlab happen to pull it. Any consumer whose resolver dropped those transitive paths would have hit an ImportError at import time. It is now declared.

#39 is deliberately left open (web3 7.15.0 → 7.16.0, python-gitlab 8.3.0 → 8.5.0). requirements.txt is CI-only, since install_requires is unpinned — so these pins decide what the library is tested against, and the consumer currently runs web3==7.15.0 and resolves python-gitlab==8.3.0. Bumping only here would put CI ahead of production, which is the same drift that previously hid gitlab.py being tested three majors behind what actually ran. Worth doing as a coordinated bump in both repos, with the consumer's suite as the check.

Version stays at 0.12.1: declaring a dependency the package already imported is a fix, not a new capability. Happy to make it 0.13.0 if you read install_requires changes as minor.

@mateuszsokola
mateuszsokola requested a review from a team as a code owner August 18, 2026 12:58
@mateuszsokola mateuszsokola changed the title fix: time out beacon POST requests fix: time out beacon POST requests, declare requests, update deps Aug 18, 2026
@mateuszsokola
mateuszsokola merged commit 226a520 into main Aug 18, 2026
2 checks passed
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.

3 participants