From ed4623cd8383ff928d4acb968d46681572f63a95 Mon Sep 17 00:00:00 2001 From: moe-berri Date: Sun, 8 Nov 2026 13:22:22 +0800 Subject: [PATCH] fix(alerts): retry failed email delivery without replaying cached errors --- .../integrations/SlackAlerting/usage_alerts.py | 12 +++++++----- .../observability/test_usage_alert_email.py | 16 ++++++++++++---- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/litellm/integrations/SlackAlerting/usage_alerts.py b/litellm/integrations/SlackAlerting/usage_alerts.py index dbfaafe1b10..0d69406ca1a 100644 --- a/litellm/integrations/SlackAlerting/usage_alerts.py +++ b/litellm/integrations/SlackAlerting/usage_alerts.py @@ -117,7 +117,7 @@ class UsageAlertDelivery: if previous is not None: if not previous.done(): raise RuntimeError("An alert email is still in flight; retry later") - if cls._email_key == key: + if cls._email_key == key and not previous.cancelled() and previous.exception() is None: cls._email_task = None cls._email_key = None return previous.result() @@ -125,10 +125,12 @@ class UsageAlertDelivery: cls._email_task = task cls._email_key = key task.add_done_callback(cls._email_finished) - await asyncio.shield(task) - if cls._email_task is task: - cls._email_task = None - cls._email_key = None + try: + await asyncio.shield(task) + finally: + if task.done() and cls._email_task is task: + cls._email_task = None + cls._email_key = None @staticmethod async def _send_email(recipient: str, subject: str, html: str) -> None: diff --git a/tests/integration/observability/test_usage_alert_email.py b/tests/integration/observability/test_usage_alert_email.py index 63c1d387945..961f304f547 100644 --- a/tests/integration/observability/test_usage_alert_email.py +++ b/tests/integration/observability/test_usage_alert_email.py @@ -139,7 +139,10 @@ async def test_cancelled_smtp_attempt_retains_only_one_send(monkeypatch: pytest. @pytest.mark.asyncio -async def test_smtp_greeting_over_two_seconds_still_delivers(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize("fail_first", (False, True)) +async def test_smtp_greeting_over_two_seconds_still_delivers( + monkeypatch: pytest.MonkeyPatch, fail_first: bool +) -> None: messages: Final[list[bytes]] = [] async def slow_greeting(reader: asyncio.StreamReader, writer: asyncio.StreamWriter) -> None: @@ -152,7 +155,7 @@ async def test_smtp_greeting_over_two_seconds_still_delivers(monkeypatch: pytest writer.write(b"354 End with a dot\r\n") await writer.drain() messages.append(await reader.readuntil(b"\r\n.\r\n")) - writer.write(b"250 Accepted\r\n") + writer.write(b"451 Retry later\r\n" if fail_first and len(messages) == 1 else b"250 Accepted\r\n") elif command.upper().startswith(b"QUIT"): writer.write(b"221 Closing\r\n") break @@ -179,8 +182,13 @@ async def test_smtp_greeting_over_two_seconds_still_delivers(monkeypatch: pytest } ) try: - assert await UsageAlertDelivery(SlackAlerting(alerting=["email"], alert_types=[])).send_issue(pending) - assert len(messages) == 1 and b"View alert" in messages[0] and b"View source activity" in messages[0] + delivery: Final = UsageAlertDelivery(SlackAlerting(alerting=["email"], alert_types=[])) + if fail_first: + with pytest.raises(smtplib.SMTPDataError): + await delivery.send_issue(pending) + assert await delivery.send_issue(pending) + assert len(messages) == (2 if fail_first else 1) + assert b"View alert" in messages[-1] and b"View source activity" in messages[-1] finally: server.close() await server.wait_closed()