Недоступный Telegram отдаёт 502, а не 500 #3456
7 changed files with 148 additions and 14 deletions
|
|
@ -53,7 +53,7 @@ from fastapi import APIRouter, Header, HTTPException, Query, Request
|
||||||
from pydantic import BaseModel, ConfigDict, ValidationError
|
from pydantic import BaseModel, ConfigDict, ValidationError
|
||||||
|
|
||||||
from app.core.config import settings
|
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__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
@ -224,7 +224,10 @@ async def glitchtip_webhook(
|
||||||
timeout=_INTERACTIVE_SEND_TIMEOUT_S,
|
timeout=_INTERACTIVE_SEND_TIMEOUT_S,
|
||||||
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
||||||
)
|
)
|
||||||
except TelegramApiError:
|
except TelegramError:
|
||||||
|
# Ловим общий предок, а не `TelegramApiError`: недоступность Telegram —
|
||||||
|
# тоже «переслать не смогли», и отвечать на неё надо задуманным 502, а не
|
||||||
|
# 500 из необработанного исключения (#3456).
|
||||||
logger.exception("glitchtip webhook: не удалось переслать алерт в Telegram")
|
logger.exception("glitchtip webhook: не удалось переслать алерт в Telegram")
|
||||||
raise HTTPException(status_code=502, detail="failed to forward alert to telegram") from None
|
raise HTTPException(status_code=502, detail="failed to forward alert to telegram") from None
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -73,7 +73,7 @@ from app.core.db import get_db
|
||||||
from app.core.ratelimit import SlidingWindowLimiter, _client_ip
|
from app.core.ratelimit import SlidingWindowLimiter, _client_ip
|
||||||
from app.services.tgbot import web_support_storage as storage
|
from app.services.tgbot import web_support_storage as storage
|
||||||
from app.services.tgbot.bridge import SERVICE_UNAVAILABLE_TEXT
|
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__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
@ -230,9 +230,11 @@ async def send_support_message(
|
||||||
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
||||||
max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S,
|
max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S,
|
||||||
)
|
)
|
||||||
except TelegramApiError:
|
except TelegramError:
|
||||||
# НЕ логируем payload.text (переписка — ПДн) и НЕ логируем токен (его в
|
# НЕ логируем payload.text (переписка — ПДн) и НЕ логируем токен (его в
|
||||||
# TelegramApiError и не бывает — см. client.py docstring про redaction).
|
# TelegramApiError и не бывает — см. client.py docstring про redaction).
|
||||||
|
# Предок, а не `TelegramApiError`: при таймауте до Telegram пользователь
|
||||||
|
# должен увидеть тот же «сервис недоступен», а не 500 (#3456).
|
||||||
logger.exception(
|
logger.exception(
|
||||||
"web support: не удалось отправить зеркало в топик (username=%s)", username
|
"web support: не удалось отправить зеркало в топик (username=%s)", username
|
||||||
)
|
)
|
||||||
|
|
@ -419,8 +421,9 @@ async def send_anon_support_message(
|
||||||
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
max_retries=_INTERACTIVE_SEND_MAX_RETRIES,
|
||||||
max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S,
|
max_backoff=_INTERACTIVE_SEND_MAX_BACKOFF_S,
|
||||||
)
|
)
|
||||||
except TelegramApiError:
|
except TelegramError:
|
||||||
# Ни текст сообщения (ПДн), ни токен (bearer треда) в лог не попадают.
|
# Ни текст сообщения (ПДн), ни токен (bearer треда) в лог не попадают.
|
||||||
|
# Про предок вместо `TelegramApiError` — см. комментарий в парной ручке.
|
||||||
logger.exception(
|
logger.exception(
|
||||||
"web support (anon): не удалось отправить зеркало в топик (%s)", display_id
|
"web support (anon): не удалось отправить зеркало в топик (%s)", display_id
|
||||||
)
|
)
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,10 @@ Docs: https://core.telegram.org/bots/api
|
||||||
- Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError`
|
- Любая другая 4xx (400/401/403/404) — НЕ ретраится, сразу `TelegramApiError`
|
||||||
(запрос некорректен или прав нет — повтор не поможет).
|
(запрос некорректен или прав нет — повтор не поможет).
|
||||||
|
|
||||||
|
Наружу летит только свой тип: `TelegramApiError` (площадка ответила отказом) или
|
||||||
|
`TelegramNetworkError` (не ответила), общий предок — `TelegramError`. Сырые
|
||||||
|
httpx-исключения из клиента не выходят.
|
||||||
|
|
||||||
БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API,
|
БЕЗОПАСНОСТЬ: наши `logger.*`-вызовы здесь содержат только имя метода API,
|
||||||
HTTP-статус и `description` из ответа Telegram — токен туда не пишем.
|
HTTP-статус и `description` из ответа Telegram — токен туда не пишем.
|
||||||
Это НЕ гарантирует, что токен не утечёт по другим стокам: он живёт в
|
Это НЕ гарантирует, что токен не утечёт по другим стокам: он живёт в
|
||||||
|
|
@ -44,7 +48,17 @@ _MAX_BACKOFF_S = 30.0
|
||||||
_DEFAULT_MAX_RETRIES = 5
|
_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` (после исчерпания ретраев, если применимо)."""
|
"""Telegram Bot API ответил `ok: false` (после исчерпания ретраев, если применимо)."""
|
||||||
|
|
||||||
def __init__(self, method: str, error_code: int, description: str) -> None:
|
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}")
|
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(
|
def _extract_retry_after(
|
||||||
response: httpx.Response, default: float = _DEFAULT_RETRY_AFTER_S
|
response: httpx.Response, default: float = _DEFAULT_RETRY_AFTER_S
|
||||||
) -> float:
|
) -> float:
|
||||||
|
|
@ -148,7 +181,7 @@ class TelegramClient:
|
||||||
attempt,
|
attempt,
|
||||||
reason,
|
reason,
|
||||||
)
|
)
|
||||||
raise
|
raise TelegramNetworkError(method, reason, attempt) from exc
|
||||||
backoff = min(2.0**attempt, backoff_cap)
|
backoff = min(2.0**attempt, backoff_cap)
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"tg client: %s — network error (попытка %d/%d): %s — retry через %.1fs",
|
"tg client: %s — network error (попытка %d/%d): %s — retry через %.1fs",
|
||||||
|
|
|
||||||
|
|
@ -16,7 +16,11 @@ import pytest
|
||||||
|
|
||||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
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
|
_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 warnings, "не было предупреждения о сетевом сбое"
|
||||||
assert "ConnectTimeout" in warnings[0], f"нет типа: {warnings[0]!r}"
|
assert "ConnectTimeout" in warnings[0], f"нет типа: {warnings[0]!r}"
|
||||||
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:
|
||||||
|
"""Исчерпали ретраи по сети — наружу свой тип, а не `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)
|
||||||
|
|
|
||||||
|
|
@ -23,7 +23,7 @@ from fastapi import FastAPI
|
||||||
from fastapi.testclient import TestClient
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
from app.api.v1 import glitchtip as glitchtip_module
|
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"
|
_SECRET = "test-shared-secret"
|
||||||
_ENDPOINT = "/api/v1/trade-in/ops/glitchtip-webhook"
|
_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
|
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 ─────────────────────────────────────────────────────────────────────
|
# ── auth ─────────────────────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -28,7 +28,7 @@ from fastapi.testclient import TestClient
|
||||||
from app.api.v1 import support as support_module
|
from app.api.v1 import support as support_module
|
||||||
from app.core.db import get_db
|
from app.core.db import get_db
|
||||||
from app.core.ratelimit import SlidingWindowLimiter
|
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)
|
@pytest.fixture(autouse=True)
|
||||||
|
|
@ -295,6 +295,30 @@ def test_send_message_telegram_failure_returns_502_and_does_not_persist(
|
||||||
assert thread_created == []
|
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 ────────────────────────────────────────────────────────────────
|
# ── 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
|
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:
|
def test_anon_bot_not_configured_503(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
monkeypatch.setattr(support_module.settings, "telegram_bot_token", "")
|
monkeypatch.setattr(support_module.settings, "telegram_bot_token", "")
|
||||||
r = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "hi"})
|
r = client.post("/api/v1/trade-in/support/anon/messages", json={"text": "hi"})
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,7 @@ import pytest
|
||||||
|
|
||||||
from app.api.v1 import support as support_module
|
from app.api.v1 import support as support_module
|
||||||
from app.services.tgbot import client as client_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:
|
class _SleepSpy:
|
||||||
|
|
@ -85,7 +85,7 @@ async def test_max_backoff_caps_network_retry_pauses(
|
||||||
_always_network_error(monkeypatch)
|
_always_network_error(monkeypatch)
|
||||||
tg = TelegramClient("fake-token")
|
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)
|
await tg.send_message(chat_id=-1, text="x", max_retries=3, max_backoff=1.0)
|
||||||
|
|
||||||
# 3 повтора → 3 паузы, каждая не выше потолка.
|
# 3 повтора → 3 паузы, каждая не выше потолка.
|
||||||
|
|
@ -99,7 +99,7 @@ async def test_without_max_backoff_worker_policy_is_unchanged(
|
||||||
_always_network_error(monkeypatch)
|
_always_network_error(monkeypatch)
|
||||||
tg = TelegramClient("fake-token")
|
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)
|
await tg.send_message(chat_id=-1, text="x", max_retries=3)
|
||||||
|
|
||||||
assert sleep_spy.slept == [2.0, 4.0, 8.0]
|
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())
|
monkeypatch.setattr(client_module.asyncio, "sleep", _SleepSpy())
|
||||||
|
|
||||||
tg = TelegramClient("fake-token")
|
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)
|
await tg.send_message(chat_id=-1, text="x", max_retries=3, max_backoff=0.0)
|
||||||
|
|
||||||
assert attempts == 4
|
assert attempts == 4
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue