diff --git a/tradein-mvp/backend/app/api/v1/payments.py b/tradein-mvp/backend/app/api/v1/payments.py index bcd8c2be..762d21ae 100644 --- a/tradein-mvp/backend/app/api/v1/payments.py +++ b/tradein-mvp/backend/app/api/v1/payments.py @@ -203,6 +203,36 @@ _ORDER_ID_PREFIX = "mera-" _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: @@ -399,42 +429,17 @@ def checkout( ) ) except TBankApiError as exc: - # Запись остаётся в БД с текстом ошибки — иначе факт попытки (и - # возможного холда, если обрыв случился после приёма запроса банком) не - # остался бы нигде. Слепой повтор Init по тому же order_id запрещён - # (см. докстринг init_payment) — это работа реконсиляции. + # Запись остаётся в БД с текстом ошибки — иначе факт попытки (и заказа, + # который банк мог принять до обрыва) не остался бы нигде. Холда здесь + # быть не может: Init только заводит заказ и отдаёт ссылку на форму, а + # авторизация суммы происходит, когда покупатель платит по форме — её + # ему не выдавали. Слепой повтор Init по тому же order_id всё равно + # запрещён (см. докстринг init_payment) — это работа реконсиляции. # - # А вот статус NEW оставлять нельзя: ссылки у этой строки нет и уже не - # будет, поэтому `_find_live_payment` её не увидит (там - # `payment_url IS NOT NULL`), но предикат UNIQUE миграции 279 — увидит, - # и следующий checkout получит конфликт вместо новой попытки. Покупатель - # оказался бы заперт на 30 минут (_ABANDONED_AFTER_MINUTES) из-за сбоя - # на стороне банка. Переводим строку в тот же терминальный - # DEADLINE_EXPIRED, которым выше помечаются брошенные попытки: он вне - # предиката 279, поэтому пара (estimate_id, product_code) освобождается - # той же транзакцией, а «почему» лежит в error_code/error_message. - # Форма покупателю не выдавалась, так что списать по этой строке нечего; - # если банк всё же пришлёт по ней нотификацию — статус-машина notify - # доведёт её до конца (терминальный статус не блокирует CONFIRMED). - 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": exc.error_code[:64], - "message": str(exc)[:500], - "order_id": order_id, - }, - ) - db.commit() + # Статус — терминальный (см. `_mark_init_failed`); если банк всё же + # пришлёт по этой строке нотификацию, статус-машина notify доведёт её до + # конца (терминальный статус не блокирует CONFIRMED). + _mark_init_failed(db, order_id, code=exc.error_code, message=str(exc)) logger.warning("checkout: Init отклонён банком, order_id=%s: %s", order_id, exc) raise HTTPException(status_code=502, detail="payment provider error") from exc @@ -442,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") diff --git a/tradein-mvp/backend/app/services/payments/tbank_client.py b/tradein-mvp/backend/app/services/payments/tbank_client.py index e9baa367..a90f9223 100644 --- a/tradein-mvp/backend/app/services/payments/tbank_client.py +++ b/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", diff --git a/tradein-mvp/backend/tests/test_payments_router.py b/tradein-mvp/backend/tests/test_payments_router.py index aa6f0b58..6636f99d 100644 --- a/tradein-mvp/backend/tests/test_payments_router.py +++ b/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: - payment.payment_url = params["payment_url"] + if sets_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: @@ -783,9 +794,11 @@ def test_bank_refusal_on_init_does_not_lock_the_buyer_out( получит конфликт и ложный 409 «retry shortly» на все 30 минут `_ABANDONED_AFTER_MINUTES` — из-за чужого сбоя, а не своего действия. - Фальсификация (проверено `git apply -R`): убрать `status = :status` из - UPDATE в ветке TBankApiError → второй checkout отвечает 409 вместо 200, - новой строки и второго Init нет. Тест краснеет по значению. + Фальсификация (проверено мутацией): убрать `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 @@ -822,6 +835,114 @@ def test_bank_refusal_on_init_does_not_lock_the_buyer_out( 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.