discord_rest: guild_snapshot fails open on non-Discord errors (server-26#225 S2 review)

A Firestore error inside classification (dropping membership, marking a
token invalid) propagated out of guild_snapshot and, behind asyncio.shield,
could die unretrieved. S3's start_stream must fail open, so the snapshot
now returns UNKNOWN (cached briefly) and logs only the exception type.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Logan Cusano
2026-10-05 02:02:29 -04:00
co-authored by Claude Opus 5.5
parent 2b3b875cdc
commit 1a6e3ca6a6
2 changed files with 31 additions and 4 deletions
+16 -4
View File
@@ -639,7 +639,19 @@ async def guild_snapshot(token_doc: dict, guild_id: str, *, refresh: bool = Fals
needed — and ``/guilds/{g}/channels``). ``refresh=True`` skips the cache needed — and ``/guilds/{g}/channels``). ``refresh=True`` skips the cache
(a concurrent load is still shared). Never raises for a Discord failure.""" (a concurrent load is still shared). Never raises for a Discord failure."""
guild_id = str(guild_id) guild_id = str(guild_id)
key = (str(token_doc.get("token_id")), guild_id, "snapshot") token_id = str(token_doc.get("token_id"))
return await access_cache.get_or_load( key = (token_id, guild_id, "snapshot")
key, lambda: _load_snapshot(token_doc, guild_id),
lambda s: ttl_for(s.status), refresh=refresh) async def _load() -> GuildSnapshot:
# Fail open on anything else too (a Firestore write inside the
# classification, a malformed body): the caller gets UNKNOWN, never
# an exception, and a shielded load never dies unretrieved.
try:
return await _load_snapshot(token_doc, guild_id)
except Exception as e:
logger.warning(f"Discord snapshot for bot {token_id} in {guild_id} failed: {type(e).__name__}")
return GuildSnapshot(token_id=token_id, guild_id=guild_id, status=UNKNOWN,
bot_user_id=token_doc.get("bot_user_id"),
error=f"check failed ({type(e).__name__})")
return await access_cache.get_or_load(key, _load, lambda s: ttl_for(s.status), refresh=refresh)
+15
View File
@@ -334,6 +334,21 @@ async def test_missing_access_alone_never_changes_membership(discord, fs):
assert snap.status == rest.UNKNOWN and fs.get("bot_tokens", "t1")["guild_ids"] == [G] assert snap.status == rest.UNKNOWN and fs.get("bot_tokens", "t1")["guild_ids"] == [G]
async def test_non_discord_failure_is_unknown_not_raised(discord, fs, monkeypatch):
# A Firestore error while classifying (here: dropping membership) must
# fail open — S3's start_stream relies on the snapshot never raising.
_guild_routes(discord)
discord.routes[("GET", f"/guilds/{G}")] = httpx.Response(404, json={"code": 10004})
tok = _token_doc(fs)
async def _boom(*a, **kw):
raise RuntimeError("firestore down")
monkeypatch.setattr(rest, "_drop_membership", _boom)
snap = await rest.guild_snapshot(tok, G)
assert snap.status == rest.UNKNOWN and "RuntimeError" in snap.error
assert TOKEN not in snap.error
async def test_unknown_result_is_cached_briefly(discord, fs): async def test_unknown_result_is_cached_briefly(discord, fs):
discord.routes[("GET", f"/guilds/{G}")] = httpx.Response(503) discord.routes[("GET", f"/guilds/{G}")] = httpx.Response(503)
tok = _token_doc(fs) tok = _token_doc(fs)