fix(payments): обрыв соединения и Init без PaymentURL перестают запирать покупателя
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
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 5m11s
All checks were successful
CI Trade-In / changes (pull_request) Successful in 9s
CI Trade-In / browser-tests (pull_request) Has been skipped
CI Trade-In / frontend-checks (pull_request) Has been skipped
CI / changes (pull_request) Successful in 11s
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 5m11s
Три дыры в одном замке (строка без payment_url невидима для _find_live_payment, но видима предикату UNIQUE 279 → ложный 409 на 30 минут): - tbank_client ловил пару (TimeoutException, NetworkError): RemoteProtocolError, ProxyError и UnsupportedProtocol летели наружу голым httpx-типом мимо `except TBankApiError` в checkout. Ловим родителя — httpx.TransportError. - Ветка «Success:true без PaymentURL» отвечала 502, не трогая статус, — здесь банк заказ ПРИНЯЛ. Тот же терминальный статус, error_code=no_payment_url; UPDATE вынесен в общий _mark_init_failed. - _FakeDb в тестах применял статус по наличию ключа в params, а не по тексту SQL: мутант без `SET status = :status` оставался зелёным. Гейт по SQL — мутант краснит все три теста про замок. Комментарий про «возможный холд» на ветке отказа Init поправлен: Init холд не создаёт, авторизация идёт с оплаты формы, а форму покупателю не выдавали.
This commit is contained in:
parent
5bd9c586b7
commit
ec8213c269
3 changed files with 184 additions and 43 deletions
|
|
@ -203,6 +203,36 @@ _ORDER_ID_PREFIX = "mera-"
|
||||||
_REPORT_TOKEN_BYTES = 32
|
_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:
|
def _require_enabled() -> None:
|
||||||
"""Kill-switch контура. 503, а не 404: путь существует, приём оплаты выключен."""
|
"""Kill-switch контура. 503, а не 404: путь существует, приём оплаты выключен."""
|
||||||
if not settings.payments_enabled:
|
if not settings.payments_enabled:
|
||||||
|
|
@ -399,42 +429,17 @@ def checkout(
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
except TBankApiError as exc:
|
except TBankApiError as exc:
|
||||||
# Запись остаётся в БД с текстом ошибки — иначе факт попытки (и
|
# Запись остаётся в БД с текстом ошибки — иначе факт попытки (и заказа,
|
||||||
# возможного холда, если обрыв случился после приёма запроса банком) не
|
# который банк мог принять до обрыва) не остался бы нигде. Холда здесь
|
||||||
# остался бы нигде. Слепой повтор Init по тому же order_id запрещён
|
# быть не может: Init только заводит заказ и отдаёт ссылку на форму, а
|
||||||
# (см. докстринг init_payment) — это работа реконсиляции.
|
# авторизация суммы происходит, когда покупатель платит по форме — её
|
||||||
|
# ему не выдавали. Слепой повтор Init по тому же order_id всё равно
|
||||||
|
# запрещён (см. докстринг init_payment) — это работа реконсиляции.
|
||||||
#
|
#
|
||||||
# А вот статус NEW оставлять нельзя: ссылки у этой строки нет и уже не
|
# Статус — терминальный (см. `_mark_init_failed`); если банк всё же
|
||||||
# будет, поэтому `_find_live_payment` её не увидит (там
|
# пришлёт по этой строке нотификацию, статус-машина notify доведёт её до
|
||||||
# `payment_url IS NOT NULL`), но предикат UNIQUE миграции 279 — увидит,
|
# конца (терминальный статус не блокирует CONFIRMED).
|
||||||
# и следующий checkout получит конфликт вместо новой попытки. Покупатель
|
_mark_init_failed(db, order_id, code=exc.error_code, message=str(exc))
|
||||||
# оказался бы заперт на 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()
|
|
||||||
logger.warning("checkout: Init отклонён банком, order_id=%s: %s", order_id, exc)
|
logger.warning("checkout: Init отклонён банком, order_id=%s: %s", order_id, exc)
|
||||||
raise HTTPException(status_code=502, detail="payment provider error") from exc
|
raise HTTPException(status_code=502, detail="payment provider error") from exc
|
||||||
|
|
||||||
|
|
@ -442,7 +447,15 @@ def checkout(
|
||||||
tbank_payment_id = init.get("PaymentId")
|
tbank_payment_id = init.get("PaymentId")
|
||||||
if not isinstance(payment_url, str) or not payment_url:
|
if not isinstance(payment_url, str) or not payment_url:
|
||||||
# Success:true без PaymentURL — контракт банка нарушен; выдумывать
|
# 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)
|
logger.error("checkout: Init без PaymentURL, order_id=%s", order_id)
|
||||||
raise HTTPException(status_code=502, detail="payment provider returned no payment url")
|
raise HTTPException(status_code=502, detail="payment provider returned no payment url")
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -126,7 +126,14 @@ class TBankClient:
|
||||||
try:
|
try:
|
||||||
async with httpx.AsyncClient(timeout=self._timeout) as client:
|
async with httpx.AsyncClient(timeout=self._timeout) as client:
|
||||||
response = await client.post(url, json=body)
|
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:
|
if attempt > max_retries:
|
||||||
logger.error(
|
logger.error(
|
||||||
"tbank client: %s — network error после %d попыток: %s",
|
"tbank client: %s — network error после %d попыток: %s",
|
||||||
|
|
|
||||||
|
|
@ -52,6 +52,7 @@ sys.modules.setdefault("weasyprint", _wp_mock)
|
||||||
sys.modules.setdefault("weasyprint.CSS", _wp_mock)
|
sys.modules.setdefault("weasyprint.CSS", _wp_mock)
|
||||||
sys.modules.setdefault("weasyprint.HTML", _wp_mock)
|
sys.modules.setdefault("weasyprint.HTML", _wp_mock)
|
||||||
|
|
||||||
|
import httpx # noqa: E402
|
||||||
import pytest # noqa: E402
|
import pytest # noqa: E402
|
||||||
from fastapi import FastAPI # noqa: E402
|
from fastapi import FastAPI # noqa: E402
|
||||||
from fastapi.testclient import TestClient # 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:
|
if "UPDATE payments" in sql and "DEADLINE_EXPIRED" in sql:
|
||||||
return _Result(self._expire_abandoned(params))
|
return _Result(self._expire_abandoned(params))
|
||||||
if "UPDATE payments" in sql:
|
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:
|
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"]
|
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)
|
return _Result(None)
|
||||||
if "FROM trade_in_estimates" in sql and sql.startswith("SELECT"):
|
if "FROM trade_in_estimates" in sql and sql.startswith("SELECT"):
|
||||||
if params.get("id") != _ESTIMATE_UUID:
|
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 минут
|
получит конфликт и ложный 409 «retry shortly» на все 30 минут
|
||||||
`_ABANDONED_AFTER_MINUTES` — из-за чужого сбоя, а не своего действия.
|
`_ABANDONED_AFTER_MINUTES` — из-за чужого сбоя, а не своего действия.
|
||||||
|
|
||||||
Фальсификация (проверено `git apply -R`): убрать `status = :status` из
|
Фальсификация (проверено мутацией): убрать `SET status = :status` из UPDATE
|
||||||
UPDATE в ветке TBankApiError → второй checkout отвечает 409 вместо 200,
|
в `_mark_init_failed` (params оставить как есть) → второй checkout отвечает
|
||||||
новой строки и второго Init нет. Тест краснеет по значению.
|
409 вместо 200, новой строки и второго Init нет. Тест краснеет по значению.
|
||||||
|
Мутант ловится потому, что `_FakeDb` применяет статус по тексту SQL, а не по
|
||||||
|
наличию ключа `status` в params.
|
||||||
"""
|
"""
|
||||||
from app.api.v1 import payments as payments_module
|
from app.api.v1 import payments as payments_module
|
||||||
from app.api.v1.payments import _REUSABLE_STATUSES
|
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
|
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:
|
def test_init_failed_status_is_outside_the_live_predicate() -> None:
|
||||||
"""`_INIT_FAILED_STATUS` обязан быть вне предиката 279 и внутри CHECK 233.
|
"""`_INIT_FAILED_STATUS` обязан быть вне предиката 279 и внутри CHECK 233.
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue