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: