Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ CLI 负责关闭自己创建的模型服务对象及其网络资源。测试或

Telegram publisher 把静默标志转换为 `disable_notification=true`。INFO 日志记录消息长度、分块数量、单条消息模式和静默投递选项,但不记录正文、Bot Token 或 Chat ID。

Telegram 拒绝请求时,publisher 记录 HTTP 状态、分块位置和安全错误类别。错误类别来自已知的 Telegram API 描述,未知响应不会原样进入日志。
Telegram 拒绝请求时,publisher 以唯一一条 WARNING 记录 HTTP 状态、分块位置和安全错误类别。错误类别来自已知的 Telegram API 描述、`parameters.migrate_to_chat_id` 字段和 HTTP 状态映射,未知响应不会原样进入日志。投递异常携带结构化错误类别;当业务消息因目标会话、Bot 身份或发送权限不可用而失败,且运维告警复用同一投递对象时,服务不再尝试通过该对象发送失败告警,只记录跳过原因

## 状态

Expand Down
24 changes: 24 additions & 0 deletions tests/test_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,30 @@ async def test_logged_client_records_http_failure_status(caplog) -> None:
assert "status_code=503" in caplog.text


async def test_logged_client_leaves_handled_response_error_warning_to_adapter(caplog) -> None:
caplog.set_level(logging.INFO, logger="weather_briefing.api_client")

async with LoggedAsyncClient(transport=httpx.MockTransport(lambda _: httpx.Response(400))) as client:
response = await client.post(
"https://example.test/api",
extensions=api_call_extensions(
"telegram",
"send-message",
response_error_handled=True,
),
)

assert response.status_code == 400
assert "API call returned handled error provider=telegram operation=send-message" in caplog.text
assert not [record for record in caplog.records if record.levelno >= logging.WARNING]


@pytest.mark.parametrize("value", (None, 0, 1, "true"))
def test_api_call_extensions_rejects_non_boolean_response_error_ownership(value) -> None:
with pytest.raises(TypeError, match="response_error_handled must be a bool"):
api_call_extensions("telegram", "send-message", response_error_handled=value)


async def test_logged_client_records_exception_type_without_message(caplog) -> None:
caplog.set_level(logging.INFO, logger="weather_briefing.api_client")

Expand Down
67 changes: 52 additions & 15 deletions tests/test_publishers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
TelegramPublisher,
_split_message,
)
from weather_briefing.reference_data import ReferenceDataError
from weather_briefing.render import PlainTextRenderer


Expand Down Expand Up @@ -68,6 +69,29 @@ def test_delivery_provider_applies_platform_limit_without_leaking_it_into_config
assert telegram_like.briefing_limit(3500) == 3500


async def test_telegram_publisher_validates_error_metadata_on_construction(monkeypatch) -> None:
def fail_validation() -> None:
raise ReferenceDataError("invalid Telegram metadata")

monkeypatch.setattr("weather_briefing.publishers.telegram_error_classification", fail_validation)

async with httpx.AsyncClient() as client:
with pytest.raises(ReferenceDataError, match="invalid Telegram metadata"):
TelegramPublisher(client, "runtime-token", "runtime-chat")


@pytest.mark.parametrize("reason", (None, 7, "private detail\nforged-log-line"))
def test_delivery_error_rejects_unsafe_structured_reason(reason) -> None:
with pytest.raises(ValueError, match="lowercase kebab-case"):
DeliveryError("Delivery failed", reason=reason)


@pytest.mark.parametrize("value", (None, 0, 1, "true"))
def test_delivery_error_rejects_non_boolean_channel_availability(value) -> None:
with pytest.raises(TypeError, match="channel_unavailable must be a bool"):
DeliveryError("Delivery failed", reason="request-error", channel_unavailable=value)


def test_split_message_prefers_line_boundary() -> None:
assert _split_message("first line\nsecond line", 12) == ("first line", "\nsecond line")

Expand Down Expand Up @@ -223,9 +247,11 @@ def handler(_: httpx.Request) -> httpx.Response:
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
assert caught.value.reason == "chat-not-found"
assert caught.value.channel_unavailable is True


async def test_telegram_failure_emits_one_warning_with_classification_at_info(caplog) -> None:
async def test_telegram_failure_emits_one_warning_with_classification(caplog) -> None:
transport = httpx.MockTransport(lambda _: httpx.Response(400, json={"description": "chat not found"}))

with caplog.at_level("INFO"):
Expand All @@ -236,36 +262,44 @@ async def test_telegram_failure_emits_one_warning_with_classification_at_info(ca

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
assert warnings[0].name == "weather_briefing.publishers"
assert warnings[0].getMessage().endswith("status_code=400 reason=chat-not-found")


@pytest.mark.parametrize(
("status_code", "payload", "expected_reason"),
("status_code", "payload", "expected_reason", "expected_channel_unavailable"),
(
(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"),
(400, {"description": "Bad Request: can't parse entities at byte offset 12"}, "invalid-html", False),
(400, {"description": "Bad Request: message is too long"}, "message-too-long", False),
(
400,
{"description": "Bad Request: not enough rights to send text messages"},
"insufficient-rights",
True,
),
(400, {"parameters": {"migrate_to_chat_id": -100123}}, "chat-migrated", True),
(401, {"description": "Unauthorized"}, "bot-token-rejected", True),
(404, {"description": "Not Found"}, "bot-endpoint-not-found", True),
(429, {"description": "Too Many Requests: retry later"}, "rate-limited", False),
(400, {"description": 123}, "api-error", False),
(500, {"description": "private provider detail"}, "api-error", False),
),
)
async def test_telegram_error_classification(
status_code: int,
payload: dict[str, object],
expected_reason: str,
expected_channel_unavailable: bool,
) -> 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):
with pytest.raises(DeliveryError, match=expected_reason) as caught:
await publisher.publish(RenderedMessage("Body", 4))

assert caught.value.channel_unavailable is expected_channel_unavailable


async def test_telegram_malformed_error_response_uses_status_classification() -> None:
async with httpx.AsyncClient(
Expand Down Expand Up @@ -294,9 +328,12 @@ def handler(request: httpx.Request) -> httpx.Response:
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")
with pytest.raises(DeliveryError, match="exceeds"):
with pytest.raises(DeliveryError, match="exceeds") as caught:
await publisher.publish(RenderedMessage("<b>short markup</b>", 4097), single_message=True)

assert caught.value.reason == "message-too-long"
assert caught.value.channel_unavailable is False


async def test_stdout_publisher_outputs_message_body(capsys) -> None:
await StdoutPublisher().publish(RenderedMessage("test body", 9))
Expand Down
54 changes: 43 additions & 11 deletions tests/test_reference_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,15 @@ def _localization_language(tables: dict[str, object], table_name: str, language:
return labels


def _telegram_classification_data() -> dict[str, object]:
return {
"description_markers": {"chat not found": "chat-not-found"},
"parameter_reasons": {"migrate_to_chat_id": "chat-migrated"},
"status_reasons": {"401": "bot-token-rejected"},
"channel_unavailable_reasons": ["chat-not-found", "chat-migrated", "bot-token-rejected"],
}


def test_packaged_reference_data_is_available() -> None:
assert reference_value("geography.json", "mainland_china_service_bounds", "latitude")
assert reference_string_tuple(
Expand Down Expand Up @@ -83,7 +92,9 @@ def test_packaged_reference_data_is_available() -> None:
assert localization_table("briefing")["zh-Hans"]["weather"] == "天气信息"
classification = telegram_error_classification()
assert ("chat not found", "chat-not-found") in classification.description_markers
assert classification.parameter_reasons["migrate_to_chat_id"] == "chat-migrated"
assert classification.status_reasons[401] == "bot-token-rejected"
assert "chat-not-found" in classification.channel_unavailable_reasons


@pytest.mark.parametrize(
Expand Down Expand Up @@ -149,27 +160,48 @@ def test_reference_string_rejects_invalid_value(monkeypatch, value) -> None:
@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"),
({}, "supported fields"),
(_telegram_classification_data() | {"description_markers": {}}, "description markers"),
(_telegram_classification_data() | {"status_reasons": {}}, "statuses"),
(
{"description_markers": {"CHAT NOT FOUND": "chat-not-found"}, "status_reasons": {"400": "api-error"}},
_telegram_classification_data() | {"description_markers": {"CHAT NOT FOUND": "chat-not-found"}},
"description markers",
),
(
{"description_markers": {"chat not found": "unsafe\nreason"}, "status_reasons": {"400": "api-error"}},
_telegram_classification_data() | {"description_markers": {"chat not found": "unsafe\nreason"}},
"description markers",
),
(
{"description_markers": {"chat not found": "chat-not-found"}, "status_reasons": {"invalid": "api-error"}},
_telegram_classification_data() | {"parameter_reasons": {}},
"parameters",
),
(
_telegram_classification_data() | {"parameter_reasons": {"migrate_to_chat_id": "unsafe\nreason"}},
"parameters",
),
(
_telegram_classification_data() | {"status_reasons": {"invalid": "api-error"}},
"statuses",
),
(
{
"description_markers": {"chat not found": "chat-not-found"},
"status_reasons": {"400": "api-error"},
"unknown": {},
},
"supported fields",
_telegram_classification_data() | {"channel_unavailable_reasons": []},
"channel availability",
),
(
_telegram_classification_data() | {"channel_unavailable_reasons": ["chat-not-found", "chat-not-found"]},
"channel availability",
),
(
_telegram_classification_data() | {"channel_unavailable_reasons": ["unknown-reason"]},
"channel availability",
),
(
_telegram_classification_data() | {"channel_unavailable_reasons": [7]},
"channel availability",
),
(
_telegram_classification_data() | {"channel_unavailable_reasons": ["unsafe\nreason"]},
"channel availability",
),
),
)
Expand Down
54 changes: 53 additions & 1 deletion tests/test_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
Warning,
WeatherContextSnapshot,
)
from weather_briefing.publishers import DeliveryProvider
from weather_briefing.publishers import DeliveryError, DeliveryProvider
from weather_briefing.render import PlainTextRenderer
from weather_briefing.service import (
BriefingService,
Expand Down Expand Up @@ -208,6 +208,26 @@ async def publish(
await super().publish(message, single_message=single_message, silent=silent)


class UnavailableChannelPublisher(RecordingPublisher):
def __init__(self) -> None:
super().__init__()
self.attempts = 0

async def publish(
self,
message: RenderedMessage,
*,
single_message: bool = False,
silent: bool = False,
) -> None:
self.attempts += 1
raise DeliveryError(
"Telegram delivery failed (chat-not-found)",
reason="chat-not-found",
channel_unavailable=True,
)


class FailingVerbatimPublisher(RecordingPublisher):
def __init__(self, failed_attempts: set[int]) -> None:
super().__init__()
Expand Down Expand Up @@ -1506,6 +1526,38 @@ async def test_task_failure_alert_delivery_failure_is_retried(
assert "Failed to publish or record briefing failure alert" in caplog.text


async def test_task_failure_alert_skips_unavailable_shared_delivery_channel(
tmp_path: Path,
caplog: pytest.LogCaptureFixture,
) -> None:
timezone = pendulum.timezone("Asia/Shanghai")
settings = _TestSettings(timezone=timezone, llm_max_attempts=1)
publisher = UnavailableChannelPublisher()
delivery = DeliveryProvider(PlainTextRenderer(), publisher)

with SQLiteStateStore(tmp_path / "unavailable-delivery.sqlite3") as state:
service = BriefingService(
settings,
_location(),
state,
EmptyRSSSource(),
EmptyContextSource(),
RecordingLLM(),
delivery,
delivery,
StaticWeatherContextProvider(),
)
with (
caplog.at_level("INFO", logger="weather_briefing.service"),
pytest.raises(DeliveryError, match="chat-not-found") as caught,
):
await service.run("briefing", pendulum.datetime(2026, 7, 13, 9, tz=timezone))

assert caught.value.reason == "chat-not-found"
assert publisher.attempts == 1
assert "Failure alert skipped reason=delivery-channel-unavailable original_reason=chat-not-found" in caplog.text


async def test_failure_recording_error_does_not_mask_task_error(
tmp_path: Path,
caplog: pytest.LogCaptureFixture,
Expand Down
26 changes: 23 additions & 3 deletions weather_briefing/api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,28 @@

_LOGGER = logging.getLogger("weather_briefing.api_client")
_API_CALL_EXTENSION = "weather_briefing.api_call"
_RESPONSE_ERROR_HANDLED_EXTENSION = "weather_briefing.response_error_handled"
_SAFE_LABEL = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*")
_CURRENT_API_CALL: ContextVar[tuple[str, str] | None] = ContextVar("current_api_call", default=None)


def api_call_extensions(provider: str, operation: str) -> dict[str, object]:
def api_call_extensions(
provider: str,
operation: str,
*,
response_error_handled: bool = False,
) -> dict[str, object]:
"""Attach non-sensitive API identity to an HTTPX request."""
if not isinstance(response_error_handled, bool):
raise TypeError("response_error_handled must be a bool")
if _SAFE_LABEL.fullmatch(provider) is None:
raise ValueError("API provider must be a lowercase kebab-case label")
if _SAFE_LABEL.fullmatch(operation) is None:
raise ValueError("API operation must be a lowercase kebab-case label")
return {_API_CALL_EXTENSION: (provider, operation)}
extensions: dict[str, object] = {_API_CALL_EXTENSION: (provider, operation)}
if response_error_handled:
extensions[_RESPONSE_ERROR_HANDLED_EXTENSION] = True
return extensions


@contextmanager
Expand Down Expand Up @@ -66,7 +77,7 @@ async def send(self, request: httpx.Request, **kwargs: Any) -> httpx.Response:
)
raise

if response.is_error:
if response.is_error and request.extensions.get(_RESPONSE_ERROR_HANDLED_EXTENSION) is not True:
_LOGGER.warning(
"API call failed provider=%s operation=%s method=%s duration_ms=%d status_code=%d",
provider,
Expand All @@ -75,6 +86,15 @@ async def send(self, request: httpx.Request, **kwargs: Any) -> httpx.Response:
_elapsed_milliseconds(started_at),
response.status_code,
)
elif response.is_error:
_LOGGER.info(
"API call returned handled error provider=%s operation=%s method=%s duration_ms=%d status_code=%d",
provider,
operation,
method,
_elapsed_milliseconds(started_at),
response.status_code,
)
else:
_LOGGER.info(
"API call succeeded provider=%s operation=%s method=%s duration_ms=%d status_code=%d",
Expand Down
Loading