fix(payments): retry provider connect failures
This commit is contained in:
@@ -1,12 +1,19 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import asyncio
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
from typing import Any, Callable, Dict, Mapping, Optional, Tuple
|
from typing import Any, Callable, Dict, Mapping, Optional, Tuple
|
||||||
|
|
||||||
from aiohttp import ClientSession, ClientTimeout, TCPConnector
|
from aiohttp import ClientError, ClientSession, ClientTimeout, TCPConnector, TraceConfig
|
||||||
|
|
||||||
SuccessCheck = Callable[[int, Any], bool]
|
SuccessCheck = Callable[[int, Any], bool]
|
||||||
|
_TRANSPORT_ATTEMPTS = 2
|
||||||
|
_PAYMENT_REQUEST_USER_AGENT = (
|
||||||
|
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) "
|
||||||
|
"AppleWebKit/537.36 (KHTML, like Gecko) "
|
||||||
|
"Chrome/125.0.0.0 Safari/537.36"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def http_ok(status: int, _body: Any) -> bool:
|
def http_ok(status: int, _body: Any) -> bool:
|
||||||
@@ -14,6 +21,29 @@ def http_ok(status: int, _body: Any) -> bool:
|
|||||||
return status == 200
|
return status == 200
|
||||||
|
|
||||||
|
|
||||||
|
def _trace_request_ctx(trace_config_ctx: Any) -> Optional[dict]:
|
||||||
|
ctx = getattr(trace_config_ctx, "trace_request_ctx", None)
|
||||||
|
return ctx if isinstance(ctx, dict) else None
|
||||||
|
|
||||||
|
|
||||||
|
async def _mark_request_headers_sent(session, trace_config_ctx, params) -> None:
|
||||||
|
ctx = _trace_request_ctx(trace_config_ctx)
|
||||||
|
if ctx is not None:
|
||||||
|
ctx["headers_sent"] = True
|
||||||
|
|
||||||
|
|
||||||
|
def _payment_trace_config() -> TraceConfig:
|
||||||
|
trace_config = TraceConfig()
|
||||||
|
trace_config.on_request_headers_sent.append(_mark_request_headers_sent)
|
||||||
|
return trace_config
|
||||||
|
|
||||||
|
|
||||||
|
def _should_retry_transport_error(exc: Exception, trace_ctx: Mapping[str, Any]) -> bool:
|
||||||
|
if trace_ctx.get("headers_sent"):
|
||||||
|
return False
|
||||||
|
return isinstance(exc, (asyncio.TimeoutError, ClientError, OSError))
|
||||||
|
|
||||||
|
|
||||||
async def post_json_request(
|
async def post_json_request(
|
||||||
session: ClientSession,
|
session: ClientSession,
|
||||||
url: str,
|
url: str,
|
||||||
@@ -29,34 +59,47 @@ async def post_json_request(
|
|||||||
returns ``(False, {"status": ..., "message": ..., "raw": ...?})`` so callers
|
returns ``(False, {"status": ..., "message": ..., "raw": ...?})`` so callers
|
||||||
can decide what to do (typically: mark the payment as ``failed_creation``).
|
can decide what to do (typically: mark the payment as ``failed_creation``).
|
||||||
"""
|
"""
|
||||||
try:
|
for attempt in range(1, _TRANSPORT_ATTEMPTS + 1):
|
||||||
async with session.post(
|
trace_ctx: dict[str, Any] = {"headers_sent": False}
|
||||||
url,
|
try:
|
||||||
json=body,
|
async with session.post(
|
||||||
headers=dict(headers) if headers else None,
|
url,
|
||||||
) as response:
|
json=body,
|
||||||
response_text = await response.text()
|
headers=dict(headers) if headers else None,
|
||||||
try:
|
trace_request_ctx=trace_ctx,
|
||||||
response_data = json.loads(response_text) if response_text else {}
|
) as response:
|
||||||
except json.JSONDecodeError:
|
response_text = await response.text()
|
||||||
logging.error("%s: invalid JSON response: %s", log_prefix, response_text)
|
try:
|
||||||
return False, {
|
response_data = json.loads(response_text) if response_text else {}
|
||||||
"status": response.status,
|
except json.JSONDecodeError:
|
||||||
"message": "invalid_json",
|
logging.error("%s: invalid JSON response: %s", log_prefix, response_text)
|
||||||
"raw": response_text,
|
return False, {
|
||||||
}
|
"status": response.status,
|
||||||
if not is_success(response.status, response_data):
|
"message": "invalid_json",
|
||||||
logging.error(
|
"raw": response_text,
|
||||||
"%s: API returned error (status=%s, body=%s)",
|
}
|
||||||
|
if not is_success(response.status, response_data):
|
||||||
|
logging.error(
|
||||||
|
"%s: API returned error (status=%s, body=%s)",
|
||||||
|
log_prefix,
|
||||||
|
response.status,
|
||||||
|
response_data,
|
||||||
|
)
|
||||||
|
return False, {"status": response.status, "message": response_data}
|
||||||
|
return True, response_data
|
||||||
|
except Exception as exc:
|
||||||
|
if attempt < _TRANSPORT_ATTEMPTS and _should_retry_transport_error(exc, trace_ctx):
|
||||||
|
logging.warning(
|
||||||
|
"%s: transport failed before request headers were sent; retrying (%s/%s): %s", # noqa: E501
|
||||||
log_prefix,
|
log_prefix,
|
||||||
response.status,
|
attempt + 1,
|
||||||
response_data,
|
_TRANSPORT_ATTEMPTS,
|
||||||
|
exc,
|
||||||
)
|
)
|
||||||
return False, {"status": response.status, "message": response_data}
|
continue
|
||||||
return True, response_data
|
logging.exception("%s: request failed.", log_prefix)
|
||||||
except Exception as exc:
|
return False, {"message": str(exc)}
|
||||||
logging.exception("%s: request failed.", log_prefix)
|
return False, {"message": "request_failed"}
|
||||||
return False, {"message": str(exc)}
|
|
||||||
|
|
||||||
|
|
||||||
def first_value(data: Optional[Mapping[str, Any]], *keys: str) -> Optional[str]:
|
def first_value(data: Optional[Mapping[str, Any]], *keys: str) -> Optional[str]:
|
||||||
@@ -94,7 +137,12 @@ class HttpClientMixin:
|
|||||||
async def _get_session(self) -> ClientSession:
|
async def _get_session(self) -> ClientSession:
|
||||||
if self._session is None or self._session.closed:
|
if self._session is None or self._session.closed:
|
||||||
connector = TCPConnector(force_close=self._connector_force_close)
|
connector = TCPConnector(force_close=self._connector_force_close)
|
||||||
self._session = ClientSession(timeout=self._timeout, connector=connector)
|
self._session = ClientSession(
|
||||||
|
timeout=self._timeout,
|
||||||
|
connector=connector,
|
||||||
|
headers={"User-Agent": _PAYMENT_REQUEST_USER_AGENT},
|
||||||
|
trace_configs=[_payment_trace_config()],
|
||||||
|
)
|
||||||
return self._session
|
return self._session
|
||||||
|
|
||||||
async def close(self) -> None:
|
async def close(self) -> None:
|
||||||
|
|||||||
@@ -1,6 +1,10 @@
|
|||||||
import unittest
|
import unittest
|
||||||
|
|
||||||
from bot.payment_providers.shared.http_client import HttpClientMixin
|
from bot.payment_providers.shared.http_client import (
|
||||||
|
HttpClientMixin,
|
||||||
|
_PAYMENT_REQUEST_USER_AGENT,
|
||||||
|
_should_retry_transport_error,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class _DummyHttpClient(HttpClientMixin):
|
class _DummyHttpClient(HttpClientMixin):
|
||||||
@@ -14,5 +18,15 @@ class PaymentHttpClientTests(unittest.IsolatedAsyncioTestCase):
|
|||||||
try:
|
try:
|
||||||
session = await client._get_session()
|
session = await client._get_session()
|
||||||
self.assertTrue(session.connector.force_close)
|
self.assertTrue(session.connector.force_close)
|
||||||
|
self.assertTrue(session.trace_configs)
|
||||||
|
self.assertEqual(session.headers["User-Agent"], _PAYMENT_REQUEST_USER_AGENT)
|
||||||
finally:
|
finally:
|
||||||
await client.close()
|
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})
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user