diff --git a/docs/design.md b/docs/design.md index 7bd67a84..44910064 100644 --- a/docs/design.md +++ b/docs/design.md @@ -164,7 +164,11 @@ CLI 负责关闭自己创建的模型服务对象及其网络资源。测试或 每天最后一个 briefing 时段会查询当天是否已经成功发送过变化提醒。尚未发送时，即使模型认为无需提醒，也会投递一条无声消息。手动运行不使用无声投递。 -模型返回平台无关的 `BriefingResult`。Telegram 和纯文本渲染器分别负责标题、链接、转义和长度限制。Telegram publisher 把静默标志转换为 `disable_notification=true`。 +模型返回平台无关的 `BriefingResult`。Telegram 和纯文本渲染器分别负责标题、链接、转义和长度限制。 + +Telegram publisher 把静默标志转换为 `disable_notification=true`。INFO 日志记录消息长度、分块数量、单条消息模式和静默投递选项，但不记录正文、Bot Token 或 Chat ID。 + +Telegram 拒绝请求时，publisher 记录 HTTP 状态、分块位置和安全错误类别。错误类别来自已知的 Telegram API 描述，未知响应不会原样进入日志。 ## 状态 diff --git a/docs/notes.md b/docs/notes.md index 69336c2b..c8bd2e09 100644 --- a/docs/notes.md +++ b/docs/notes.md @@ -117,3 +117,11 @@ any-llm 提供稳定的统一生命周期接口后，应删除属性探测并直 完整正文诊断只记录应用自己控制的字段，并要求额外的限时开关。 如果第三方 SDK 以后提供稳定的结构化安全日志接口，可以评估接入；不能依赖异常消息或事后正则清洗来保护隐私。 + +## Telegram 投递继续直接调用 Bot API + +Telegram 投递目前只调用 Bot API 的 `sendMessage`。项目继续复用已有的 HTTPX 客户端、统一网络生命周期和隐私安全日志，不为这一个端点引入 `python-telegram-bot` 等完整框架。 + +Telegram publisher 只负责构造请求、平台长度限制、HTML 分块和安全错误分类。它不实现更新轮询、Webhook、会话状态或 Bot 命令路由。 + +如果投递开始使用多个 Telegram 端点、需要接收更新，或者 Bot API 兼容工作持续增加，就重新评估维护活跃的 SDK，并删除能由 SDK 可靠承担的自定义协议代码。 diff --git a/tests/test_publishers.py b/tests/test_publishers.py index c5b4e1e8..ccd6d097 100644 --- a/tests/test_publishers.py +++ b/tests/test_publishers.py @@ -3,6 +3,7 @@ import httpx import pytest +from weather_briefing.api_client import LoggedAsyncClient from weather_briefing.models import RenderedMessage from weather_briefing.publishers import ( DeliveryError, @@ -117,7 +118,10 @@ def handler(request: httpx.Request) -> httpx.Response: assert payload["chat_id"] == "runtime-chat" assert payload["parse_mode"] == "HTML" assert payload["disable_notification"] is False - assert "Telegram delivery prepared: visible_characters=11 payload_characters=18 chunks=1" in caplog.text + assert ( + "Telegram delivery prepared: visible_characters=11 payload_characters=18 chunks=1 " + "single_message=False silent=False" + ) in caplog.text assert "Telegram chunk accepted: index=1/1 payload_characters=18" in caplog.text assert ( "Sensitive rendered text diagnostic: stage=telegram-chunk-1-of-1 body='Title\\n\\nBody'" @@ -126,19 +130,21 @@ def handler(request: httpx.Request) -> httpx.Response: assert "runtime-chat" not in caplog.text -async def test_telegram_publisher_uses_bot_api_silent_delivery() -> None: +async def test_telegram_publisher_uses_bot_api_silent_delivery(caplog) -> None: requests: list[httpx.Request] = [] def handler(request: httpx.Request) -> httpx.Response: requests.append(request) return httpx.Response(200, json={"ok": True}) - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") - await publisher.publish(RenderedMessage("Final briefing", 14), silent=True) + with caplog.at_level("INFO", logger="weather_briefing.publishers"): + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: + publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") + await publisher.publish(RenderedMessage("Final briefing", 14), silent=True) payload = json.loads(requests[0].content) assert payload["disable_notification"] is True + assert "single_message=False silent=True" in caplog.text async def test_telegram_checks_runtime_diagnostics_once_for_multiple_chunks(caplog) -> None: @@ -194,19 +200,97 @@ async def test_runtime_diagnostic_failure_does_not_block_delivery(caplog) -> Non assert "Rendered text diagnostic state check failed" in caplog.text -async def test_telegram_error_does_not_expose_token() -> None: +async def test_telegram_error_logs_safe_api_reason_without_private_response(caplog) -> None: def handler(_: httpx.Request) -> httpx.Response: - return httpx.Response(500) + return httpx.Response( + 400, + json={ + "ok": False, + "error_code": 400, + "description": "Bad Request: chat not found; private response detail", + }, + ) - async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: - publisher = TelegramPublisher(client, "private-token", "runtime-chat") - with pytest.raises(DeliveryError) as caught: - await publisher.publish(RenderedMessage("Body", 4)) + with caplog.at_level("INFO", logger="weather_briefing.publishers"): + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: + publisher = TelegramPublisher(client, "private-token", "runtime-chat") + with pytest.raises(DeliveryError, match="chat-not-found") as caught: # pragma: no branch + await publisher.publish(RenderedMessage("Body", 4)) assert "private-token" not in str(caught.value) + assert "runtime-chat" not in caplog.text + assert "private response detail" not in caplog.text + assert "Telegram delivery prepared: visible_characters=4 payload_characters=4 chunks=1" in caplog.text + assert "status_code=400 reason=chat-not-found" in caplog.text assert caught.value.__cause__ is None +async def test_telegram_failure_emits_one_warning_with_classification_at_info(caplog) -> None: + transport = httpx.MockTransport(lambda _: httpx.Response(400, json={"description": "chat not found"})) + + with caplog.at_level("INFO"): + async with LoggedAsyncClient(transport=transport) as client: + publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") + with pytest.raises(DeliveryError, match="chat-not-found"): # pragma: no branch + await publisher.publish(RenderedMessage("Body", 4)) + + warnings = [record for record in caplog.records if record.levelno == 30] + assert len(warnings) == 1 + assert warnings[0].name == "weather_briefing.api_client" + assert "status_code=400 reason=chat-not-found" in caplog.text + + +@pytest.mark.parametrize( + ("status_code", "payload", "expected_reason"), + ( + (400, {"description": "Bad Request: can't parse entities at byte offset 12"}, "invalid-html"), + (400, {"description": "Bad Request: message is too long"}, "message-too-long"), + (400, {"description": "Bad Request: not enough rights to send text messages"}, "insufficient-rights"), + (400, {"parameters": {"migrate_to_chat_id": -100123}}, "chat-migrated"), + (401, {"description": "Unauthorized"}, "bot-token-rejected"), + (404, {"description": "Not Found"}, "bot-endpoint-not-found"), + (429, {"description": "Too Many Requests: retry later"}, "rate-limited"), + (400, {"description": 123}, "api-error"), + (500, {"description": "private provider detail"}, "api-error"), + ), +) +async def test_telegram_error_classification( + status_code: int, + payload: dict[str, object], + expected_reason: str, +) -> None: + async with httpx.AsyncClient( + transport=httpx.MockTransport(lambda _: httpx.Response(status_code, json=payload)) + ) as client: + publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") + with pytest.raises(DeliveryError, match=expected_reason): + await publisher.publish(RenderedMessage("Body", 4)) + + +async def test_telegram_malformed_error_response_uses_status_classification() -> None: + async with httpx.AsyncClient( + transport=httpx.MockTransport(lambda _: httpx.Response(403, text="private provider response")) + ) as client: + publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") + with pytest.raises(DeliveryError, match="forbidden"): + await publisher.publish(RenderedMessage("Body", 4)) + + +async def test_telegram_request_error_logs_chunk_context_without_private_detail(caplog) -> None: + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("private network detail", request=request) + + with caplog.at_level("INFO", logger="weather_briefing.publishers"): + async with httpx.AsyncClient(transport=httpx.MockTransport(handler)) as client: + publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") + with pytest.raises(DeliveryError, match="request-error"): # pragma: no branch + await publisher.publish(RenderedMessage("Body", 4)) + + assert "Telegram delivery request failed index=1/1 message_visible_characters=4 payload_characters=4" in caplog.text + assert "reason=ConnectError" in caplog.text + assert "private network detail" not in caplog.text + + async def test_telegram_rejects_oversized_single_message_before_delivery() -> None: async with httpx.AsyncClient(transport=httpx.MockTransport(lambda _: httpx.Response(200))) as client: publisher = TelegramPublisher(client, "runtime-token", "runtime-chat") diff --git a/tests/test_reference_data.py b/tests/test_reference_data.py index 9a7d04b9..b77df472 100644 --- a/tests/test_reference_data.py +++ b/tests/test_reference_data.py @@ -12,6 +12,7 @@ reference_string, reference_string_tuple, reference_value, + telegram_error_classification, ) @@ -47,6 +48,9 @@ def test_packaged_reference_data_is_available() -> None: assert reference_string("provider_defaults.json", "qweather_allergen_index_type") == "7" assert localization_table("weather_document")["ja"]["forecast"] == "天気予報" assert localization_table("briefing")["zh-Hans"]["weather"] == "天气信息" + classification = telegram_error_classification() + assert ("chat not found", "chat-not-found") in classification.description_markers + assert classification.status_reasons[401] == "bot-token-rejected" def test_air_quality_guidance_covers_values_above_last_bounded_band() -> None: @@ -89,6 +93,41 @@ def test_reference_string_rejects_invalid_value(monkeypatch, value) -> None: reference_string("provider_defaults.json", "qweather_allergen_index_type") +@pytest.mark.parametrize( + ("value", "message"), + ( + ({"description_markers": {}, "status_reasons": {"400": "api-error"}}, "description markers"), + ({"description_markers": {"chat not found": "chat-not-found"}, "status_reasons": {}}, "statuses"), + ( + {"description_markers": {"CHAT NOT FOUND": "chat-not-found"}, "status_reasons": {"400": "api-error"}}, + "description markers", + ), + ( + {"description_markers": {"chat not found": "unsafe\nreason"}, "status_reasons": {"400": "api-error"}}, + "description markers", + ), + ( + {"description_markers": {"chat not found": "chat-not-found"}, "status_reasons": {"invalid": "api-error"}}, + "statuses", + ), + ( + { + "description_markers": {"chat not found": "chat-not-found"}, + "status_reasons": {"400": "api-error"}, + "unknown": {}, + }, + "supported fields", + ), + ), +) +def test_telegram_error_classification_rejects_invalid_data(monkeypatch, value, message) -> None: + monkeypatch.setattr("weather_briefing.reference_data.load_reference_data", lambda filename: value) + telegram_error_classification.cache_clear() + + with pytest.raises(ReferenceDataError, match=message): + telegram_error_classification() + + def test_load_reference_data_rejects_non_dict_root(monkeypatch) -> None: from weather_briefing.reference_data import load_reference_data diff --git a/weather_briefing/data/telegram_error_classification.json b/weather_briefing/data/telegram_error_classification.json new file mode 100644 index 00000000..7ce4ea76 --- /dev/null +++ b/weather_briefing/data/telegram_error_classification.json @@ -0,0 +1,19 @@ +{ + "description_markers": { + "chat not found": "chat-not-found", + "bot was blocked by the user": "bot-blocked", + "user is deactivated": "user-deactivated", + "not enough rights": "insufficient-rights", + "have no rights to send a message": "insufficient-rights", + "can't parse entities": "invalid-html", + "message is too long": "message-too-long", + "message text is empty": "empty-message", + "too many requests": "rate-limited" + }, + "status_reasons": { + "401": "bot-token-rejected", + "403": "forbidden", + "404": "bot-endpoint-not-found", + "429": "rate-limited" + } +} diff --git a/weather_briefing/publishers.py b/weather_briefing/publishers.py index b43b00a1..a00ad9f8 100644 --- a/weather_briefing/publishers.py +++ b/weather_briefing/publishers.py @@ -11,6 +11,7 @@ from .api_client import api_call_extensions from .models import Article, BriefingResult, RenderedMessage, SourceDocument +from .reference_data import telegram_error_classification from .render import MessageRenderer _LOGGER = logging.getLogger("weather_briefing.publishers") @@ -138,12 +139,14 @@ async def publish( if single_message and message.visible_length > self.MAX_MESSAGE_LENGTH: raise DeliveryError("Telegram single message exceeds the platform limit") chunks = (message.body,) if single_message else _split_message(message.body, self.MAX_MESSAGE_LENGTH) - _LOGGER.debug( - "Telegram delivery prepared: visible_characters=%d payload_characters=%d chunks=%d single_message=%s", + _LOGGER.info( + "Telegram delivery prepared: visible_characters=%d payload_characters=%d chunks=%d " + "single_message=%s silent=%s", message.visible_length, len(message.body), len(chunks), single_message, + silent, ) log_rendered_text = _rendered_text_logging_enabled(self._diagnostics) for index, chunk in enumerate(chunks, start=1): @@ -167,8 +170,30 @@ async def publish( extensions=api_call_extensions("telegram", "send-message"), ) response.raise_for_status() - except httpx.HTTPError: - raise DeliveryError("Telegram delivery failed") from None + except httpx.HTTPStatusError as exc: + reason = _telegram_error_reason(exc.response) + _LOGGER.info( + "Telegram delivery rejected index=%d/%d message_visible_characters=%d payload_characters=%d " + "status_code=%d reason=%s", + index, + len(chunks), + message.visible_length, + len(chunk), + exc.response.status_code, + reason, + ) + raise DeliveryError(f"Telegram delivery failed ({reason})") from None + except httpx.RequestError as exc: + _LOGGER.info( + "Telegram delivery request failed index=%d/%d message_visible_characters=%d payload_characters=%d " + "reason=%s", + index, + len(chunks), + message.visible_length, + len(chunk), + type(exc).__name__, + ) + raise DeliveryError("Telegram delivery failed (request-error)") from None _LOGGER.debug( "Telegram chunk accepted: index=%d/%d payload_characters=%d", index, @@ -197,6 +222,28 @@ def _rendered_text_logging_enabled(diagnostics: RenderedTextDiagnostics | None) return enabled and _LOGGER.isEnabledFor(logging.DEBUG) +def _telegram_error_reason(response: httpx.Response) -> str: + """Classify a Telegram API error without logging its response body.""" + try: + payload = response.json() + except ValueError: + payload = None + + if isinstance(payload, dict): + parameters = payload.get("parameters") + if isinstance(parameters, dict) and type(parameters.get("migrate_to_chat_id")) is int: + return "chat-migrated" + + description = payload.get("description") + if isinstance(description, str): + normalized = description.casefold() + for marker, reason in telegram_error_classification().description_markers: + if marker in normalized: + return reason + + return telegram_error_classification().status_reasons.get(response.status_code, "api-error") + + def _split_message(body: str, limit: int) -> tuple[str, ...]: if len(body) <= limit: return (body,) diff --git a/weather_briefing/reference_data.py b/weather_briefing/reference_data.py index 1239aa08..365d96d8 100644 --- a/weather_briefing/reference_data.py +++ b/weather_briefing/reference_data.py @@ -3,7 +3,9 @@ from __future__ import annotations import json +import re from collections.abc import Mapping +from dataclasses import dataclass from functools import cache from importlib import resources from pathlib import PurePath @@ -63,6 +65,7 @@ ), } _LOCALIZATION_LANGUAGES = frozenset({"zh-CN", "zh-TW", "en", "ja"}) +_CLASSIFICATION_REASON = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*") def _is_normalized_language(value: object) -> bool: @@ -86,6 +89,14 @@ class ReferenceDataError(RuntimeError): """Raised when packaged domain reference data is missing or malformed.""" +@dataclass(frozen=True, slots=True) +class TelegramErrorClassification: + """Validated Telegram API error-description and status mappings.""" + + description_markers: tuple[tuple[str, str], ...] + status_reasons: Mapping[int, str] + + @cache def load_reference_data(filename: str) -> dict[str, object]: """Load and validate one packaged JSON reference-data object.""" @@ -131,6 +142,49 @@ def reference_string_tuple(filename: str, *path: str) -> tuple[str, ...]: return tuple(value) +@cache +def telegram_error_classification() -> TelegramErrorClassification: + """Return validated Telegram API error classification data.""" + value = load_reference_data("telegram_error_classification.json") + markers = value.get("description_markers") + statuses = value.get("status_reasons") + if set(value) != {"description_markers", "status_reasons"}: + raise ReferenceDataError("Telegram error classification must contain the supported fields") + if not isinstance(markers, dict) or not markers: + raise ReferenceDataError("Telegram description markers must map normalized strings to reasons") + validated_markers: list[tuple[str, str]] = [] + for marker, reason in markers.items(): + if ( + not isinstance(marker, str) + or not marker.strip() + or marker != marker.casefold() + or not isinstance(reason, str) + or _CLASSIFICATION_REASON.fullmatch(reason) is None + ): + raise ReferenceDataError("Telegram description markers must map normalized strings to reasons") + validated_markers.append((marker, reason)) + + if not isinstance(statuses, dict) or not statuses: + raise ReferenceDataError("Telegram statuses must map HTTP error codes to reasons") + validated_statuses: dict[int, str] = {} + for status, reason in statuses.items(): + if ( + not isinstance(status, str) + or not status.isascii() + or not status.isdigit() + or not 400 <= int(status) <= 599 + or not isinstance(reason, str) + or _CLASSIFICATION_REASON.fullmatch(reason) is None + ): + raise ReferenceDataError("Telegram statuses must map HTTP error codes to reasons") + validated_statuses[int(status)] = reason + + return TelegramErrorClassification( + description_markers=tuple(validated_markers), + status_reasons=MappingProxyType(validated_statuses), + ) + + @cache def localization_table(name: str) -> Mapping[str, Mapping[str, str]]: """Return one fully validated localized scaffold table."""