fix(tg): ретраим весь TransportError, остальной RequestError → 502 без ретраев
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 10s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m10s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 8s
CI / changes (pull_request) Successful in 10s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / backend-tests (pull_request) Has been skipped
CI / frontend-tests (pull_request) Has been skipped
CI / openapi-codegen-check (pull_request) Has been skipped
CI Trade-In / backend-tests (pull_request) Successful in 5m10s
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.
This commit is contained in:
parent
ec245cf2b3
commit
087c48fef5
2 changed files with 102 additions and 13 deletions
|
|
@ -12,14 +12,21 @@ Docs: https://core.telegram.org/bots/api
|
||||||
Ретраи:
|
Ретраи:
|
||||||
- HTTP 429 (Too Many Requests) — уважаем `parameters.retry_after` из тела ответа
|
- HTTP 429 (Too Many Requests) — уважаем `parameters.retry_after` из тела ответа
|
||||||
(Telegram сам говорит сколько ждать), fallback на `_DEFAULT_RETRY_AFTER_S`.
|
(Telegram сам говорит сколько ждать), fallback на `_DEFAULT_RETRY_AFTER_S`.
|
||||||
- HTTP 5xx / сетевые ошибки (timeout/connect) — экспоненциальный backoff,
|
- HTTP 5xx / транспортные ошибки (`httpx.TransportError`: timeout, connect,
|
||||||
`capped` на `_MAX_BACKOFF_S`.
|
обрыв протокола, прокси) — экспоненциальный backoff, `capped` на
|
||||||
|
`_MAX_BACKOFF_S`.
|
||||||
|
- Прочие отказы запроса (`httpx.RequestError`: битый ответ) — НЕ ретряются,
|
||||||
|
сразу `TelegramNetworkError`: повтор не чинит ни испорченный ответ, ни
|
||||||
|
кривую конфигурацию.
|
||||||
- Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError`
|
- Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError`
|
||||||
(запрос некорректен или прав нет — повтор не поможет).
|
(запрос некорректен или прав нет — повтор не поможет).
|
||||||
|
|
||||||
Наружу летит только свой тип: `TelegramApiError` (площадка ответила отказом) или
|
Наружу летит только свой тип: `TelegramApiError` (площадка ответила отказом) или
|
||||||
`TelegramNetworkError` (не ответила), общий предок — `TelegramError`. Сырые
|
`TelegramNetworkError` (не ответила), общий предок — `TelegramError`. Сырые
|
||||||
httpx-исключения из клиента не выходят.
|
httpx-исключения из клиента не выходят: инвариант держат ДВА `except` в
|
||||||
|
`_request` — `httpx.TransportError` (ретраится) и страховочный
|
||||||
|
`httpx.RequestError` (не ретраится), вместе покрывающие всё дерево отказов
|
||||||
|
запроса, включая те, что появятся в httpx позже.
|
||||||
|
|
||||||
БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API,
|
БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API,
|
||||||
HTTP-статус и `description` из ответа Telegram — токен туда не пишем.
|
HTTP-статус и `description` из ответа Telegram — токен туда не пишем.
|
||||||
|
|
@ -165,7 +172,21 @@ class TelegramClient:
|
||||||
try:
|
try:
|
||||||
async with httpx.AsyncClient(timeout=effective_timeout) as client:
|
async with httpx.AsyncClient(timeout=effective_timeout) as client:
|
||||||
response = await client.post(url, json=payload)
|
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). У
|
# Тип исключения обязан попасть в строку (#3156). У
|
||||||
# httpx.ReadError и httpx.ConnectError `str(exc)` пуст, и лог
|
# httpx.ReadError и httpx.ConnectError `str(exc)` пуст, и лог
|
||||||
# выглядел так: «network error (попытка 1/3): — retry через 2s»
|
# выглядел так: «network error (попытка 1/3): — retry через 2s»
|
||||||
|
|
@ -193,6 +214,26 @@ class TelegramClient:
|
||||||
)
|
)
|
||||||
await asyncio.sleep(backoff)
|
await asyncio.sleep(backoff)
|
||||||
continue
|
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:
|
if response.status_code == 429:
|
||||||
retry_after = _extract_retry_after(response)
|
retry_after = _extract_retry_after(response)
|
||||||
|
|
|
||||||
|
|
@ -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}"
|
assert "таймаут соединения" in warnings[0], f"текст исключения потерян: {warnings[0]!r}"
|
||||||
|
|
||||||
|
|
||||||
async def test_network_exhaustion_raises_own_type_not_raw_httpx() -> None:
|
@pytest.mark.parametrize(
|
||||||
"""Исчерпали ретраи по сети — наружу свой тип, а не `httpx.ConnectTimeout`.
|
("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-ручках
|
Сырой httpx пролетал мимо `except TelegramApiError` во всех трёх HTTP-ручках
|
||||||
и превращался в 500 вместо задуманного 502 (#3456). Тип отказа при этом
|
и превращался в 500 вместо задуманного 502 (#3456). Первый заход закрыл
|
||||||
терять нельзя — он остаётся в `__cause__`, иначе в GlitchTip не отличить
|
только `(TimeoutException, NetworkError)`, а `RemoteProtocolError` («Server
|
||||||
таймаут соединения от сброса TLS.
|
disconnected without sending a response» — бытовой ответ api.telegram.org из
|
||||||
|
РФ), `ProxyError` и `DecodingError` — сёстры по `TransportError`/
|
||||||
|
`RequestError`, не наследники `NetworkError`, и дыра оставалась открытой.
|
||||||
|
|
||||||
|
Тип отказа при этом терять нельзя — он остаётся в `__cause__`, иначе в
|
||||||
|
GlitchTip не отличить таймаут соединения от сброса TLS.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def handler(request: httpx.Request) -> httpx.Response:
|
def handler(request: httpx.Request) -> httpx.Response:
|
||||||
raise httpx.ConnectTimeout("таймаут соединения")
|
raise exc_type("сбой транспорта")
|
||||||
|
|
||||||
_install_transport(handler)
|
_install_transport(handler)
|
||||||
with pytest.raises(TelegramNetworkError) as caught:
|
with pytest.raises(TelegramNetworkError) as caught:
|
||||||
await TelegramClient(token="t").send_message(chat_id=-1, text="x", max_retries=1)
|
await TelegramClient(token="t").send_message(chat_id=-1, text="x", max_retries=1)
|
||||||
|
|
||||||
assert caught.value.method == "sendMessage"
|
assert caught.value.method == "sendMessage"
|
||||||
assert caught.value.attempts == 2, "число попыток должно попасть в исключение"
|
assert caught.value.attempts == expected_attempts, "число попыток должно попасть в исключение"
|
||||||
assert "ConnectTimeout" in caught.value.reason
|
assert exc_type.__name__ in caught.value.reason
|
||||||
assert isinstance(caught.value.__cause__, httpx.ConnectTimeout), "причина потеряна"
|
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:
|
async def test_network_error_is_not_api_error() -> None:
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue