From 46326ba96ee19e098c253e21b77f03d358c9821a Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 12 Sep 2026 03:02:03 +0300 Subject: [PATCH] =?UTF-8?q?fix(tg):=20=D0=BD=D0=B5=D0=B4=D0=BE=D1=81=D1=82?= =?UTF-8?q?=D1=83=D0=BF=D0=BD=D1=8B=D0=B9=20Telegram=20=D0=BE=D1=82=D0=B4?= =?UTF-8?q?=D0=B0=D1=91=D1=82=20502,=20=D0=B0=20=D0=BD=D0=B5=20500?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Прод 11.09.2026, 01:35 и 01:38 MSK — два 500 на glitchtip-webhook. Причина не в вебхуке: `TelegramClient._request` после исчерпания сетевых ретраев делал голый `raise`, наружу летел `httpx.ConnectTimeout`. Все три HTTP-ручки ловят `TelegramApiError` — сырой httpx пролетал мимо, и FastAPI отдавал 500 вместо задуманного 502. Отказ площадки и её недоступность для вызывающего неразличимы: переслать не смогли и там, и там. Клиент больше не выпускает наружу чужой тип. Появился общий предок `TelegramError`, под ним прежний `TelegramApiError` (ответили `ok: false`) и новый `TelegramNetworkError` (не ответили вовсе). Раздельно, а не наследником, потому что у сетевого отказа нет ни `error_code`, ни `description` — брать их неоткуда, а `bridge` по `error_code == 403` разбирает «бот заблокирован» и недоступность в этот разбор попадать не должна. Причина сохраняется в `__cause__`: в GlitchTip по-прежнему видно, таймаут это соединения или сброс TLS (#3156). Три ручки — вебхук GlitchTip и обе ручки поддержки, авторизованная и анонимная — ловят предок. Поведение воркеров не менялось: poll loop в `bridge` и так ловит `Exception`, бюджеты ретраев те же. Тесты: два в клиенте (свой тип наружу, причина не потеряна, это НЕ `TelegramApiError`), три на ручках (502 на недоступности, ничего не персистится, анонимной куки не выдаём). Четыре теста бюджета ретраев ждали `httpx.ConnectTimeout` — ждут новый тип, проверяемые паузы прежние. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01VQ8jqr4SFirX5tFLwdSrXh --- tradein-mvp/backend/app/api/v1/glitchtip.py | 7 +++- tradein-mvp/backend/app/api/v1/support.py | 9 ++-- .../backend/app/services/tgbot/client.py | 37 +++++++++++++++- .../tests/services/tgbot/test_client.py | 42 ++++++++++++++++++- .../backend/tests/test_glitchtip_webhook.py | 19 ++++++++- tradein-mvp/backend/tests/test_support.py | 40 +++++++++++++++++- .../tests/test_tgsupport_retry_budget.py | 8 ++-- 7 files changed, 148 insertions(+), 14 deletions(-) diff --git a/tradein-mvp/backend/app/api/v1/glitchtip.py b/tradein-mvp/backend/app/api/v1/glitchtip.py index bc3f9a8c..fd612af9 100644 --- a/tradein-mvp/backend/app/api/v1/glitchtip.py +++ b/tradein-mvp/backend/app/api/v1/glitchtip.py @@ -53,7 +53,7 @@ from fastapi import APIRouter, Header, HTTPException, Query, Request from pydantic import BaseModel, ConfigDict, ValidationError from app.core.config import settings -from app.services.tgbot.client import TelegramApiError, TelegramClient +from app.services.tgbot.client import TelegramClient, TelegramError logger = logging.getLogger(__name__) @@ -224,7 +224,10 @@ async def glitchtip_webhook( timeout=_INTERACTIVE_SEND_TIMEOUT_S, max_retries=_INTERACTIVE_SEND_MAX_RETRIES, ) - except TelegramApiError: + except TelegramError: + # Ловим общий предок, а не `TelegramApiError`: недоступность Telegram — + # тоже «переслать не смогли», и отвечать на неё надо задуманным 502, а не + # 500 из необработанного исключения (#3456). logger.exception("glitchtip webhook: не удалось переслать алерт в Telegram") raise HTTPException(status_code=502, detail="failed to forward alert to telegram") from None diff --git a/tradein-mvp/backend/app/api/v1/support.py b/tradein-mvp/backend/app/api/v1/support.py index 96fc7074..e98d9ec0 100644 --- a/tradein-mvp/backend/app/api/v1/support.py +++ b/tradein-mvp/backend/app/api/v1/support.py @@ -73,7 +73,7 @@ from app.core.db import get_db from app.core.ratelimit import SlidingWindowLimiter, _client_ip from app.services.tgbot import web_support_storage as storage from app.services.tgbot.bridge import SERVICE_UNAVAILABLE_TEXT -from app.services.tgbot.client import TelegramApiError, TelegramClient +from app.services.tgbot.client import TelegramClient, TelegramError logger = logging.getLogger(__name__) @@ -230,9 +230,11 @@ async def send_support_message( max_retries=_INTERACTIVE_SEND_MAX_RETRIES, max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S, ) - except TelegramApiError: + except TelegramError: # НЕ логируем payload.text (переписка — ПДн) и НЕ логируем токен (его в # TelegramApiError и не бывает — см. client.py docstring про redaction). + # Предок, а не `TelegramApiError`: при таймауте до Telegram пользователь + # должен увидеть тот же «сервис недоступен», а не 500 (#3456). logger.exception( "web support: не удалось отправить зеркало в топик (username=%s)", username ) @@ -419,8 +421,9 @@ async def send_anon_support_message( max_retries=_INTERACTIVE_SEND_MAX_RETRIES, max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S, ) - except TelegramApiError: + except TelegramError: # Ни текст сообщения (ПДн), ни токен (bearer треда) в лог не попадают. + # Про предок вместо `TelegramApiError` — см. комментарий в парной ручке. logger.exception( "web support (anon): не удалось отправить зеркало в топик (%s)", display_id ) diff --git a/tradein-mvp/backend/app/services/tgbot/client.py b/tradein-mvp/backend/app/services/tgbot/client.py index 91f6de45..50a2cacf 100644 --- a/tradein-mvp/backend/app/services/tgbot/client.py +++ b/tradein-mvp/backend/app/services/tgbot/client.py @@ -17,6 +17,10 @@ Docs: https://core.telegram.org/bots/api - Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError` (запрос некорректен или прав нет — повтор не поможет). +Наружу летит только свой тип: `TelegramApiError` (площадка ответила отказом) или +`TelegramNetworkError` (не ответила), общий предок — `TelegramError`. Сырые +httpx-исключения из клиента не выходят. + БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API, HTTP-статус и `description` из ответа Telegram — токен туда не пишем. Это НЕ гарантирует, что токен не утечёт по другим стокам: он живёт в @@ -44,7 +48,17 @@ _MAX_BACKOFF_S = 30.0 _DEFAULT_MAX_RETRIES = 5 -class TelegramApiError(Exception): +class TelegramError(Exception): + """Общий предок отказов клиента: и «ответил ok: false», и «не ответил вовсе». + + Нужен ровно затем, чтобы вызывающий мог одной строкой сказать «Telegram не + сработал» и отдать свой 502. До #3456 сетевой отказ прилетал наружу сырым + `httpx.ConnectTimeout`, мимо `except TelegramApiError`, и FastAPI отдавал + 500 — см. `TelegramNetworkError`. + """ + + +class TelegramApiError(TelegramError): """Telegram Bot API ответил `ok: false` (после исчерпания ретраев, если применимо).""" def __init__(self, method: str, error_code: int, description: str) -> None: @@ -54,6 +68,25 @@ class TelegramApiError(Exception): super().__init__(f"Telegram API {method} failed: {error_code} {description}") +class TelegramNetworkError(TelegramError): + """Ответа от Telegram не было: таймаут/обрыв, ретраи исчерпаны. + + Отдельный тип, а не `TelegramApiError`, потому что `error_code`/`description` + брать неоткуда — Telegram ничего не сказал. Вызывающие, которым важна ТОЛЬКО + реакция площадки (`bridge`, разбирающий 403 «бот заблокирован»), продолжают + ловить `TelegramApiError` и этот отказ не перехватывают. + + Причина сохраняется в `__cause__`: в GlitchTip виден исходный httpx-класс, + по которому и отличают таймаут соединения от сброса TLS (#3156). + """ + + def __init__(self, method: str, reason: str, attempts: int) -> None: + self.method = method + self.reason = reason + self.attempts = attempts + super().__init__(f"Telegram {method} unreachable after {attempts} attempts: {reason}") + + def _extract_retry_after( response: httpx.Response, default: float = _DEFAULT_RETRY_AFTER_S ) -> float: @@ -148,7 +181,7 @@ class TelegramClient: attempt, reason, ) - raise + raise TelegramNetworkError(method, reason, attempt) from exc backoff = min(2.0**attempt, backoff_cap) logger.warning( "tg client: %s — network error (попытка %d/%d): %s — retry через %.1fs", diff --git a/tradein-mvp/backend/tests/services/tgbot/test_client.py b/tradein-mvp/backend/tests/services/tgbot/test_client.py index 6a09e9ba..97d87912 100644 --- a/tradein-mvp/backend/tests/services/tgbot/test_client.py +++ b/tradein-mvp/backend/tests/services/tgbot/test_client.py @@ -16,7 +16,11 @@ import pytest os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test") -from app.services.tgbot.client import TelegramApiError, TelegramClient +from app.services.tgbot.client import ( + TelegramApiError, + TelegramClient, + TelegramNetworkError, +) _REAL_ASYNC_CLIENT = httpx.AsyncClient @@ -216,3 +220,39 @@ async def test_network_error_log_keeps_text_when_exception_has_one(caplog) -> No assert warnings, "не было предупреждения о сетевом сбое" assert "ConnectTimeout" in warnings[0], f"нет типа: {warnings[0]!r}" assert "таймаут соединения" in warnings[0], f"текст исключения потерян: {warnings[0]!r}" + + +async def test_network_exhaustion_raises_own_type_not_raw_httpx() -> None: + """Исчерпали ретраи по сети — наружу свой тип, а не `httpx.ConnectTimeout`. + + Сырой httpx пролетал мимо `except TelegramApiError` во всех трёх HTTP-ручках + и превращался в 500 вместо задуманного 502 (#3456). Тип отказа при этом + терять нельзя — он остаётся в `__cause__`, иначе в GlitchTip не отличить + таймаут соединения от сброса TLS. + """ + + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("таймаут соединения") + + _install_transport(handler) + with pytest.raises(TelegramNetworkError) as caught: + await TelegramClient(token="t").send_message(chat_id=-1, text="x", max_retries=1) + + assert caught.value.method == "sendMessage" + assert caught.value.attempts == 2, "число попыток должно попасть в исключение" + assert "ConnectTimeout" in caught.value.reason + assert isinstance(caught.value.__cause__, httpx.ConnectTimeout), "причина потеряна" + + +async def test_network_error_is_not_api_error() -> None: + """`bridge` разбирает `error_code` (403 «бот заблокирован») — недоступность + площадки в этот разбор попадать не должна, у неё кода ответа нет вовсе.""" + + def handler(request: httpx.Request) -> httpx.Response: + raise httpx.ConnectTimeout("таймаут соединения") + + _install_transport(handler) + with pytest.raises(TelegramNetworkError) as caught: + await TelegramClient(token="t").send_message(chat_id=-1, text="x", max_retries=0) + + assert not isinstance(caught.value, TelegramApiError) diff --git a/tradein-mvp/backend/tests/test_glitchtip_webhook.py b/tradein-mvp/backend/tests/test_glitchtip_webhook.py index 643b26ea..f422be94 100644 --- a/tradein-mvp/backend/tests/test_glitchtip_webhook.py +++ b/tradein-mvp/backend/tests/test_glitchtip_webhook.py @@ -23,7 +23,7 @@ from fastapi import FastAPI from fastapi.testclient import TestClient from app.api.v1 import glitchtip as glitchtip_module -from app.services.tgbot.client import TelegramApiError +from app.services.tgbot.client import TelegramApiError, TelegramNetworkError _SECRET = "test-shared-secret" _ENDPOINT = "/api/v1/trade-in/ops/glitchtip-webhook" @@ -142,6 +142,23 @@ def test_telegram_failure_returns_502_not_500( assert r.status_code != 500 +def test_telegram_unreachable_returns_502_not_500( + client: TestClient, _fake_telegram_client: Any +) -> None: + """Прод 11.09.2026, 01:35 и 01:38 MSK: два 500 на этой ручке. + + Telegram не ответил вовсе, клиент отдавал сырой `httpx.ConnectTimeout`, он + пролетал мимо `except TelegramApiError` — и вместо задуманного 502 наружу + уходил необработанный 500 (#3456). Отказ площадки и её недоступность для + отправителя алерта неразличимы: переслать не смогли и там, и там. + """ + _FakeTelegramClient._response = TelegramNetworkError("sendMessage", "ConnectTimeout", 4) + + r = client.post(f"{_ENDPOINT}?secret={_SECRET}", json=_ISSUE_PAYLOAD) + + assert r.status_code == 502, "недоступный Telegram снова отдаёт 500" + + # ── auth ───────────────────────────────────────────────────────────────────── diff --git a/tradein-mvp/backend/tests/test_support.py b/tradein-mvp/backend/tests/test_support.py index f89370ad..eb00480c 100644 --- a/tradein-mvp/backend/tests/test_support.py +++ b/tradein-mvp/backend/tests/test_support.py @@ -28,7 +28,7 @@ from fastapi.testclient import TestClient from app.api.v1 import support as support_module from app.core.db import get_db from app.core.ratelimit import SlidingWindowLimiter -from app.services.tgbot.client import TelegramApiError +from app.services.tgbot.client import TelegramApiError, TelegramNetworkError @pytest.fixture(autouse=True) @@ -295,6 +295,30 @@ def test_send_message_telegram_failure_returns_502_and_does_not_persist( assert thread_created == [] +def test_send_message_telegram_unreachable_returns_502_not_500( + client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any +) -> None: + """Пользователь не должен получить 500 из-за таймаута до Telegram (#3456). + + Отказ площадки уже отдавал задуманный 502, а вот её недоступность летела + сырым `httpx.ConnectTimeout` мимо `except TelegramApiError` — FastAPI + превращал это в 500, и в GlitchTip падала ошибка сервера вместо внятного + «сервис временно недоступен». + """ + _fake_telegram_client._response = TelegramNetworkError("sendMessage", "ConnectTimeout", 4) + record_called = [] + monkeypatch.setattr( + support_module.storage, + "record_inbound", + lambda *a, **kw: record_called.append(1), + ) + + r = client.post("/api/v1/trade-in/support/messages", json={"text": "hi"}, headers=_auth()) + + assert r.status_code == 502, "недоступный Telegram снова отдаёт 500" + assert record_called == [] + + # ── rate limit ──────────────────────────────────────────────────────────────── @@ -710,6 +734,20 @@ def test_anon_failed_send_sets_no_cookie_and_writes_nothing( assert client.cookies.get(support_module._ANON_COOKIE_NAME) is None +def test_anon_telegram_unreachable_returns_502_not_500( + client: TestClient, monkeypatch: pytest.MonkeyPatch, _fake_telegram_client: Any +) -> None: + """Тот же контракт на анонимной ручке — она открыта наружу без авторизации.""" + seen_keys = _patch_anon_storage(monkeypatch) + _fake_telegram_client._response = TelegramNetworkError("sendMessage", "ConnectTimeout", 4) + + r = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "hi"}) + + assert r.status_code == 502, "недоступный Telegram снова отдаёт 500" + assert seen_keys == [] + assert client.cookies.get(support_module._ANON_COOKIE_NAME) is None + + def test_anon_bot_not_configured_503(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(support_module.settings, "telegram_bot_token", "") r = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "hi"}) diff --git a/tradein-mvp/backend/tests/test_tgsupport_retry_budget.py b/tradein-mvp/backend/tests/test_tgsupport_retry_budget.py index d6d0c1ad..168ea4ad 100644 --- a/tradein-mvp/backend/tests/test_tgsupport_retry_budget.py +++ b/tradein-mvp/backend/tests/test_tgsupport_retry_budget.py @@ -17,7 +17,7 @@ import pytest from app.api.v1 import support as support_module from app.services.tgbot import client as client_module -from app.services.tgbot.client import TelegramApiError, TelegramClient +from app.services.tgbot.client import TelegramApiError, TelegramClient, TelegramNetworkError class _SleepSpy: @@ -85,7 +85,7 @@ async def test_max_backoff_caps_network_retry_pauses( _always_network_error(monkeypatch) tg = TelegramClient("fake-token") - with pytest.raises(httpx.ConnectTimeout): + with pytest.raises(TelegramNetworkError): await tg.send_message(chat_id=-1, text="x", max_retries=3, max_backoff=1.0) # 3 повтора → 3 паузы, каждая не выше потолка. @@ -99,7 +99,7 @@ async def test_without_max_backoff_worker_policy_is_unchanged( _always_network_error(monkeypatch) tg = TelegramClient("fake-token") - with pytest.raises(httpx.ConnectTimeout): + with pytest.raises(TelegramNetworkError): await tg.send_message(chat_id=-1, text="x", max_retries=3) assert sleep_spy.slept == [2.0, 4.0, 8.0] @@ -154,7 +154,7 @@ async def test_retry_count_is_attempts_minus_one(monkeypatch: pytest.MonkeyPatch monkeypatch.setattr(client_module.asyncio, "sleep", _SleepSpy()) tg = TelegramClient("fake-token") - with pytest.raises(httpx.ConnectTimeout): + with pytest.raises(TelegramNetworkError): await tg.send_message(chat_id=-1, text="x", max_retries=3, max_backoff=0.0) assert attempts == 4 -- 2.45.3