From 32e93a56771379f24970b12c966eda0eb7c848fd Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:23:03 +0000 Subject: [PATCH 1/4] Fix peer.py to catch requests.HTTPError not httpx.HTTPStatusError update_peer_meta calls requests.get and then response.raise_for_status(), which raises requests.exceptions.HTTPError, but the handler caught httpx.HTTPStatusError. Any non-2xx peer response therefore escaped the handler instead of marking the peer unreachable. Catch the correct exception type and drop the now-unused httpx import. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/peer.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shard_core/service/peer.py b/shard_core/service/peer.py index f0d040b7..b693da59 100644 --- a/shard_core/service/peer.py +++ b/shard_core/service/peer.py @@ -1,7 +1,6 @@ import asyncio import logging -import httpx import requests from fastapi.requests import Request from http_message_signatures import HTTPSignatureKeyResolver, algorithms @@ -49,7 +48,7 @@ def do_request(): try: response.raise_for_status() - except httpx.HTTPStatusError as e: + except requests.exceptions.HTTPError as e: log.debug(f"Could not update peer meta for {peer.short_id}: {e}") async with db_conn() as conn: await db_peers.update_by_id(conn, peer.id, {"is_reachable": False}) From 6f8e8162aa340a6dd4b230671f0f165f7715450b Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:23:12 +0000 Subject: [PATCH 2/4] Isolate peer refresh failures with return_exceptions in gather update_all_peer_pubkeys gathered update_peer_meta for every peer without return_exceptions, so one peer raising (e.g. an unexpected error not caught inside update_peer_meta) aborted refreshing all remaining peers for that cycle. Collect exceptions instead and log each, so one bad peer cannot starve the others. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/peer.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/shard_core/service/peer.py b/shard_core/service/peer.py index b693da59..bf0cd353 100644 --- a/shard_core/service/peer.py +++ b/shard_core/service/peer.py @@ -29,7 +29,15 @@ async def update_all_peer_pubkeys(): async with db_conn() as conn: all_peers = await db_peers.get_all(conn) peers_with_pubkey = [Peer(**p) for p in all_peers if p.get("public_bytes_b64")] - await asyncio.gather(*[update_peer_meta(peer) for peer in peers_with_pubkey]) + results = await asyncio.gather( + *[update_peer_meta(peer) for peer in peers_with_pubkey], + return_exceptions=True, + ) + for peer, result in zip(peers_with_pubkey, results): + if isinstance(result, Exception): + log.warning( + f"Failed to update peer meta for {peer.short_id}: {result}" + ) async def update_peer_meta(peer: Peer): From c59b355b06577eff3ee7f235787c9061fd6e3de9 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:27:28 +0000 Subject: [PATCH 3/4] Add regression tests for peer refresh error isolation Two tests over update_all_peer_pubkeys: a peer returning 500 is marked unreachable without blocking other peers' updates (covers the corrected requests.HTTPError handling), and a peer raising an unexpected error does not abort the refresh cycle for the others (covers gather return_exceptions). Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/peer.py | 4 +- tests/test_peer_refresh.py | 79 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 tests/test_peer_refresh.py diff --git a/shard_core/service/peer.py b/shard_core/service/peer.py index bf0cd353..7c872066 100644 --- a/shard_core/service/peer.py +++ b/shard_core/service/peer.py @@ -35,9 +35,7 @@ async def update_all_peer_pubkeys(): ) for peer, result in zip(peers_with_pubkey, results): if isinstance(result, Exception): - log.warning( - f"Failed to update peer meta for {peer.short_id}: {result}" - ) + log.warning(f"Failed to update peer meta for {peer.short_id}: {result}") async def update_peer_meta(peer: Peer): diff --git a/tests/test_peer_refresh.py b/tests/test_peer_refresh.py new file mode 100644 index 00000000..1b48a702 --- /dev/null +++ b/tests/test_peer_refresh.py @@ -0,0 +1,79 @@ +import responses +from starlette import status + +from shard_core.data_model.identity import Identity, OutputIdentity +from shard_core.database import peers as db_peers +from shard_core.database.connection import db_conn +from shard_core.data_model.peer import Peer +from shard_core.service.peer import update_all_peer_pubkeys + + +def _whoareyou_url(identity: Identity) -> str: + return f"https://{identity.short_id}.freeshard.cloud/core/public/meta/whoareyou" + + +async def _insert_peer(identity: Identity, name: str): + async with db_conn() as conn: + await db_peers.insert( + conn, + { + "id": identity.id, + "name": name, + "public_bytes_b64": identity.public_key_pem, + "is_reachable": True, + }, + ) + + +async def _get_peer(identity: Identity) -> Peer: + async with db_conn() as conn: + return Peer(**await db_peers.get_by_id_prefix(conn, identity.id)) + + +async def test_unreachable_peer_does_not_block_others(db): + bad = Identity.create("bad peer") + good = Identity.create("good peer") + await _insert_peer(bad, name="stale bad") + await _insert_peer(good, name="stale good") + + with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: + rsps.get(_whoareyou_url(bad), status=status.HTTP_500_INTERNAL_SERVER_ERROR) + rsps.get( + _whoareyou_url(good), + json=OutputIdentity(**good.model_dump()).model_dump(), + ) + + await update_all_peer_pubkeys() + + good_peer = await _get_peer(good) + assert good_peer.name == "good peer" + assert good_peer.is_reachable is True + + bad_peer = await _get_peer(bad) + assert bad_peer.is_reachable is False + + +async def test_unexpected_peer_error_does_not_block_others(db): + broken = Identity.create("broken peer") + good = Identity.create("good peer") + await _insert_peer(broken, name="stale broken") + await _insert_peer(good, name="stale good") + + with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: + # broken peer answers with a different identity than its stored id, + # which raises inside update_peer_meta and is not caught there + rsps.get( + _whoareyou_url(broken), + json=OutputIdentity( + **Identity.create("impostor").model_dump() + ).model_dump(), + ) + rsps.get( + _whoareyou_url(good), + json=OutputIdentity(**good.model_dump()).model_dump(), + ) + + await update_all_peer_pubkeys() + + good_peer = await _get_peer(good) + assert good_peer.name == "good peer" From 2121f8d35d03073c9e3217b018d649781e4d84bd Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 10:06:02 +0000 Subject: [PATCH 4/4] Address review: strengthen peer refresh tests and alias exception - Use requests.HTTPError alias to read symmetrically with the adjacent requests.ConnectionError handler. - Insert the good peer as is_reachable=False so the success path's flip to True is actually asserted (was decorative). - Assert both whoareyou requests fired, pinning the HTTPError branch rather than a silent ConnectionError fallback. - Assert the unexpected-error peer is left untouched and that the swallowed failure is surfaced via a warning log. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/peer.py | 2 +- tests/test_peer_refresh.py | 26 +++++++++++++++++++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/shard_core/service/peer.py b/shard_core/service/peer.py index 7c872066..55dbe80f 100644 --- a/shard_core/service/peer.py +++ b/shard_core/service/peer.py @@ -54,7 +54,7 @@ def do_request(): try: response.raise_for_status() - except requests.exceptions.HTTPError as e: + except requests.HTTPError as e: log.debug(f"Could not update peer meta for {peer.short_id}: {e}") async with db_conn() as conn: await db_peers.update_by_id(conn, peer.id, {"is_reachable": False}) diff --git a/tests/test_peer_refresh.py b/tests/test_peer_refresh.py index 1b48a702..2db3c49b 100644 --- a/tests/test_peer_refresh.py +++ b/tests/test_peer_refresh.py @@ -1,3 +1,5 @@ +import logging + import responses from starlette import status @@ -12,7 +14,7 @@ def _whoareyou_url(identity: Identity) -> str: return f"https://{identity.short_id}.freeshard.cloud/core/public/meta/whoareyou" -async def _insert_peer(identity: Identity, name: str): +async def _insert_peer(identity: Identity, name: str, is_reachable: bool = True): async with db_conn() as conn: await db_peers.insert( conn, @@ -20,7 +22,7 @@ async def _insert_peer(identity: Identity, name: str): "id": identity.id, "name": name, "public_bytes_b64": identity.public_key_pem, - "is_reachable": True, + "is_reachable": is_reachable, }, ) @@ -34,7 +36,7 @@ async def test_unreachable_peer_does_not_block_others(db): bad = Identity.create("bad peer") good = Identity.create("good peer") await _insert_peer(bad, name="stale bad") - await _insert_peer(good, name="stale good") + await _insert_peer(good, name="stale good", is_reachable=False) with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: rsps.get(_whoareyou_url(bad), status=status.HTTP_500_INTERNAL_SERVER_ERROR) @@ -45,6 +47,10 @@ async def test_unreachable_peer_does_not_block_others(db): await update_all_peer_pubkeys() + # both peers were really contacted (the 500 path was exercised, not a + # silent ConnectionError fallback from a mismatched URL) + assert len(rsps.calls) == 2 + good_peer = await _get_peer(good) assert good_peer.name == "good peer" assert good_peer.is_reachable is True @@ -53,11 +59,11 @@ async def test_unreachable_peer_does_not_block_others(db): assert bad_peer.is_reachable is False -async def test_unexpected_peer_error_does_not_block_others(db): +async def test_unexpected_peer_error_does_not_block_others(db, memory_logger): broken = Identity.create("broken peer") good = Identity.create("good peer") await _insert_peer(broken, name="stale broken") - await _insert_peer(good, name="stale good") + await _insert_peer(good, name="stale good", is_reachable=False) with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps: # broken peer answers with a different identity than its stored id, @@ -77,3 +83,13 @@ async def test_unexpected_peer_error_does_not_block_others(db): good_peer = await _get_peer(good) assert good_peer.name == "good peer" + assert good_peer.is_reachable is True + + # the unexpected error is swallowed by the gather and left the broken peer + # untouched, and it was surfaced via a warning + broken_peer = await _get_peer(broken) + assert broken_peer.name == "stale broken" + assert any( + r.levelno == logging.WARNING and broken.short_id in r.getMessage() + for r in memory_logger.records + )