Merge pull request 'fix(tradein/payments): отказ банка на Init не блокирует покупателя на 30 минут — мёртвая попытка уходит из предиката живого платежа' (#3361) from fix/3323-payment-init-fail-409 into main

tradein-mvp/backend/app/api/v1/payments.py

@@ -187,12 +187,52 @@ _ABANDONABLE_STATUSES = frozenset({"NEW", "FORM_SHOWED"})
 # списания требуют, чтобы человек намеренно оплатил обе формы.
 _ABANDONED_AFTER_MINUTES = 30
 
+# Статус для строки, чей Init отвалился: своего кода вроде INIT_FAILED в
+# CHECK миграции 233 нет, а заводить его ради этого случая значило бы менять
+# схему ради ярлыка — «почему» и так лежит в error_code/error_message. Берём
+# терминальный DEADLINE_EXPIRED, которым checkout уже помечает попытки, из
+# которых платёж не выйдет сам. Обязательное свойство ровно одно: статус ВНЕ
+# предиката 279 (= вне _REUSABLE_STATUSES), иначе мёртвая строка продолжит
+# держать пару (estimate_id, product_code). Это стережёт
+# tests/test_payments_router.py::test_init_failed_status_is_terminal.
+_INIT_FAILED_STATUS = "DEADLINE_EXPIRED"
+
 _ORDER_ID_PREFIX = "mera-"
 # 32 байта энтропии (43 символа base64url) — перебор capability-ссылки
 # неосуществим, а сама ссылка остаётся кликабельной в мессенджере.
 _REPORT_TOKEN_BYTES = 32
 
 
+def _mark_init_failed(db: Session, order_id: str, *, code: str, message: str) -> None:
+    """Переводит строку провалившегося Init в терминальный статус + «почему».
+
+    Общая для ВСЕХ исходов, после которых ссылки у строки не будет: отказ банка
+    и Success:true без PaymentURL. Статус NEW тут оставлять нельзя — см.
+    комментарий к `_INIT_FAILED_STATUS`: строка без `payment_url` невидима для
+    `_find_live_payment`, но видима предикату UNIQUE миграции 279, и следующий
+    checkout получил бы ложный 409 на все `_ABANDONED_AFTER_MINUTES`.
+    """
+    db.execute(
+        text(
+            """
+            UPDATE payments
+            SET status = :status,
+                error_code = :code,
+                error_message = :message,
+                updated_at = NOW()
+            WHERE order_id = :order_id
+            """
+        ),
+        {
+            "status": _INIT_FAILED_STATUS,
+            "code": code[:64],
+            "message": message[:500],
+            "order_id": order_id,
+        },
+    )
+    db.commit()
+
+
 def _require_enabled() -> None:
     """Kill-switch контура. 503, а не 404: путь существует, приём оплаты выключен."""
     if not settings.payments_enabled:
@@ -389,21 +429,17 @@ def checkout(
             )
         )
     except TBankApiError as exc:
-        # Запись остаётся в БД со статусом NEW и текстом ошибки — иначе факт
+        # Запись остаётся в БД с текстом ошибки — иначе факт попытки (и заказа,
-        # попытки (и возможного холда, если обрыв случился после приёма запроса
+        # который банк мог принять до обрыва) не остался бы нигде. Холда здесь
-        # банком) не остался бы нигде. Слепой повтор Init по тому же order_id
+        # быть не может: Init только заводит заказ и отдаёт ссылку на форму, а
+        # авторизация суммы происходит, когда покупатель платит по форме — её
+        # ему не выдавали. Слепой повтор Init по тому же order_id всё равно
         # запрещён (см. докстринг init_payment) — это работа реконсиляции.
-        db.execute(
+        #
-            text(
+        # Статус — терминальный (см. `_mark_init_failed`); если банк всё же
-                """
+        # пришлёт по этой строке нотификацию, статус-машина notify доведёт её до
-                UPDATE payments
+        # конца (терминальный статус не блокирует CONFIRMED).
-                SET error_code = :code, error_message = :message, updated_at = NOW()
+        _mark_init_failed(db, order_id, code=exc.error_code, message=str(exc))
-                WHERE order_id = :order_id
-                """
-            ),
-            {"code": exc.error_code[:64], "message": str(exc)[:500], "order_id": order_id},
-        )
-        db.commit()
         logger.warning("checkout: Init отклонён банком, order_id=%s: %s", order_id, exc)
         raise HTTPException(status_code=502, detail="payment provider error") from exc
 
@@ -411,7 +447,15 @@ def checkout(
     tbank_payment_id = init.get("PaymentId")
     if not isinstance(payment_url, str) or not payment_url:
         # Success:true без PaymentURL — контракт банка нарушен; выдумывать
-        # ссылку нечем.
+        # ссылку нечем. Замок тот же, что и у отказа выше, и даже вернее: банк
+        # заказ ПРИНЯЛ, а ссылки у строки уже не будет — оставить её в NEW
+        # значит отдать следующему checkout ложный 409 на 30 минут.
+        _mark_init_failed(
+            db,
+            order_id,
+            code="no_payment_url",
+            message=f"Init: Success без PaymentURL (Status={init.get('Status')!r})",
+        )
         logger.error("checkout: Init без PaymentURL, order_id=%s", order_id)
         raise HTTPException(status_code=502, detail="payment provider returned no payment url")
 

tradein-mvp/backend/app/services/payments/tbank_client.py

@@ -126,7 +126,14 @@ class TBankClient:
             try:
                 async with httpx.AsyncClient(timeout=self._timeout) as client:
                     response = await client.post(url, json=body)
-            except (httpx.TimeoutException, httpx.NetworkError) as exc:
+            # Родитель всех транспортных отказов, а не пара TimeoutException +
+            # NetworkError: RemoteProtocolError (банк оборвал ответ), ProxyError и
+            # UnsupportedProtocol мимо той пары летели наружу голым httpx-исключением.
+            # Вызывающая сторона ловит только TBankApiError, поэтому строка платежа
+            # оставалась NEW без payment_url — то есть невидимой для
+            # _find_live_payment, но видимой предикату UNIQUE миграции 279, и
+            # покупатель запирался на _ABANDONED_AFTER_MINUTES из-за сбоя банка.
+            except httpx.TransportError as exc:
                 if attempt > max_retries:
                     logger.error(
                         "tbank client: %s — network error после %d попыток: %s",

tradein-mvp/backend/tests/test_payments_router.py

@@ -52,6 +52,7 @@ sys.modules.setdefault("weasyprint", _wp_mock)
 sys.modules.setdefault("weasyprint.CSS", _wp_mock)
 sys.modules.setdefault("weasyprint.HTML", _wp_mock)
 
+import httpx  # noqa: E402
 import pytest  # noqa: E402
 from fastapi import FastAPI  # noqa: E402
 from fastapi.testclient import TestClient  # noqa: E402
@@ -163,11 +164,21 @@ class _FakeDb:
         if "UPDATE payments" in sql and "DEADLINE_EXPIRED" in sql:
             return _Result(self._expire_abandoned(params))
         if "UPDATE payments" in sql:
+            # Колонки берутся из ТЕКСТА запроса, а не из наличия ключа в params:
+            # по params фейк применял бы статус и к запросу, из которого
+            # `SET status = :status` выкинули, — мутант оставался бы зелёным.
+            sets_status = "status = :status" in sql
+            sets_url = "payment_url = :payment_url" in sql
+            sets_error = "error_code = :code" in sql
             for payment in self.payments:
-                if payment.order_id == params["order_id"] and "status" in params:
+                if payment.order_id != params["order_id"]:
+                    continue
+                if sets_status:
                     payment.status = params["status"]
-                    if "payment_url" in params:
+                if sets_url:
-                        payment.payment_url = params["payment_url"]
+                    payment.payment_url = params["payment_url"]
+                if sets_error:
+                    payment.error_code = params["code"]
             return _Result(None)
         if "FROM trade_in_estimates" in sql and sql.startswith("SELECT"):
             if params.get("id") != _ESTIMATE_UUID:
@@ -772,6 +783,193 @@ def test_abandoned_checkout_does_not_lock_the_buyer_out(
     assert len(_paid_report_rows(db)) == 2
 
 
+def test_bank_refusal_on_init_does_not_lock_the_buyer_out(
+    client: TestClient, db: _FakeDb, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Отказ банка на Init не запирает покупателя до истечения окна.
+
+    Строка после провального Init остаётся навсегда без `payment_url`:
+    `_find_live_payment` её не видит (там `payment_url IS NOT NULL`), а предикат
+    UNIQUE миграции 279 — видит. Если оставить ей статус NEW, следующий checkout
+    получит конфликт и ложный 409 «retry shortly» на все 30 минут
+    `_ABANDONED_AFTER_MINUTES` — из-за чужого сбоя, а не своего действия.
+
+    Фальсификация (проверено мутацией): убрать `SET status = :status` из UPDATE
+    в `_mark_init_failed` (params оставить как есть) → второй checkout отвечает
+    409 вместо 200, новой строки и второго Init нет. Тест краснеет по значению.
+    Мутант ловится потому, что `_FakeDb` применяет статус по тексту SQL, а не по
+    наличию ключа `status` в params.
+    """
+    from app.api.v1 import payments as payments_module
+    from app.api.v1.payments import _REUSABLE_STATUSES
+    from app.services.payments.tbank_client import TBankApiError
+
+    calls: list[dict[str, Any]] = []
+
+    class _FlakyClient:
+        async def init_payment(self, **kwargs: Any) -> dict[str, Any]:
+            calls.append(kwargs)
+            if len(calls) == 1:
+                raise TBankApiError("Init", "9999", "терминал временно недоступен")
+            return {
+                "Success": True,
+                "Status": "NEW",
+                "PaymentId": "3000000042",
+                "PaymentURL": "https://securepayments.tinkoff.ru/after-retry",
+            }
+
+    monkeypatch.setattr(payments_module, "_client", _FlakyClient)
+
+    failed = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+    assert failed.status_code == 502, failed.text
+
+    dead = _paid_report_rows(db)[0]
+    assert dead.status not in _REUSABLE_STATUSES, "мёртвая строка осталась в предикате 279"
+    assert dead.payment_url is None
+
+    retry = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+
+    assert retry.status_code == 200, retry.text
+    assert retry.json()["payment_url"] == "https://securepayments.tinkoff.ru/after-retry"
+    assert len(_paid_report_rows(db)) == 2, "повтор после отказа банка не создал новую попытку"
+    assert len(calls) == 2
+
+
+def test_broken_connection_to_bank_does_not_lock_the_buyer_out(
+    client: TestClient, db: _FakeDb, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Обрыв соединения (`RemoteProtocolError`) — тот же замок, что и отказ банка.
+
+    `_request` ловил пару (TimeoutException, NetworkError), мимо которой летят
+    RemoteProtocolError (банк оборвал ответ), ProxyError и UnsupportedProtocol.
+    Такое исключение выходило наружу голым httpx-типом, `except TBankApiError` в
+    checkout его не видел, статус строки оставался NEW без `payment_url` — и
+    следующий checkout получал ложный 409 на все 30 минут.
+
+    Фальсификация (проверено мутацией): вернуть в `tbank_client._request`
+    `except (httpx.TimeoutException, httpx.NetworkError)` → первый checkout
+    отвечает 500 вместо 502, строка остаётся NEW, повтор отвечает 409. Красное
+    по значению.
+    """
+    from app.api.v1 import payments as payments_module
+    from app.api.v1.payments import _REUSABLE_STATUSES
+
+    async def _no_sleep(_delay: float) -> None:
+        return None
+
+    async def _broken_post(*_args: Any, **_kwargs: Any) -> Any:
+        raise httpx.RemoteProtocolError("Server disconnected without sending a response")
+
+    # Ретраи `_request` спят 2+4+8 с — сон гасим, иначе тест стоит 14 секунд.
+    monkeypatch.setattr("app.services.payments.tbank_client.asyncio.sleep", _no_sleep)
+    monkeypatch.setattr(httpx.AsyncClient, "post", _broken_post)
+
+    failed = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+    assert failed.status_code == 502, failed.text
+
+    dead = _paid_report_rows(db)[0]
+    assert dead.status not in _REUSABLE_STATUSES, "оборванный Init оставил строку в предикате 279"
+    assert dead.status == "DEADLINE_EXPIRED"
+    assert dead.error_code == "network_error"
+    assert dead.payment_url is None
+
+    # Повтор идёт мимо httpx: проверяется, что пара (estimate_id, product_code)
+    # освободилась, а не то, как ведёт себя транспорт во второй раз.
+    class _WorkingClient:
+        async def init_payment(self, **_kwargs: Any) -> dict[str, Any]:
+            return {
+                "Success": True,
+                "Status": "NEW",
+                "PaymentId": "3000000043",
+                "PaymentURL": "https://securepayments.tinkoff.ru/after-disconnect",
+            }
+
+    monkeypatch.setattr(payments_module, "_client", _WorkingClient)
+
+    retry = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+
+    assert retry.status_code == 200, retry.text
+    assert retry.json()["payment_url"] == "https://securepayments.tinkoff.ru/after-disconnect"
+    assert len(_paid_report_rows(db)) == 2, "повтор после обрыва не создал новую попытку"
+
+
+def test_init_without_payment_url_does_not_lock_the_buyer_out(
+    client: TestClient, db: _FakeDb, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """`Success:true` без `PaymentURL` — замок тот же, и здесь банк заказ ПРИНЯЛ.
+
+    Ветка отвечала 502, не тронув статус: строка навсегда оставалась NEW без
+    ссылки, то есть невидимой для `_find_live_payment` и видимой предикату
+    UNIQUE миграции 279. Помечаем её тем же терминальным статусом с говорящим
+    `error_code`.
+
+    Фальсификация (проверено мутацией): убрать `_mark_init_failed` из ветки
+    «нет PaymentURL» → повтор отвечает 409 вместо 200, второго Init нет.
+    """
+    from app.api.v1 import payments as payments_module
+    from app.api.v1.payments import _REUSABLE_STATUSES
+
+    calls: list[dict[str, Any]] = []
+
+    class _UrllessClient:
+        async def init_payment(self, **kwargs: Any) -> dict[str, Any]:
+            calls.append(kwargs)
+            if len(calls) == 1:
+                # Контракт банка нарушен: Success есть, ссылки нет.
+                return {"Success": True, "Status": "NEW", "PaymentId": "3000000044"}
+            return {
+                "Success": True,
+                "Status": "NEW",
+                "PaymentId": "3000000045",
+                "PaymentURL": "https://securepayments.tinkoff.ru/after-urlless",
+            }
+
+    monkeypatch.setattr(payments_module, "_client", _UrllessClient)
+
+    failed = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+    assert failed.status_code == 502, failed.text
+
+    dead = _paid_report_rows(db)[0]
+    assert dead.status not in _REUSABLE_STATUSES, "строка без ссылки осталась в предикате 279"
+    assert dead.status == "DEADLINE_EXPIRED"
+    assert dead.error_code == "no_payment_url"
+    assert dead.payment_url is None
+
+    retry = client.post("/api/v1/trade-in/payments/checkout", json={"estimate_id": _ESTIMATE_UUID})
+
+    assert retry.status_code == 200, retry.text
+    assert retry.json()["payment_url"] == "https://securepayments.tinkoff.ru/after-urlless"
+    assert len(_paid_report_rows(db)) == 2, "повтор после пустого PaymentURL не создал попытку"
+    assert len(calls) == 2
+
+
+def test_init_failed_status_is_outside_the_live_predicate() -> None:
+    """`_INIT_FAILED_STATUS` обязан быть вне предиката 279 и внутри CHECK 233.
+
+    Сверка идёт с ТЕКСТОМ миграций, а не с константами того же модуля: статус,
+    попавший в предикат, вернул бы ложный 409, а статус вне CHECK уронил бы
+    UPDATE прямо в ветке обработки отказа банка.
+    """
+    from app.api.v1.payments import _INIT_FAILED_STATUS
+
+    sql_dir = _REPO_ROOT / "backend" / "data" / "sql"
+    predicate = re.search(
+        r"AND status IN \((.*?)\)",
+        (sql_dir / "279_payments_live_checkout_uidx.sql").read_text(encoding="utf-8"),
+        re.S,
+    )
+    assert predicate is not None
+    assert _INIT_FAILED_STATUS not in set(re.findall(r"'([^']+)'", predicate.group(1)))
+
+    check = re.search(
+        r"payments_status_check\s+CHECK \(status IN \((.*?)\)\)",
+        (sql_dir / "233_payments.sql").read_text(encoding="utf-8"),
+        re.S,
+    )
+    assert check is not None, "CHECK статусов не найден в миграции 233"
+    assert _INIT_FAILED_STATUS in set(re.findall(r"'([^']+)'", check.group(1)))
+
+
 def test_checkout_rejects_foreign_estimate(
     client: TestClient, db: _FakeDb, bank: list[dict[str, Any]], monkeypatch: pytest.MonkeyPatch
 ) -> None: