fix(payments): apply request timeout changes without restart
PAYMENT_REQUEST_TIMEOUT_SECONDS was read once in each provider's __init__ and baked into the aiohttp session, so admin overrides (applied in-process) only took effect after a container restart. Providers now hand HttpClientMixin a timeout source callable; the mixin builds the session with the current value and swaps in a fresh session when the value changes, closing the replaced one only after any in-flight request on it is bound by its own total timeout. Also: - check the Heleket payment-info success flag before reading the payload so a non-dict provider response cannot raise in the pending-payment reuse path - add PAYMENT_REQUEST_TIMEOUT_SECONDS to the FreeKassa settings stub in test_security.py (fixes three tests broken by the new field)
This commit is contained in:
@@ -156,7 +156,7 @@ class FreeKassaService(HttpClientMixin):
|
||||
self.default_currency: str = default_payment_currency_code_for_settings(settings).upper()
|
||||
|
||||
self.api_base_url: str = "https://api.fk.life/v1"
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._nonce_lock = asyncio.Lock()
|
||||
self._last_nonce = int(time.time() * 1000)
|
||||
|
||||
|
||||
@@ -246,7 +246,7 @@ class HeleketService(HttpClientMixin):
|
||||
self.referral_service = referral_service
|
||||
self._default_return_url = default_return_url
|
||||
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
if not self.configured:
|
||||
logging.warning(
|
||||
"HeleketService initialized but not fully configured. Payments disabled."
|
||||
@@ -418,8 +418,10 @@ class HeleketService(HttpClientMixin):
|
||||
return None
|
||||
|
||||
success, data = await self.get_payment_info(payment_uuid)
|
||||
if not success or not isinstance(data, dict):
|
||||
return None
|
||||
status = str(data.get("payment_status") or data.get("status") or "").lower()
|
||||
if not success or status != "check" or bool(data.get("is_final")):
|
||||
if status != "check" or bool(data.get("is_final")):
|
||||
return None
|
||||
if str(data.get("uuid") or "") != payment_uuid:
|
||||
return None
|
||||
|
||||
@@ -546,7 +546,7 @@ class PaykillaService(HttpClientMixin):
|
||||
self._exchange_rate_cache: Dict[tuple[str, str], tuple[float, Decimal]] = {}
|
||||
self._currency_cache: tuple[float, List[Dict[str, Any]]] = (0, [])
|
||||
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
if not self.configured:
|
||||
logging.warning(
|
||||
"PaykillaService initialized but not fully configured. Payments disabled."
|
||||
|
||||
@@ -158,7 +158,7 @@ class PlategaService(HttpClientMixin):
|
||||
self.referral_service = referral_service
|
||||
self._default_return_url = default_return_url
|
||||
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
if not self.configured:
|
||||
logging.warning(
|
||||
"PlategaService initialized but not fully configured. Payments disabled."
|
||||
|
||||
@@ -138,7 +138,7 @@ class SeverPayService(HttpClientMixin):
|
||||
self.referral_service = referral_service
|
||||
self._default_return_url = default_return_url
|
||||
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
|
||||
if not self.configured:
|
||||
logging.warning(
|
||||
|
||||
@@ -3,12 +3,14 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
import json
|
||||
import logging
|
||||
from typing import Any, Callable, Dict, Mapping, Optional, Tuple
|
||||
from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Tuple, Union
|
||||
|
||||
from aiohttp import ClientError, ClientSession, ClientTimeout, TraceConfig
|
||||
|
||||
SuccessCheck = Callable[[int, Any], bool]
|
||||
TimeoutSource = Union[float, Callable[[], float]]
|
||||
_TRANSPORT_ATTEMPTS = 2
|
||||
_DEFAULT_TIMEOUT_SECONDS = 20.0
|
||||
|
||||
|
||||
def http_ok(status: int, _body: Any) -> bool:
|
||||
@@ -115,25 +117,68 @@ class HttpClientMixin:
|
||||
``__init__`` and inherits ``_get_session`` / ``close``. The session is
|
||||
created on first use and recreated transparently if it was closed.
|
||||
|
||||
``total_timeout`` may be a callable so the timeout follows runtime
|
||||
settings changes (admin overrides apply in-process without a restart).
|
||||
When the value changes, the next request gets a fresh session; the old
|
||||
session stays open until its own in-flight requests cannot outlive it.
|
||||
|
||||
Provider API calls are traced so callers can retry transport failures only
|
||||
when aiohttp has not sent request headers yet.
|
||||
"""
|
||||
|
||||
_timeout: ClientTimeout
|
||||
_timeout_source: TimeoutSource
|
||||
_session: Optional[ClientSession]
|
||||
_stale_sessions: List[ClientSession]
|
||||
_session_cleanup_tasks: Set["asyncio.Task[None]"]
|
||||
|
||||
def _init_http_client(self, *, total_timeout: float = 20.0) -> None:
|
||||
self._timeout = ClientTimeout(total=total_timeout)
|
||||
def _init_http_client(self, *, total_timeout: TimeoutSource = _DEFAULT_TIMEOUT_SECONDS) -> None:
|
||||
self._timeout_source = total_timeout
|
||||
self._session = None
|
||||
self._stale_sessions = []
|
||||
self._session_cleanup_tasks = set()
|
||||
|
||||
def _current_timeout_seconds(self) -> float:
|
||||
source = self._timeout_source
|
||||
try:
|
||||
seconds = float(source() if callable(source) else source)
|
||||
except Exception:
|
||||
return _DEFAULT_TIMEOUT_SECONDS
|
||||
return seconds if seconds > 0 else _DEFAULT_TIMEOUT_SECONDS
|
||||
|
||||
async def _get_session(self) -> ClientSession:
|
||||
if self._session is None or self._session.closed:
|
||||
self._session = ClientSession(
|
||||
timeout=self._timeout,
|
||||
timeout_seconds = self._current_timeout_seconds()
|
||||
session = self._session
|
||||
if session is not None and not session.closed and session.timeout.total != timeout_seconds:
|
||||
self._session = None
|
||||
self._stale_sessions.append(session)
|
||||
task = asyncio.create_task(self._close_stale_session(session))
|
||||
self._session_cleanup_tasks.add(task)
|
||||
task.add_done_callback(self._session_cleanup_tasks.discard)
|
||||
session = None
|
||||
if session is None or session.closed:
|
||||
session = ClientSession(
|
||||
timeout=ClientTimeout(total=timeout_seconds),
|
||||
trace_configs=[_payment_trace_config()],
|
||||
)
|
||||
return self._session
|
||||
self._session = session
|
||||
return session
|
||||
|
||||
async def _close_stale_session(self, session: ClientSession) -> None:
|
||||
# Any request started on this session is bound by its total timeout,
|
||||
# so after that long it is safe to close without cutting one off.
|
||||
await asyncio.sleep((session.timeout.total or _DEFAULT_TIMEOUT_SECONDS) + 1.0)
|
||||
if session in self._stale_sessions:
|
||||
self._stale_sessions.remove(session)
|
||||
if not session.closed:
|
||||
await session.close()
|
||||
|
||||
async def close(self) -> None:
|
||||
if self._session and not self._session.closed:
|
||||
await self._session.close()
|
||||
for task in list(self._session_cleanup_tasks):
|
||||
task.cancel()
|
||||
self._session_cleanup_tasks.clear()
|
||||
sessions = [self._session, *self._stale_sessions]
|
||||
self._session = None
|
||||
self._stale_sessions = []
|
||||
for session in sessions:
|
||||
if session and not session.closed:
|
||||
await session.close()
|
||||
|
||||
@@ -204,7 +204,7 @@ class WataService(HttpClientMixin):
|
||||
self._default_return_url = default_return_url
|
||||
self._cached_public_key_pem = None # populated by webhook on first verify
|
||||
|
||||
self._init_http_client(total_timeout=self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
self._init_http_client(total_timeout=lambda: self.settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
if not self.configured:
|
||||
logging.warning("WataService initialized but not fully configured. Payments disabled.")
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import unittest
|
||||
from types import SimpleNamespace
|
||||
|
||||
from bot.payment_providers.shared.http_client import (
|
||||
HttpClientMixin,
|
||||
@@ -7,8 +8,8 @@ from bot.payment_providers.shared.http_client import (
|
||||
|
||||
|
||||
class _DummyHttpClient(HttpClientMixin):
|
||||
def __init__(self):
|
||||
self._init_http_client(total_timeout=20)
|
||||
def __init__(self, total_timeout=20):
|
||||
self._init_http_client(total_timeout=total_timeout)
|
||||
|
||||
|
||||
class PaymentHttpClientTests(unittest.IsolatedAsyncioTestCase):
|
||||
@@ -22,9 +23,33 @@ class PaymentHttpClientTests(unittest.IsolatedAsyncioTestCase):
|
||||
await client.close()
|
||||
|
||||
async def test_http_client_retries_only_before_headers_are_sent(self):
|
||||
self.assertTrue(
|
||||
_should_retry_transport_error(TimeoutError(), {"headers_sent": False})
|
||||
)
|
||||
self.assertFalse(
|
||||
_should_retry_transport_error(TimeoutError(), {"headers_sent": True})
|
||||
)
|
||||
self.assertTrue(_should_retry_transport_error(TimeoutError(), {"headers_sent": False}))
|
||||
self.assertFalse(_should_retry_transport_error(TimeoutError(), {"headers_sent": True}))
|
||||
|
||||
async def test_http_client_applies_runtime_timeout_changes(self):
|
||||
settings = SimpleNamespace(PAYMENT_REQUEST_TIMEOUT_SECONDS=20)
|
||||
client = _DummyHttpClient(total_timeout=lambda: settings.PAYMENT_REQUEST_TIMEOUT_SECONDS)
|
||||
try:
|
||||
first = await client._get_session()
|
||||
self.assertEqual(first.timeout.total, 20)
|
||||
self.assertIs(await client._get_session(), first)
|
||||
|
||||
settings.PAYMENT_REQUEST_TIMEOUT_SECONDS = 5
|
||||
second = await client._get_session()
|
||||
self.assertIsNot(second, first)
|
||||
self.assertEqual(second.timeout.total, 5)
|
||||
# The replaced session must stay usable for in-flight requests;
|
||||
# it is closed later, and close() always sweeps it up.
|
||||
self.assertFalse(first.closed)
|
||||
finally:
|
||||
await client.close()
|
||||
self.assertTrue(first.closed)
|
||||
self.assertTrue(second.closed)
|
||||
|
||||
async def test_http_client_falls_back_to_default_timeout_on_bad_source(self):
|
||||
client = _DummyHttpClient(total_timeout=lambda: None)
|
||||
try:
|
||||
session = await client._get_session()
|
||||
self.assertEqual(session.timeout.total, 20.0)
|
||||
finally:
|
||||
await client.close()
|
||||
|
||||
@@ -181,6 +181,7 @@ class FreeKassaServiceTests(unittest.TestCase):
|
||||
|
||||
settings = SimpleNamespace(
|
||||
DEFAULT_CURRENCY_SYMBOL="RUB",
|
||||
PAYMENT_REQUEST_TIMEOUT_SECONDS=15,
|
||||
trusted_proxies=["127.0.0.1"],
|
||||
)
|
||||
config = FreeKassaConfig(
|
||||
|
||||
Reference in New Issue
Block a user