-
Notifications
You must be signed in to change notification settings - Fork 9
Chore: align sync/async clients, update stk_push tests and add 100-call STK push stability tests #127
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Chore: align sync/async clients, update stk_push tests and add 100-call STK push stability tests #127
Changes from all commits
da3adfb
139847c
33cea54
b0dac3e
0a4ef72
bbe169f
7e38506
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,98 @@ | ||
| """MpesaAsyncHttpClient: An asynchronous client for making HTTP requests to the M-Pesa API.""" | ||
|
|
||
| from typing import Dict, Any, Optional | ||
| from urllib import response | ||
| import httpx | ||
| import logging | ||
|
|
||
| from mpesakit.errors import MpesaError, MpesaApiException | ||
| from .http_client import AsyncHttpClient | ||
| from urllib.parse import urljoin | ||
|
|
||
| from tenacity import ( | ||
| AsyncRetrying, | ||
| RetryCallState, | ||
| before_sleep_log, | ||
| retry_if_exception_type, | ||
| stop_after_attempt, | ||
| wait_random_exponential, | ||
| ) | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def handle_request_error(response: httpx.Response): | ||
| """Handles non-successful HTTP responses. | ||
|
|
||
| This function is now responsible for converting HTTP status codes | ||
| and JSON parsing errors into MpesaApiException. | ||
| """ | ||
| if response.is_success: | ||
| return | ||
| try: | ||
| response_data = response.json() | ||
| except ValueError: | ||
| response_data = {"errorMessage": response.text.strip() or ""} | ||
|
|
||
| error_message = response_data.get("errorMessage", "") | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code=f"HTTP_{response.status_code}", | ||
| error_message=error_message, | ||
| status_code=response.status_code, | ||
| raw_response=response.text, | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def handle_retry_exception(retry_state: RetryCallState): | ||
| """Custom hook to handle exceptions after all retries fail. | ||
|
|
||
| It raises a custom MpesaApiException with the appropriate error code. | ||
| """ | ||
| if retry_state.outcome: | ||
| exception = retry_state.outcome.exception() | ||
|
|
||
| if isinstance(exception, httpx.TimeoutException): | ||
| raise MpesaApiException( | ||
| MpesaError(error_code="REQUEST_TIMEOUT", error_message=str(exception)) | ||
| ) from exception | ||
| elif isinstance(exception, httpx.ConnectError): | ||
| raise MpesaApiException( | ||
| MpesaError(error_code="CONNECTION_ERROR", error_message="Failed to connect to M-Pesa API.") | ||
| ) from exception | ||
|
|
||
| raise MpesaApiException( | ||
| MpesaError(error_code="REQUEST_FAILED", error_message=str(exception)) | ||
| ) from exception | ||
|
|
||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="REQUEST_FAILED", | ||
| error_message="An unknown retry error occurred.", | ||
| ) | ||
| ) | ||
|
|
||
|
|
||
| def retry_enabled(enabled: bool): | ||
| """Factory function to conditionally enable retries. | ||
|
|
||
| Args: | ||
| enabled (bool): Whether to enable retry logic. | ||
|
|
||
| Returns: | ||
| A retry condition function. | ||
| """ | ||
| base_retry = retry_if_exception_type( | ||
| httpx.TimeoutException | ||
| ) | retry_if_exception_type(httpx.ConnectError) | ||
|
|
||
| def _retry(retry_state): | ||
| if not enabled: | ||
| return False | ||
| return base_retry(retry_state) | ||
|
|
||
| return _retry | ||
|
|
||
|
|
||
| class MpesaAsyncHttpClient(AsyncHttpClient): | ||
|
|
@@ -20,140 +108,150 @@ class MpesaAsyncHttpClient(AsyncHttpClient): | |
| base_url: str | ||
| _client: httpx.AsyncClient | ||
|
|
||
| def __init__(self, env: str = "sandbox"): | ||
| def __init__(self, env: str = "sandbox", retry_enabled_flag: bool = True, wait_strategy=None, stop_strategy=None): | ||
| """Initializes the MpesaAsyncHttpClient with the specified environment.""" | ||
| self.base_url = self._resolve_base_url(env) | ||
| self._client = httpx.AsyncClient(base_url=self.base_url) | ||
|
|
||
| self._retry_enabled_flag = retry_enabled_flag | ||
| self._wait_strategy = wait_strategy or wait_random_exponential(multiplier=5, max=8) | ||
| self._stop_strategy = stop_strategy or stop_after_attempt(3) | ||
|
|
||
| def _resolve_base_url(self, env: str) -> str: | ||
| if env.lower() == "production": | ||
| return "https://api.safaricom.co.ke" | ||
| return "https://sandbox.safaricom.co.ke" | ||
|
|
||
| def _build_retrying(self): | ||
| return AsyncRetrying( | ||
| retry=retry_enabled(enabled=self._retry_enabled_flag), | ||
| wait=self._wait_strategy, | ||
| stop=self._stop_strategy, | ||
| retry_error_callback=handle_retry_exception, | ||
| before_sleep=before_sleep_log(logger, logging.WARNING), | ||
| reraise=True, | ||
| ) | ||
|
Comment on lines
+125
to
+133
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Find project dependency files to identify tenacity version
find . -type f \( -name "requirements.txt" -o -name "setup.py" -o -name "pyproject.toml" -o -name "setup.cfg" \) | head -20Repository: Byte-Barn/mpesakit Length of output: 78 🏁 Script executed: # Read the specific file being reviewed
head -150 mpesakit/http_client/mpesa_async_http_client.py | tail -50Repository: Byte-Barn/mpesakit Length of output: 1975 🏁 Script executed: # Get the full context of the _build_retrying method and surrounding code
sed -n '110,145p' mpesakit/http_client/mpesa_async_http_client.pyRepository: Byte-Barn/mpesakit Length of output: 1545 🌐 Web query:
💡 Result: In tenacity, when retries are exhausted (i.e., the stop condition is met),
So the effective precedence is:
Sources: [1] [2] 🏁 Script executed: # Find where handle_retry_exception is defined
rg "def handle_retry_exception" --type py -A 10Repository: Byte-Barn/mpesakit Length of output: 1988 🏁 Script executed: # Check imports in the file to understand where handle_retry_exception comes from
head -30 mpesakit/http_client/mpesa_async_http_client.pyRepository: Byte-Barn/mpesakit Length of output: 851 🏁 Script executed: # Search for handle_retry_exception usage and definition across the codebase
rg "handle_retry_exception" --type py -B 2 -A 5Repository: Byte-Barn/mpesakit Length of output: 3175 Remove Tenacity's The synchronous counterpart in 🤖 Prompt for AI Agents |
||
|
|
||
| async def __aenter__(self): | ||
| return self | ||
|
|
||
| async def __aexit__(self, exc_type, exc_val, exc_tb): | ||
| await self._client.aclose() | ||
|
|
||
| async def async_raw_post( | ||
| self, url: str, json: Dict[str, Any], headers: Dict[str, str], timeout: int = 10 | ||
| ) -> httpx.Response: | ||
| """Low-level POST request - may raise httpx exceptions.""" | ||
| full_url = urljoin(self.base_url, url) | ||
|
|
||
| async def post( | ||
| self, url: str, json: Dict[str, Any], headers: Dict[str, str] | ||
| ) -> Dict[str, Any]: | ||
| """Sends an asynchronous POST request to the M-Pesa API.""" | ||
| try: | ||
| async for attempt in self._build_retrying(): | ||
| with attempt: | ||
| response = await self._client.post( | ||
| full_url, json=json, headers=headers, timeout=timeout | ||
| ) | ||
| break | ||
|
|
||
| if response is None: | ||
| raise RuntimeError("Retry loop exited without returning a response or raising an exception.") | ||
|
|
||
| return response | ||
|
|
||
|
Comment on lines
+137
to
+154
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pipeline failure: Mypy After the 🛠 Proposed fix async def async_raw_post(
self, url: str, json: Dict[str, Any], headers: Dict[str, str], timeout: int = 10
) -> httpx.Response:
"""Low-level POST request - may raise httpx exceptions."""
full_url = urljoin(self.base_url, url)
async for attempt in self._build_retrying():
with attempt:
return await self._client.post(
full_url, json=json, headers=headers, timeout=timeout
)
+
+ raise RuntimeError("async_raw_post: unreachable – retry_error_callback should have raised")🧰 Tools🪛 GitHub Actions: Code Quality[error] 136-136: Mypy: Missing return statement. [return] 🤖 Prompt for AI Agents |
||
| response = await self._client.post( | ||
| url, json=json, headers=headers, timeout=10 | ||
| ) | ||
|
|
||
|
|
||
| try: | ||
| response_data = response.json() | ||
| except ValueError: | ||
| response_data = {"errorMessage": response.text.strip() or ""} | ||
|
|
||
| if not response.is_success: | ||
| error_message = response_data.get("errorMessage", "") | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code=f"HTTP_{response.status_code}", | ||
| error_message=error_message, | ||
| status_code=response.status_code, | ||
| raw_response=response_data, | ||
| ) | ||
| ) | ||
| async def post( | ||
| self, url: str, json: Dict[str, Any], headers: Dict[str, str] | ||
| ) -> Dict[str, Any]: | ||
| """Sends a POST request to the M-Pesa API. | ||
|
|
||
| return response_data | ||
| Args: | ||
| url (str): The URL path for the request. | ||
| json (Dict[str, Any]): The JSON payload for the request body. | ||
| headers (Dict[str, str]): The HTTP headers for the request. | ||
| timeout (int): The timeout for the request in seconds. | ||
|
|
||
| except httpx.TimeoutException: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="REQUEST_TIMEOUT", | ||
| error_message="Request to Mpesa timed out.", | ||
| status_code=None, | ||
| ) | ||
| ) | ||
| except httpx.ConnectError: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="CONNECTION_ERROR", | ||
| error_message="Failed to connect to Mpesa API. Check network or URL.", | ||
| status_code=None, | ||
| ) | ||
| Returns: | ||
| Dict[str, Any]: The JSON response from the API. | ||
| """ | ||
| response: httpx.Response | None = None | ||
| try: | ||
| response = await self.async_raw_post( | ||
| url, json=json, headers=headers, timeout=10 | ||
| ) | ||
| except httpx.HTTPError as e: | ||
| handle_request_error(response) | ||
| return response.json() | ||
|
|
||
| except (httpx.RequestError, ValueError) as e: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="REQUEST_FAILED", | ||
| error_message=f"HTTP request failed: {str(e)}", | ||
| status_code=None, | ||
| raw_response=None, | ||
| error_message="HTTP request failed.", | ||
| status_code=getattr(response, "status_code", None), | ||
| raw_response=getattr(response, "text", None), | ||
| ) | ||
| ) | ||
| ) from e | ||
|
Comment on lines
+179
to
+187
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When The same issue affects 🛠 Proposed fix- except (httpx.RequestError, ValueError) as e:
+ except httpx.TimeoutException as e:
raise MpesaApiException(
MpesaError(
- error_code="REQUEST_FAILED",
- error_message="HTTP request failed.",
+ error_code="REQUEST_TIMEOUT",
+ error_message=str(e),
status_code=getattr(response, "status_code", None),
raw_response=getattr(response, "text", None),
)
) from e
+ except httpx.ConnectError as e:
+ raise MpesaApiException(
+ MpesaError(
+ error_code="CONNECTION_ERROR",
+ error_message="Failed to connect to M-Pesa API.",
+ status_code=getattr(response, "status_code", None),
+ raw_response=getattr(response, "text", None),
+ )
+ ) from e
+ except (httpx.RequestError, ValueError) as e:
+ raise MpesaApiException(
+ MpesaError(
+ error_code="REQUEST_FAILED",
+ error_message="HTTP request failed.",
+ status_code=getattr(response, "status_code", None),
+ raw_response=getattr(response, "text", None),
+ )
+ ) from e🤖 Prompt for AI Agents |
||
|
|
||
| async def get( | ||
|
|
||
| async def async_raw_get( | ||
| self, | ||
| url: str, | ||
| params: Optional[Dict[str, Any]] = None, | ||
| headers: Optional[Dict[str, str]] = None, | ||
| ) -> Dict[str, Any]: | ||
| """Sends an asynchronous GET request to the M-Pesa API.""" | ||
| try: | ||
| if headers is None: | ||
| headers = {} | ||
|
|
||
| response = await self._client.get( | ||
| url, params=params, headers=headers, timeout=10 | ||
| ) | ||
| timeout: int = 10, | ||
| ) -> httpx.Response: | ||
| """Low-level GET request - may raise httpx exceptions.""" | ||
| if headers is None: | ||
| headers = {} | ||
| full_url = urljoin(self.base_url, url) | ||
|
|
||
| try: | ||
| response_data = response.json() | ||
| except ValueError: | ||
| response_data = {"errorMessage": response.text.strip() or ""} | ||
|
|
||
| if not response.is_success: | ||
| error_message = response_data.get("errorMessage", "") | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code=f"HTTP_{response.status_code}", | ||
| error_message=error_message, | ||
| status_code=response.status_code, | ||
| raw_response=response_data, | ||
| ) | ||
| async for attempt in self._build_retrying(): | ||
| with attempt: | ||
| response = await self._client.get( | ||
| full_url, params=params, headers=headers, timeout=timeout | ||
| ) | ||
|
|
||
| if response is None: | ||
| raise RuntimeError("Retry loop exited without returning a response or raising an exception.") | ||
|
|
||
| return response | ||
|
|
||
| return response_data | ||
|
|
||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| except httpx.TimeoutException: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="REQUEST_TIMEOUT", | ||
| error_message="Request to Mpesa timed out.", | ||
| status_code=None, | ||
| ) | ||
| ) | ||
| except httpx.ConnectError: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="CONNECTION_ERROR", | ||
| error_message="Failed to connect to Mpesa API. Check network or URL.", | ||
| status_code=None, | ||
| ) | ||
| ) | ||
| except httpx.HTTPError as e: | ||
| async def get( | ||
| self, | ||
| url: str, | ||
| params: Optional[Dict[str, Any]] = None, | ||
| headers: Optional[Dict[str, str]] = None, | ||
| ) -> Dict[str, Any]: | ||
| """Sends a GET request to the M-Pesa API. | ||
|
|
||
| Args: | ||
| url (str): The URL path for the request. | ||
| params (Optional[Dict[str, Any]]): The URL parameters. | ||
| headers (Optional[Dict[str, str]]): The HTTP headers. | ||
| timeout (int): The timeout for the request in seconds. | ||
|
|
||
| Returns: | ||
| Dict[str, Any]: The JSON response from the API. | ||
| """ | ||
| response: httpx.Response | None = None | ||
| try: | ||
| response = await self.async_raw_get(url, params, headers, timeout = 10) | ||
| handle_request_error(response) | ||
| return response.json() | ||
| except (httpx.RequestError, ValueError) as e: | ||
| raise MpesaApiException( | ||
| MpesaError( | ||
| error_code="REQUEST_FAILED", | ||
| error_message=f"HTTP request failed: {str(e)}", | ||
| status_code=None, | ||
| raw_response=None, | ||
| error_message="HTTP request failed.", | ||
| status_code=getattr(response, "status_code", None), | ||
| raw_response=getattr(response, "text", None), | ||
| ) | ||
| ) | ||
| ) from e | ||
|
|
||
| async def aclose(self): | ||
| """Manually close the underlying httpx client connection pool.""" | ||
| await self._client.aclose() | ||
|
|
||
| async def __aenter__(self) -> "MpesaAsyncHttpClient": | ||
| """Context manager entry point.""" | ||
| return self | ||
|
|
||
| async def __aexit__(self, exc_type, exc_val, exc_tb) -> None: | ||
| """Context manager exit point. Closes the client.""" | ||
| await self._client.aclose() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 836
🏁 Script executed:
cat -n mpesakit/http_client/mpesa_async_http_client.py | wc -lRepository: Byte-Barn/mpesakit
Length of output: 65
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 1021
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 869
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 1150
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 1289
🏁 Script executed:
Repository: Byte-Barn/mpesakit
Length of output: 5253
raw_responsefield receives inconsistent types across error paths.handle_request_errorpassesraw_response=response_data(adict), while theexceptblocks inpost()andget()passraw_response=getattr(response, "text", None)(astrorNone). Any downstream code that readsMpesaError.raw_responsewill encounter different types depending on the error path, which will causeAttributeError/TypeErrorat runtime if the field is assumed to be one type.Normalize to a consistent type (e.g., always
stror alwaysdict):🛠 Proposed fix for `handle_request_error`
raise MpesaApiException( MpesaError( error_code=f"HTTP_{response.status_code}", error_message=error_message, status_code=response.status_code, - raw_response=response_data, + raw_response=response.text, ) )Also applies to synchronous equivalents in
mpesakit/http_client/mpesa_http_client.py.📝 Committable suggestion
🤖 Prompt for AI Agents