fix: a refused attachment no longer costs a group its release notes

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Krzysztof Rudnicki 2026-08-30 21:33:56 +02:00
parent 3d6ae0680c
commit dc642b8a1f
2 changed files with 41 additions and 64 deletions

View File

@ -100,13 +100,17 @@ async def announce_to(
text = render(pope, pending)
try:
image = await portraits.get_base64(pope)
except Exception:
# The marker is already written, so this must never propagate: a
# Commons outage costs the portrait, not the release notes.
logger.exception("portrait for %s unavailable; announcing %s as text", version, group_id)
await transport.send_text(text, group_id)
return
await transport.send_image(image, group_id, caption=text)
except Exception:
# Both the download and the send are inside this: the marker is
# already written, so either failing would otherwise cost the group
# its release notes permanently. The attachment turns a 2 KB text
# post into a megabyte of base64, which signal-cli can refuse on its
# own terms while it is perfectly able to take the text -- and a
# duplicate post, if the send failed after delivery, is far cheaper
# than release notes nobody ever sees.
logger.exception("photo announcement of %s failed for %s; sending text", version, group_id)
await transport.send_text(text, group_id)
async def announce_all(

View File

@ -8,7 +8,7 @@ from typing import TYPE_CHECKING
import httpx
from signal_bot.announce import ANNOUNCED_KEY, announce_all, announce_to, render
from signal_bot.announce import ANNOUNCED_KEY, announce_all, announce_to
from signal_bot.changelog import parse
from signal_bot.http_client import ImageFetcher
from signal_bot.portraits import PortraitCache
@ -30,7 +30,6 @@ DOCUMENT = """\
- pierwsza zmiana
"""
RELEASES = parse(DOCUMENT)
PETRUS = pope_for(1)
LINUS = pope_for(2)
@ -69,63 +68,6 @@ async def _announce(
await announce_to(transport, store, GROUP, RELEASES, pope, cache)
def test_a_single_release_is_rendered_flat() -> None:
"""The header already names it; a per-release block would just repeat it."""
message = render(LINUS, RELEASES[1:])
assert message == (
"Wersja 0.0.1 «Linus» wydana\n"
"https://pl.wikipedia.org/wiki/Linus_%28papie%C5%BC%29\n"
"Changelog:\n- druga zmiana"
)
def test_a_long_backlog_is_capped_and_says_so() -> None:
"""A group added years from now must not be handed 200 releases."""
document = "\n\n".join(
f"## {v} — Pope{i}\n\n- zmiana {i}"
for i, v in enumerate(
[
"0.0.0",
"0.0.1",
"0.0.2",
"0.0.3",
"0.0.4",
"0.0.5",
"0.0.6",
"0.0.7",
"0.1.0",
"0.1.1",
"0.1.2",
"0.1.3",
]
)
)
message = render(pope_for(12), parse(document))
assert "(pominieto 2 starszych wersji)" in message
assert "zmiana 0" not in message
assert "zmiana 11" in message
def test_a_block_heading_ignores_the_name_typed_in_the_document() -> None:
"""The heading says Pope3; popes.json is what the group is actually told."""
message = render(pope_for(12), parse("## 0.0.2 — Pope3\n\n- x\n\n## 0.0.3 — Pope4\n\n- y"))
assert "0.0.2 «Anacletus»" in message
assert "0.0.3 «Clemens I»" in message
assert "Pope3" not in message
def test_render_names_the_version_and_lists_every_pending_line() -> None:
message = render(LINUS, RELEASES)
assert message.startswith("Wersja 0.0.1 «Linus» wydana")
assert "- pierwsza zmiana" in message
assert "- druga zmiana" in message
def test_render_links_the_polish_article() -> None:
"""The same article !papież posts, built from the disambiguated title."""
assert "https://pl.wikipedia.org/wiki/Piotr_Aposto%C5%82" in render(PETRUS, RELEASES)
async def test_a_group_told_nothing_yet_hears_the_ladder_from_peter(
store: Store, transport: FakeTransport
) -> None:
@ -165,6 +107,29 @@ async def test_an_unreachable_portrait_costs_the_photo_not_the_announcement(
assert store.meta(GROUP, ANNOUNCED_KEY) == "0.0.1"
async def test_a_refused_attachment_still_lands_the_notes_as_text(
store: Store, transport: FakeTransport
) -> None:
"""A megabyte of base64 is refusable in ways a 2 KB text post is not."""
class RefusingImages(FakeTransport):
async def send_image(
self,
image_base64: str,
recipient: str,
quote: Quote | None = None,
caption: str = "",
) -> None:
raise RuntimeError(recipient)
refusing = RefusingImages()
await _announce(store, refusing)
assert len(refusing.sent) == 1
assert refusing.sent[0].image_base64 is None
assert "Wersja 0.0.1 «Linus» wydana" in (refusing.sent[0].text or "")
assert store.meta(GROUP, ANNOUNCED_KEY) == "0.0.1"
async def test_a_group_already_on_this_version_hears_nothing(
store: Store, transport: FakeTransport
) -> None:
@ -199,6 +164,9 @@ async def test_the_version_is_recorded_before_the_send(store: Store) -> None:
"""A crash between the two must not re-announce on every restart."""
class Exploding(FakeTransport):
async def send_text(self, text: str, recipient: str, quote: Quote | None = None) -> None:
raise RuntimeError(recipient, text)
async def send_image(
self,
image_base64: str,
@ -217,6 +185,11 @@ async def test_one_failing_group_does_not_silence_the_others(store: Store) -> No
other = "group.two"
class RefusingFirst(FakeTransport):
async def send_text(self, text: str, recipient: str, quote: Quote | None = None) -> None:
if recipient == GROUP:
raise RuntimeError(recipient)
await super().send_text(text, recipient, quote)
async def send_image(
self,
image_base64: str,