From 087c48fef55a3df2babc337d99e06a03e33b2b36 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 12 Sep 2026 09:19:42 +0300 Subject: [PATCH] =?UTF-8?q?fix(tg):=20=D1=80=D0=B5=D1=82=D1=80=D0=B0=D0=B8?= =?UTF-8?q?=D0=BC=20=D0=B2=D0=B5=D1=81=D1=8C=20TransportError,=20=D0=BE?= =?UTF-8?q?=D1=81=D1=82=D0=B0=D0=BB=D1=8C=D0=BD=D0=BE=D0=B9=20RequestError?= =?UTF-8?q?=20=E2=86=92=20502=20=D0=B1=D0=B5=D0=B7=20=D1=80=D0=B5=D1=82?= =?UTF-8?q?=D1=80=D0=B0=D0=B5=D0=B2?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up к #3456. Тот PR научил три HTTP-ручки ловить общий `TelegramError` и отдавать 502, но закрыл дыру не до конца: клиент по-прежнему выпускал наружу сырой httpx. Ретраящийся `except` перехватывал узкий кортеж `(httpx.TimeoutException, httpx.NetworkError)`, а `RemoteProtocolError`, `ProxyError`, `LocalProtocolError` и `UnsupportedProtocol` — не наследники `NetworkError`, а сёстры по `TransportError`. Проверено запуском на httpx 0.28.1, не по памяти. Практическое следствие — ровно тот отказ, который #3456 и чинил. `RemoteProtocolError` («Server disconnected without sending a response») для api.telegram.org из РФ — бытовой ответ, а не экзотика. Он вылетал из `_request` сырым, проходил мимо `except TelegramError` в glitchtip.py:227 и support.py:233 и :424, и FastAPI снова отдавал 500. Глобального обработчика, который поймал бы его выше, нет: в `core/http_errors.py` зарегистрирован только `RequestValidationError`. Вдобавок такой отказ не ретраился ни разу — вылетал с первой попытки, без backoff и без строки лога о сетевом сбое, так что в проде отличить его от исчерпания бюджета было нечем. Теперь два `except`, и вместе они покрывают всё дерево отказов запроса. Ретраящийся расширен до `httpx.TransportError` — тело не тронуто, те же reason, backoff, лог и `TelegramNetworkError` из #3156. Ниже страховочный `httpx.RequestError` без ретраев: сегодня это `DecodingError`, завтра — всё, что httpx заведёт под `RequestError`. Порядок значим — `TransportError` наследник `RequestError` и обязан стоять выше, иначе сетевые отказы перестали бы ретраиться. Повторов у страховочного нет намеренно: испорченный ответ и кривую конфигурацию повтор не лечит, а пять попыток с backoff подвесили бы интерактивную ручку почти на минуту впустую. Расширение ретраев на `RemoteProtocolError` наследует уже принятый в этом клиенте риск at-least-once: запрос мог дойти до Telegram, а ответ потеряться. Риск тот же, что у давно ретраящегося `ReadTimeout`, политика не меняется. Прецедент лова именно `TransportError` в этом же репозитории — `app/services/payments/tbank_client.py:136`. Не тронуто: ручки (они уже ловят предок), `bridge.py` (`except TelegramApiError` там намеренный — разбор 403 «бот заблокирован»), `_extract_retry_after`, обработка 429/5xx, потолки backoff. Тесты: прежний тест «наружу свой тип» параметризован по `ConnectTimeout`, `RemoteProtocolError`, `ProxyError`, `DecodingError` с ожидаемым числом попыток; новый тест фиксирует разницу бюджета — обрыв протокола ретраится, битый ответ нет. Прогон по четырём затронутым файлам: 80 passed. --- .../backend/app/services/tgbot/client.py | 49 ++++++++++++-- .../tests/services/tgbot/test_client.py | 66 ++++++++++++++++--- 2 files changed, 102 insertions(+), 13 deletions(-) diff --git a/tradein-mvp/backend/app/services/tgbot/client.py b/tradein-mvp/backend/app/services/tgbot/client.py index 50a2cacf..b0e3dd16 100644 --- a/tradein-mvp/backend/app/services/tgbot/client.py +++ b/tradein-mvp/backend/app/services/tgbot/client.py @@ -12,14 +12,21 @@ Docs: https://core.telegram.org/bots/api Ретраи: - HTTP 429 (Too Many Requests) — уважаем `parameters.retry_after` из тела ответа (Telegram сам говорит сколько ждать), fallback на `_DEFAULT_RETRY_AFTER_S`. - - HTTP 5xx / сетевые ошибки (timeout/connect) — экспоненциальный backoff, - `capped` на `_MAX_BACKOFF_S`. + - HTTP 5xx / транспортные ошибки (`httpx.TransportError`: timeout, connect, + обрыв протокола, прокси) — экспоненциальный backoff, `capped` на + `_MAX_BACKOFF_S`. + - Прочие отказы запроса (`httpx.RequestError`: битый ответ) — НЕ ретряются, + сразу `TelegramNetworkError`: повтор не чинит ни испорченный ответ, ни + кривую конфигурацию. - Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError` (запрос некорректен или прав нет — повтор не поможет). Наружу летит только свой тип: `TelegramApiError` (площадка ответила отказом) или `TelegramNetworkError` (не ответила), общий предок — `TelegramError`. Сырые -httpx-исключения из клиента не выходят. +httpx-исключения из клиента не выходят: инвариант держат ДВА `except` в +`_request` — `httpx.TransportError` (ретраится) и страховочный +`httpx.RequestError` (не ретраится), вместе покрывающие всё дерево отказов +запроса, включая те, что появятся в httpx позже. БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API, HTTP-статус и `description` из ответа Telegram — токен туда не пишем. @@ -165,7 +172,21 @@ class TelegramClient: try: async with httpx.AsyncClient(timeout=effective_timeout) as client: response = await client.post(url, json=payload) - except (httpx.TimeoutException, httpx.NetworkError) as exc: + except httpx.TransportError as exc: + # Ловим ВЕСЬ `TransportError`, а не узкий кортеж + # `(TimeoutException, NetworkError)`: `RemoteProtocolError` + # («Server disconnected without sending a response» — бытовой + # ответ api.telegram.org из РФ), `ProxyError`, + # `LocalProtocolError` и `UnsupportedProtocol` — СЁСТРЫ + # `NetworkError` по `TransportError`, а не наследники. Кортеж + # оставлял дыру ровно того класса, который чинил #3456: отказ + # вылетал сырым httpx мимо `except TelegramError` в ручках и + # снова давал 500 вместо 502 — и вдобавок не ретраился ни разу. + # Расширение ретраев на `RemoteProtocolError` наследует уже + # принятый здесь риск at-least-once (запрос мог дойти до + # Telegram, потерялся ответ) — он тот же, что у давно + # ретраящегося `ReadTimeout`; политика не меняется. + # # Тип исключения обязан попасть в строку (#3156). У # httpx.ReadError и httpx.ConnectError `str(exc)` пуст, и лог # выглядел так: «network error (попытка 1/3): — retry через 2s» @@ -193,6 +214,26 @@ class TelegramClient: ) await asyncio.sleep(backoff) continue + except httpx.RequestError as exc: + # Страховка на остаток дерева отказов запроса: сегодня это + # `DecodingError` (битая компрессия в ответе), завтра — всё, что + # httpx заведёт под `RequestError`. `TooManyRedirects` сюда НЕ + # относится: клиент создаётся с дефолтным `follow_redirects=False` + # и редиректы не ходит. Порядок `except`-ов + # значим: `TransportError` — наследник `RequestError`, и стоять + # обязан ВЫШЕ, иначе сетевые отказы перестали бы ретраиться. + # + # Без ретраев намеренно: это не «площадка недоступна», а + # испорченный ответ или кривая конфигурация — повтор не лечит + # ни то, ни другое, а пять попыток с backoff подвесили бы + # интерактивную ручку почти на минуту впустую. Свой тип тут + # нужен ровно за тем же, за чем и выше: чтобы ручка увидела + # `TelegramError` и отдала 502, а не 500. + reason = f"{type(exc).__name__}: {exc}" if str(exc) else type(exc).__name__ + logger.error( + "tg client: %s — запрос не состоялся (без ретраев): %s", method, reason + ) + raise TelegramNetworkError(method, reason, attempt) from exc if response.status_code == 429: retry_after = _extract_retry_after(response) diff --git a/tradein-mvp/backend/tests/services/tgbot/test_client.py b/tradein-mvp/backend/tests/services/tgbot/test_client.py index 97d87912..908c6159 100644 --- a/tradein-mvp/backend/tests/services/tgbot/test_client.py +++ b/tradein-mvp/backend/tests/services/tgbot/test_client.py @@ -222,26 +222,74 @@ async def test_network_error_log_keeps_text_when_exception_has_one(caplog) -> No assert "таймаут соединения" in warnings[0], f"текст исключения потерян: {warnings[0]!r}" -async def test_network_exhaustion_raises_own_type_not_raw_httpx() -> None: - """Исчерпали ретраи по сети — наружу свой тип, а не `httpx.ConnectTimeout`. +@pytest.mark.parametrize( + ("exc_type", "expected_attempts"), + [ + (httpx.ConnectTimeout, 2), + (httpx.RemoteProtocolError, 2), + (httpx.ProxyError, 2), + (httpx.DecodingError, 1), + ], +) +async def test_request_failure_raises_own_type_not_raw_httpx( + exc_type: type[Exception], expected_attempts: int +) -> None: + """Любой отказ запроса — наружу свой тип, а не сырой httpx. Сырой httpx пролетал мимо `except TelegramApiError` во всех трёх HTTP-ручках - и превращался в 500 вместо задуманного 502 (#3456). Тип отказа при этом - терять нельзя — он остаётся в `__cause__`, иначе в GlitchTip не отличить - таймаут соединения от сброса TLS. + и превращался в 500 вместо задуманного 502 (#3456). Первый заход закрыл + только `(TimeoutException, NetworkError)`, а `RemoteProtocolError` («Server + disconnected without sending a response» — бытовой ответ api.telegram.org из + РФ), `ProxyError` и `DecodingError` — сёстры по `TransportError`/ + `RequestError`, не наследники `NetworkError`, и дыра оставалась открытой. + + Тип отказа при этом терять нельзя — он остаётся в `__cause__`, иначе в + GlitchTip не отличить таймаут соединения от сброса TLS. """ def handler(request: httpx.Request) -> httpx.Response: - raise httpx.ConnectTimeout("таймаут соединения") + raise exc_type("сбой транспорта") _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), "причина потеряна" + assert caught.value.attempts == expected_attempts, "число попыток должно попасть в исключение" + assert exc_type.__name__ in caught.value.reason + assert isinstance(caught.value.__cause__, exc_type), "причина потеряна" + + +async def test_remote_protocol_error_is_retried_but_decoding_error_is_not() -> None: + """Разница бюджета между двумя `except`: что чинится повтором, а что нет. + + `RemoteProtocolError` — «площадка не ответила», ровно как таймаут: повтор + осмыслен, и он наследует уже принятый здесь риск at-least-once (запрос мог + дойти до Telegram, потерялся ответ) — тот же, что у `ReadTimeout`. + `DecodingError` — испорченный ответ / кривая конфигурация: пять попыток с + backoff подвесили бы интерактивную ручку почти на минуту без единого шанса + на успех. + """ + counts: dict[str, int] = {} + + async def _attempts_for(exc_type: type[Exception]) -> int: + counts[exc_type.__name__] = 0 + + def handler(request: httpx.Request) -> httpx.Response: + counts[exc_type.__name__] += 1 + raise exc_type("сбой транспорта") + + _install_transport(handler) + with pytest.raises(TelegramNetworkError): + await TelegramClient(token="t").send_message(chat_id=-1, text="x", max_retries=2) + return counts[exc_type.__name__] + + assert await _attempts_for(httpx.RemoteProtocolError) == 3, ( + "обрыв протокола обязан ретраиться наравне с таймаутом" + ) + assert await _attempts_for(httpx.DecodingError) == 1, ( + "битый ответ ретраить нельзя — повтор не лечит, а бюджет ручки съедает" + ) async def test_network_error_is_not_api_error() -> None: