From c8276997ca3d915d05ab287d3c7d453606b9ba8e Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Tue, 18 Aug 2026 08:55:28 +0800 Subject: [PATCH 1/5] Migrate CardHolderData to Pydantic v2 (0.6.0) Replace the hand-rolled CardHolderData class with a Pydantic v2 BaseModel, gaining automatic validation, EmailStr checking, and model_dump-based serialization in place of the manual to_dict construction. Add pydantic>=2.0 and email-validator>=2.0 as dependencies, and an example script demonstrating the validation behaviour. BREAKING CHANGE: CardHolderData now requires keyword arguments. Positional construction, e.g. CardHolderData("0912345678", "Wang", "a@b.com"), raises TypeError. EmailStr is also stricter than the plain str field it replaces, so addresses that were previously accepted may now fail validation. See the migration guide in CHANGELOG.md. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 42 ++++++++++ README.md | 3 + examples/pydantic_validation.py | 133 ++++++++++++++++++++++++++++++++ pyproject.toml | 4 +- tappay/client.py | 2 +- tappay/models.py | 78 +++++++++---------- tests/test_client.py | 4 +- 7 files changed, 224 insertions(+), 42 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 examples/pydantic_validation.py diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..32aedc4 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,42 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [0.6.0] - 2025-12-17 + +### Added +- **Pydantic v2 Integration**: Migrated `CardHolderData` model to use Pydantic v2's `BaseModel` + - Automatic data validation with clear error messages + - Email validation using `EmailStr` type + - Type safety and IDE autocomplete support + - Automatic serialization/deserialization + - JSON schema generation for API documentation +- Added `email-validator` dependency for email validation +- Added example file demonstrating Pydantic v2 validation features (`examples/pydantic_validation.py`) + +### Changed +- `CardHolderData` now requires keyword arguments instead of positional arguments (Pydantic v2 requirement) +- Updated `to_dict()` method to use Pydantic's `model_dump()` for better performance +- Bumped version to 0.6.0 + +### Migration Guide +If you were using positional arguments: +```python +# Old (v0.5.x) +card_holder = Models.CardHolderData("0912345678", "Wang Xiao Ming", "test@example.com") + +# New (v0.6.0+) +card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="test@example.com" +) +``` + +## [0.5.2] - 2024-XX-XX + +### Changed +- Previous version before Pydantic integration diff --git a/README.md b/README.md index 46986d8..a9e19c4 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,9 @@ > [!IMPORTANT] > **Python 2 Support Dropped**: As of version 0.5.0, this library no longer supports Python 2.7. Please use Python 3.8 or newer. +> [!NOTE] +> **Pydantic v2 Integration**: As of version 0.6.0, this library uses Pydantic v2 for enhanced data validation, type safety, and automatic serialization. This provides better error messages and ensures data integrity when working with TapPay APIs. + This is the unofficial Python client library for TapPay's Backend API. To use it you'll need a TapPay account. Sign up at [tappaysdk.com](https://www.tappaysdk.com). ## Installation diff --git a/examples/pydantic_validation.py b/examples/pydantic_validation.py new file mode 100644 index 0000000..9b7758d --- /dev/null +++ b/examples/pydantic_validation.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +""" +Example demonstrating Pydantic v2 validation in TapPay SDK. + +This example shows how Pydantic v2 provides automatic validation +and helpful error messages when creating CardHolderData objects. +""" + +from pydantic import ValidationError + +from tappay import Client, Models + + +def example_valid_cardholder(): + """Example of creating a valid CardHolderData object.""" + print("=" * 60) + print("Example 1: Valid CardHolderData") + print("=" * 60) + + card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="test@example.com", + zip_code="100", + address="台北市中正區", + ) + + print("✓ CardHolderData created successfully!") + print(f" Name: {card_holder.name}") + print(f" Email: {card_holder.email}") + print(f" Phone: {card_holder.phone_number}") + print() + + # Convert to dictionary (for API calls) + data_dict = card_holder.to_dict() + print(f"✓ Serialized to dict: {data_dict}") + print() + + +def example_invalid_email(): + """Example showing email validation.""" + print("=" * 60) + print("Example 2: Invalid Email Validation") + print("=" * 60) + + try: + _card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="invalid-email", # Invalid email format + ) + except ValidationError as e: + print("✗ Validation failed (as expected):") + print(f" {e.errors()[0]['msg']}") + print(f" Field: {e.errors()[0]['loc'][0]}") + print() + + +def example_missing_required_fields(): + """Example showing required field validation.""" + print("=" * 60) + print("Example 3: Missing Required Fields") + print("=" * 60) + + try: + _card_holder = Models.CardHolderData( + phone_number="0912345678", + # Missing 'name' and 'email' required fields + ) + except ValidationError as e: + print("✗ Validation failed (as expected):") + for error in e.errors(): + print(f" Missing field: {error['loc'][0]}") + print() + + +def example_optional_fields(): + """Example showing optional fields can be omitted.""" + print("=" * 60) + print("Example 4: Optional Fields") + print("=" * 60) + + # Only required fields + card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="test@example.com", + ) + + print("✓ CardHolderData created with only required fields") + data_dict = card_holder.to_dict() + print(f" Serialized (None values excluded): {data_dict}") + print() + + +def example_with_client(): + """Example showing usage with TapPay client.""" + print("=" * 60) + print("Example 5: Using with TapPay Client") + print("=" * 60) + + # Initialize client (sandbox mode) + client = Client( + is_sandbox=True, + partner_key="your_partner_key", + merchant_id="your_merchant_id", + ) + + # Create validated cardholder data + _card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="test@example.com", + ) + + print("✓ Client initialized") + print("✓ CardHolderData validated and ready for API calls") + print(f" API Host: {client.api_host}") + print() + + +if __name__ == "__main__": + print("\n🎯 TapPay SDK - Pydantic v2 Validation Examples\n") + + example_valid_cardholder() + example_invalid_email() + example_missing_required_fields() + example_optional_fields() + example_with_client() + + print("=" * 60) + print("✨ All examples completed!") + print("=" * 60) diff --git a/pyproject.toml b/pyproject.toml index bc00b7c..c07379d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tappay" -version = "0.5.2" +version = "0.6.0" authors = [ { name="Shih Wei Chris Lo", email="shihwei@gmail.com" }, ] @@ -26,6 +26,8 @@ classifiers = [ ] dependencies = [ "requests>=2.4.2", + "pydantic>=2.0", + "email-validator>=2.0", ] [project.urls] diff --git a/tappay/client.py b/tappay/client.py index 37a3ce4..9f14847 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -18,7 +18,7 @@ # or just duplicate/move it. Let's define it in client.py for now to # avoid circular dependency if __init__ imports client. Actually, the # original code used it in User-Agent. -VERSION = "0.5.2" +VERSION = "0.6.0" class Client: diff --git a/tappay/models.py b/tappay/models.py index 43ebed3..e61a652 100644 --- a/tappay/models.py +++ b/tappay/models.py @@ -1,4 +1,6 @@ -from typing import Any, Dict, Optional +from typing import Optional + +from pydantic import BaseModel, EmailStr, Field class Models: @@ -9,44 +11,42 @@ class Currencies: TWD = "TWD" - class CardHolderData: - """Card holder data model.""" - - phone_number: Optional[str] = None - name: Optional[str] = None - email: Optional[str] = None - zip_code: Optional[str] = None - address: Optional[str] = None - national_id: Optional[str] = None - - def __init__( - self, - phone_number: str, - name: str, - email: str, - zip_code: Optional[str] = None, - address: Optional[str] = None, - national_id: Optional[str] = None, - ): - self.phone_number = phone_number - self.name = name - self.email = email - self.zip_code = zip_code - self.address = address - self.national_id = national_id - - def to_dict(self) -> Dict[str, Any]: - result_dict = { - "phone_number": self.phone_number, - "name": self.name, - "email": self.email, + class CardHolderData(BaseModel): + """Card holder data model using Pydantic v2. + + This model provides automatic validation and serialization + for cardholder information required by TapPay APIs. + """ + + phone_number: str = Field(..., description="Cardholder's phone number") + name: str = Field(..., description="Cardholder's full name") + email: EmailStr = Field(..., description="Cardholder's email address") + zip_code: Optional[str] = Field(None, description="Cardholder's zip code") + address: Optional[str] = Field(None, description="Cardholder's address") + national_id: Optional[str] = Field(None, description="Cardholder's national ID") + + model_config = { + "json_schema_extra": { + "examples": [ + { + "phone_number": "0912345678", + "name": "Joe Chen", + "email": "test@example.com", + "zip_code": "100", + "address": "台北市中正區", + "national_id": "A123456789", + } + ] } + } + + def to_dict(self) -> dict: + """Convert the model to a dictionary, excluding None values. - if self.zip_code: - result_dict["zip_code"] = self.zip_code - if self.address: - result_dict["address"] = self.address - if self.national_id: - result_dict["national_id"] = self.national_id + This method maintains backward compatibility with the previous + implementation while leveraging Pydantic's serialization. - return result_dict + Returns: + dict: Dictionary representation with None values excluded + """ + return self.model_dump(exclude_none=True, by_alias=False) diff --git a/tests/test_client.py b/tests/test_client.py index 2d5a1e7..d7efb9e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -55,7 +55,9 @@ def test_api_error_handling_401(sandbox_client): prime="p", amount=1, details="d", - card_holder_data=Models.CardHolderData("p", "n", "e"), + card_holder_data=Models.CardHolderData( + phone_number="p", name="n", email="e@example.com" + ), ) From 3129e5a85873780d25aec1cb8bc414aab010d40f Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Tue, 18 Aug 2026 08:55:50 +0800 Subject: [PATCH 2/5] Add request timeouts, redact secrets from logs (0.6.1) Security: debug logging previously emitted the x-api-key partner key, the full request body (partner_key, prime, card_key, card_token and the entire cardholder block), and the raw response body including card_secret and card_info. Any application running at DEBUG level was writing payment credentials and cardholder PII to its logs. These values are now replaced with ***REDACTED*** before reaching the logger, matched case-insensitively at any nesting depth. Log records are built lazily behind isEnabledFor(), so no redaction or response parsing happens when debug logging is off. Requests previously had no timeout at all, so a stalled TapPay endpoint could block the calling thread indefinitely and exhaust a web application's worker pool. Client now takes a timeout argument defaulting to (3.05, 27.0) seconds, and every API method accepts a keyword-only timeout override. The parameter is keyword-only deliberately: **kwargs forwards into the request body, so a positional timeout would otherwise be posted as an API field. A sentinel distinguishes an omitted argument from an explicit None, keeping None's "no timeout" meaning intact at the constructor. Also: - Add a py.typed marker (PEP 561) so the type hints this library already ships are visible to consumers' type checkers instead of being ignored. - Source the version from installed distribution metadata. __version__ reported 0.5.2 while the package was 0.6.0; it is now declared only in pyproject.toml, so __version__, client.VERSION and the User-Agent header cannot drift apart. - AuthenticationError was raised as a bare class with no message; it now reports "401 response from " like the other error branches. - Correct return annotations to Optional[Dict[str, Any]]; every method can return None on an HTTP 204. - Remove leftover development scratch notes from client.py. Test coverage rises from 73% to 92% (client.py 65% to 89%). Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 42 ++++++++ MANIFEST.in | 1 + README.md | 37 +++++++ pyproject.toml | 5 +- tappay/__init__.py | 4 +- tappay/_version.py | 15 +++ tappay/client.py | 237 +++++++++++++++++++++++++++++++++-------- tappay/py.typed | 0 tests/test_client.py | 247 ++++++++++++++++++++++++++++++++++++++++++- 9 files changed, 539 insertions(+), 49 deletions(-) create mode 100644 tappay/_version.py create mode 100644 tappay/py.typed diff --git a/CHANGELOG.md b/CHANGELOG.md index 32aedc4..5f6e246 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,48 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.6.1] - 2026-08-18 + +### Security +- **Credentials and cardholder PII are no longer written to logs.** Debug logging + previously emitted the full request headers (including the `x-api-key` partner + key), the full request body (`partner_key`, `prime`, `card_key`, `card_token` + and the whole `cardholder` block), and the raw response body (including + `card_secret` and `card_info`). Any application running with `logging.DEBUG` + enabled was writing payment credentials and personal data to its logs. These + values are now replaced with `***REDACTED***` before reaching the logger. +- Debug log records are now built lazily and guarded by `logger.isEnabledFor()`, + so no redaction or response parsing happens when debug logging is off. + +### Added +- **Request timeouts.** `Client` accepts a `timeout` argument, defaulting to + `DEFAULT_TIMEOUT` (`(3.05, 27.0)` seconds). Previously requests had no timeout + at all, so a stalled TapPay endpoint could block the calling thread forever and + exhaust a web application's worker pool. Every API method also accepts a + keyword-only `timeout` to override the client default per call. +- `py.typed` marker (PEP 561), so the type hints this library already ships are + now visible to consumers' type checkers instead of being silently ignored. + +### Fixed +- `tappay.__version__` reported `0.5.2` while the package was `0.6.0`. The version + is now declared only in `pyproject.toml` and read from the installed + distribution metadata, so `__version__`, `client.VERSION` and the `User-Agent` + header can no longer drift apart. +- `AuthenticationError` was raised as a bare class and carried no message; it now + reports `"401 response from "` like the other error branches. +- Return annotations on the API methods said `Dict[str, Any]` but every method can + return `None` on an HTTP 204. They are now `Optional[Dict[str, Any]]`. +- Removed a block of leftover development scratch notes from `client.py`. + +### Notes +- Passing `timeout=None` to `Client(...)` restores the old unbounded behaviour. + Passing `timeout=None` to an individual call means "inherit the client default"; + omitting it entirely does the same. +- `CardHolderData` silently ignores unknown keyword arguments (Pydantic's default), + so a misspelled field is dropped without warning. `EmailStr` validation added in + 0.6.0 is also stricter than the plain `str` field it replaced. Both are unchanged + in this release but worth knowing when upgrading from 0.5.x. + ## [0.6.0] - 2025-12-17 ### Added diff --git a/MANIFEST.in b/MANIFEST.in index 4d12a2e..5c219bc 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,3 +1,4 @@ include CHANGES.md include LICENSE.txt include README.md +include tappay/py.typed diff --git a/README.md b/README.md index a9e19c4..58843e3 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,9 @@ > [!NOTE] > **Pydantic v2 Integration**: As of version 0.6.0, this library uses Pydantic v2 for enhanced data validation, type safety, and automatic serialization. This provides better error messages and ensures data integrity when working with TapPay APIs. +> [!NOTE] +> **Typed**: As of version 0.6.1, this package ships a `py.typed` marker (PEP 561), so mypy, Pyright, and your IDE will use the library's own type hints. + This is the unofficial Python client library for TapPay's Backend API. To use it you'll need a TapPay account. Sign up at [tappaysdk.com](https://www.tappaysdk.com). ## Installation @@ -44,6 +47,40 @@ For production, you can set `TAPPAY_PARTNER_KEY` and `TAPPAY_MERCHANT_ID` enviro client = tappay.Client(is_sandbox=False) ``` +### Timeouts + +Every request carries a timeout by default: `(3.05, 27.0)` seconds, as a +`(connect, read)` pair. Override it for all calls on a client: + +```python +client = tappay.Client(is_sandbox=False, timeout=10.0) +``` + +Or for a single call, using the keyword-only `timeout` argument available on +every API method: + +```python +response = client.refund(rec_trade_id="rec_trade_id", amount=100, timeout=(3.05, 60.0)) +``` + +Passing `timeout=None` to the constructor disables the timeout entirely, which +lets a stalled request block the calling thread indefinitely. This is almost +never what you want in a server process. + +### Logging + +The client logs request and response details at `DEBUG` level. Credentials +(`partner_key`, `x-api-key`), card handles (`prime`, `card_key`, `card_token`), +and cardholder PII (name, email, phone number, address, national ID) are +replaced with `***REDACTED***` before anything reaches the logger, so enabling +debug logging will not spill payment data into your log aggregator. + +```python +import logging + +logging.getLogger("tappay.client").setLevel(logging.DEBUG) +``` + ### Pay by Prime ```python diff --git a/pyproject.toml b/pyproject.toml index c07379d..6f4b298 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tappay" -version = "0.6.0" +version = "0.6.1" authors = [ { name="Shih Wei Chris Lo", email="shihwei@gmail.com" }, ] @@ -34,6 +34,9 @@ dependencies = [ "Homepage" = "https://github.com/shihweilo/tappay-python" "Bug Tracker" = "https://github.com/shihweilo/tappay-python/issues" +[tool.setuptools.package-data] +tappay = ["py.typed"] + [tool.setuptools.packages.find] where = ["."] include = ["tappay*"] diff --git a/tappay/__init__.py b/tappay/__init__.py index ce00781..0d0b873 100644 --- a/tappay/__init__.py +++ b/tappay/__init__.py @@ -1,5 +1,4 @@ -__version__ = "0.5.2" - +from tappay._version import __version__ from tappay.client import Client from tappay.exceptions import ( AuthenticationError, @@ -11,6 +10,7 @@ from tappay.models import Models __all__ = [ + "__version__", "Client", "Models", "Exceptions", diff --git a/tappay/_version.py b/tappay/_version.py new file mode 100644 index 0000000..0d45a58 --- /dev/null +++ b/tappay/_version.py @@ -0,0 +1,15 @@ +"""Single source of truth for the package version. + +The version is declared once in ``pyproject.toml`` and read back from the +installed distribution metadata, so ``tappay.__version__`` and the +``User-Agent`` header can never drift from the published package. +""" + +from importlib.metadata import PackageNotFoundError, version + +try: + __version__ = version("tappay") +except PackageNotFoundError: # pragma: no cover - running from an uninstalled checkout + __version__ = "0.0.0.dev0" + +__all__ = ["__version__"] diff --git a/tappay/client.py b/tappay/client.py index 9f14847..36d907c 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -1,24 +1,94 @@ import logging import os from platform import python_version -from typing import Any, Dict, Optional +from typing import Any, Dict, Optional, Tuple, Union import requests +from tappay._version import __version__ from tappay.exceptions import Exceptions from tappay.models import Models logger = logging.getLogger(__name__) -# We need to access __version__ from somewhere, commonly from a package -# level or hardcoded here then imported. For now I will hardcode it here -# or pass it in. Better yet, I will define __version__ in __init__.py and -# import it here? No, circular import. I will put __version__ in a -# separate file or keep it in __init__ and pass it to Client if needed, -# or just duplicate/move it. Let's define it in client.py for now to -# avoid circular dependency if __init__ imports client. Actually, the -# original code used it in User-Agent. -VERSION = "0.6.0" +#: Retained for backward compatibility; prefer :data:`tappay.__version__`. +VERSION = __version__ + +#: A ``(connect, read)`` pair in seconds. The connect value sits just above a +#: multiple of the common 3 second TCP retransmission window, as recommended by +#: the ``requests`` documentation. +DEFAULT_TIMEOUT: Tuple[float, float] = (3.05, 27.0) + +#: Anything ``requests`` accepts for its ``timeout`` argument. ``None`` means +#: "block indefinitely" and is strongly discouraged for server-side use. +TimeoutType = Union[float, Tuple[float, float], None] + + +class _Unset: + """Sentinel distinguishing "argument omitted" from an explicit ``None``.""" + + def __repr__(self) -> str: + return "" + + +_UNSET = _Unset() + +_REDACTED = "***REDACTED***" + +#: Keys whose values are credentials, card secrets, or cardholder PII. Matched +#: case-insensitively against both request payloads and response bodies, at any +#: depth, before anything is handed to the logger. +_SENSITIVE_KEYS = frozenset( + { + # Credentials + "partner_key", + "x-api-key", + "authorization", + # Card secrets and handles + "prime", + "card_key", + "card_token", + "card_number", + # Partial PAN returned in `card_info` + "bin_code", + "last_four", + # Cardholder PII + "phone_number", + "name", + "email", + "zip_code", + "address", + "national_id", + } +) + + +def _redact(value: Any) -> Any: + """Return a copy of ``value`` with sensitive fields replaced. + + Recurses through nested mappings and sequences so that container fields + such as ``cardholder`` and ``card_secret`` are cleaned in place rather than + blanked wholesale, keeping the surrounding structure useful for debugging. + """ + if isinstance(value, dict): + return { + key: (_REDACTED if str(key).lower() in _SENSITIVE_KEYS else _redact(item)) + for key, item in value.items() + } + if isinstance(value, (list, tuple)): + return [_redact(item) for item in value] + return value + + +def _redacted_body(response: requests.Response) -> Any: + """Render a response body for logging without leaking card secrets.""" + try: + return _redact(response.json()) + except ValueError: + try: + return f"<{len(response.content)} bytes, unparsed>" + except TypeError: # pragma: no cover - non-standard response object + return "" class Client: @@ -31,6 +101,7 @@ def __init__( merchant_id: Optional[str] = None, app_name: Optional[str] = None, app_version: Optional[str] = None, + timeout: TimeoutType = DEFAULT_TIMEOUT, ): """ Create a Client object to start making calls to TapPay APIs. @@ -40,6 +111,9 @@ def __init__( :param str merchant_id: Your TapPay merchant ID (optional) :param str app_name: This optional value is added to the user-agent header :param str app_version: This optional value is added to the user-agent header + :param timeout: Default request timeout in seconds, as a float or a + ``(connect, read)`` tuple. Passing ``None`` disables the timeout and + lets a stalled request block the calling thread forever. """ if not isinstance(is_sandbox, bool): raise TypeError( @@ -54,10 +128,12 @@ def __init__( if self.merchant_id is None: raise ValueError("Missing required value for `merchant_id`") + self.timeout = timeout + subdomain = "sandbox" if is_sandbox else "prod" self.api_host = f"{subdomain}.tappaysdk.com" - user_agent = f"tappay-python/{VERSION} python/{python_version()}" + user_agent = f"tappay-python/{__version__} python/{python_version()}" if app_name and app_version: user_agent += f" {app_name}/{app_version}" @@ -74,8 +150,10 @@ def pay_by_prime( amount: int, details: str, card_holder_data: Models.CardHolderData, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, - ) -> Dict[str, Any]: + ) -> Optional[Dict[str, Any]]: """ Make a payment using "prime" obtained from TapPay frontend SDK Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#pay-by-prime-api @@ -98,7 +176,7 @@ def pay_by_prime( params.update(**kwargs) return self.__post_with_partner_key_and_merchant_id( - "/tpc/payment/pay-by-prime", params + "/tpc/payment/pay-by-prime", params, timeout ) def pay_by_token( @@ -107,8 +185,10 @@ def pay_by_token( card_token: str, amount: int, details: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, - ) -> Dict[str, Any]: + ) -> Optional[Dict[str, Any]]: """ Make a payment using previously obtained card secrets (key & token) Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#pay-by-card-token-api @@ -125,10 +205,17 @@ def pay_by_token( params.update(**kwargs) return self.__post_with_partner_key_and_merchant_id( - "/tpc/payment/pay-by-token", params + "/tpc/payment/pay-by-token", params, timeout ) - def refund(self, rec_trade_id: str, amount: int, **kwargs: Any) -> Dict[str, Any]: + def refund( + self, + rec_trade_id: str, + amount: int, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + **kwargs: Any, + ) -> Optional[Dict[str, Any]]: """ Refund a payment Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#refund-api @@ -141,7 +228,7 @@ def refund(self, rec_trade_id: str, amount: int, **kwargs: Any) -> Dict[str, Any if kwargs: params.update(**kwargs) - return self.__post_with_partner_key("/tpc/transaction/refund", params) + return self.__post_with_partner_key("/tpc/transaction/refund", params, timeout) def get_records( self, @@ -149,7 +236,9 @@ def get_records( page: int = 0, records_per_page: int = 50, order_by_dict: Optional[Dict[str, Any]] = None, - ) -> Dict[str, Any]: + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: """ Query historical records Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#record-api @@ -163,9 +252,14 @@ def get_records( if order_by_dict: params["order_by"] = order_by_dict - return self.__post_with_partner_key("/tpc/transaction/query", params) + return self.__post_with_partner_key("/tpc/transaction/query", params, timeout) - def capture_today(self, rec_trade_id: str) -> Dict[str, Any]: + def capture_today( + self, + rec_trade_id: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: """ Capture specific payment record Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#cap-today-api @@ -174,9 +268,14 @@ def capture_today(self, rec_trade_id: str) -> Dict[str, Any]: "rec_trade_id": rec_trade_id, } - return self.__post_with_partner_key("/tpc/transaction/cap", params) + return self.__post_with_partner_key("/tpc/transaction/cap", params, timeout) - def cancel_capture(self, rec_trade_id: str) -> Dict[str, Any]: + def cancel_capture( + self, + rec_trade_id: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: """ Cancel a specific capture Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#cap-cancel-api @@ -185,9 +284,16 @@ def cancel_capture(self, rec_trade_id: str) -> Dict[str, Any]: "rec_trade_id": rec_trade_id, } - return self.__post_with_partner_key("/tpc/transaction/cap/cancel", params) + return self.__post_with_partner_key( + "/tpc/transaction/cap/cancel", params, timeout + ) - def get_trade_history(self, rec_trade_id: str) -> Dict[str, Any]: + def get_trade_history( + self, + rec_trade_id: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: """ Get record and status of a specific transaction Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#trade-history-api @@ -196,14 +302,18 @@ def get_trade_history(self, rec_trade_id: str) -> Dict[str, Any]: "rec_trade_id": rec_trade_id, } - return self.__post_with_partner_key("/tpc/transaction/trade-history", params) + return self.__post_with_partner_key( + "/tpc/transaction/trade-history", params, timeout + ) def bind_card( self, prime: str, card_holder_data: Models.CardHolderData, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, - ) -> Dict[str, Any]: + ) -> Optional[Dict[str, Any]]: """ Bind new credit card Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#bind-card-api @@ -223,9 +333,17 @@ def bind_card( if kwargs: params.update(**kwargs) - return self.__post_with_partner_key_and_merchant_id("/tpc/card/bind", params) + return self.__post_with_partner_key_and_merchant_id( + "/tpc/card/bind", params, timeout + ) - def remove_card(self, card_key: str, card_token: str) -> Dict[str, Any]: + def remove_card( + self, + card_key: str, + card_token: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: """ Remove bound credit card Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#remove-card-api @@ -235,9 +353,15 @@ def remove_card(self, card_key: str, card_token: str) -> Dict[str, Any]: "card_token": card_token, } - return self.__post_with_partner_key("/tpc/card/remove", params) + return self.__post_with_partner_key("/tpc/card/remove", params, timeout) - def cancel_refund(self, rec_trade_id: str, **kwargs: Any) -> Dict[str, Any]: + def cancel_refund( + self, + rec_trade_id: str, + *, + timeout: Union[TimeoutType, _Unset] = _UNSET, + **kwargs: Any, + ) -> Optional[Dict[str, Any]]: """ Cancel a single refund Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#refund-cancel-api @@ -249,37 +373,60 @@ def cancel_refund(self, rec_trade_id: str, **kwargs: Any) -> Dict[str, Any]: if kwargs: params.update(**kwargs) - return self.__post_with_partner_key("/tpc/transaction/refund/cancel", params) + return self.__post_with_partner_key( + "/tpc/transaction/refund/cancel", params, timeout + ) def __post_with_partner_key( - self, request_uri: str, params: Dict[str, Any] - ) -> Dict[str, Any]: + self, + request_uri: str, + params: Dict[str, Any], + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: params = dict(params, partner_key=self.partner_key) - return self.__post(request_uri, params) + return self.__post(request_uri, params, timeout) def __post_with_partner_key_and_merchant_id( - self, request_uri: str, params: Dict[str, Any] - ) -> Dict[str, Any]: + self, + request_uri: str, + params: Dict[str, Any], + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: params = dict(params, merchant_id=self.merchant_id) - return self.__post_with_partner_key(request_uri, params) + return self.__post_with_partner_key(request_uri, params, timeout) - def __post(self, request_uri: str, params: Dict[str, Any]) -> Dict[str, Any]: + def __post( + self, + request_uri: str, + params: Dict[str, Any], + timeout: Union[TimeoutType, _Unset] = _UNSET, + ) -> Optional[Dict[str, Any]]: uri = f"https://{self.api_host}{request_uri}" params = dict(params) - logger.debug(f"POST to: {uri}") - logger.debug(f"POST headers: {self.headers}") - logger.debug(f"POST params: {params}") + effective_timeout = self.timeout if isinstance(timeout, _Unset) else timeout - response = requests.post(uri, json=params, headers=self.headers) + # Guarded so the redaction pass is skipped entirely when debug logging + # is off, and so credentials are never formatted into a log record. + if logger.isEnabledFor(logging.DEBUG): + logger.debug("POST to: %s", uri) + logger.debug("POST headers: %s", _redact(self.headers)) + logger.debug("POST params: %s", _redact(params)) + logger.debug("POST timeout: %s", effective_timeout) + + response = requests.post( + uri, json=params, headers=self.headers, timeout=effective_timeout + ) return self.__parse(response) def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: - logger.debug(f"response status: {response.status_code}") - logger.debug(f"response content: {response.content}") + if logger.isEnabledFor(logging.DEBUG): + logger.debug("response status: %s", response.status_code) + logger.debug("response content: %s", _redacted_body(response)) if response.status_code == 401: - raise Exceptions.AuthenticationError + message = f"{response.status_code} response from {self.api_host}" + raise Exceptions.AuthenticationError(message) elif response.status_code == 204: return None elif 200 <= response.status_code < 300: diff --git a/tappay/py.typed b/tappay/py.typed new file mode 100644 index 0000000..e69de29 diff --git a/tests/test_client.py b/tests/test_client.py index d7efb9e..0552825 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,8 +1,12 @@ +import importlib.metadata +import logging +import pathlib from unittest.mock import Mock, patch import pytest -from tappay.client import Client, Models +import tappay +from tappay.client import DEFAULT_TIMEOUT, VERSION, Client, Models, _redact from tappay.exceptions import AuthenticationError, ClientError, ServerError @@ -79,3 +83,244 @@ def test_api_error_handling_500(sandbox_client): with pytest.raises(ServerError): sandbox_client.capture_today("id") + + +# --- Timeout handling (0.6.1) --------------------------------------------- + + +def _mock_post(status_code=200, payload=None): + """Build a patched `requests.post` returning a canned response.""" + mock_response = Mock() + mock_response.status_code = status_code + mock_response.json.return_value = payload if payload is not None else {"status": 0} + return mock_response + + +def test_default_timeout_is_applied(sandbox_client): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + sandbox_client.capture_today("id") + + assert mock_post.call_args.kwargs["timeout"] == DEFAULT_TIMEOUT + + +def test_client_level_timeout_override(card_holder): + client = Client(is_sandbox=True, partner_key="pk", merchant_id="mid", timeout=5.0) + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder + ) + + assert mock_post.call_args.kwargs["timeout"] == 5.0 + + +def test_per_call_timeout_overrides_client_default(sandbox_client, card_holder): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + sandbox_client.pay_by_prime( + prime="p", + amount=1, + details="d", + card_holder_data=card_holder, + timeout=(1.0, 2.0), + ) + + assert mock_post.call_args.kwargs["timeout"] == (1.0, 2.0) + + +def test_explicit_none_timeout_is_preserved(sandbox_client): + """`None` means "no timeout" and must not be swallowed by the sentinel.""" + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + sandbox_client.capture_today("id", timeout=None) + + assert mock_post.call_args.kwargs["timeout"] is None + + +def test_timeout_is_not_forwarded_as_an_api_field(sandbox_client): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + sandbox_client.refund("rec", 100, timeout=9.0) + + assert "timeout" not in mock_post.call_args.kwargs["json"] + + +def test_timeout_propagates_to_every_endpoint(sandbox_client, card_holder): + """Guard against a new method forgetting to thread `timeout` through.""" + calls = [ + ("pay_by_token", ("ck", "ct", 100, "d"), {}), + ("refund", ("rec", 100), {}), + ("get_records", ({"time": {}},), {}), + ("capture_today", ("rec",), {}), + ("cancel_capture", ("rec",), {}), + ("get_trade_history", ("rec",), {}), + ("remove_card", ("ck", "ct"), {}), + ("cancel_refund", ("rec",), {}), + ("bind_card", ("prime",), {"card_holder_data": card_holder}), + ] + + for method_name, args, kwargs in calls: + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + getattr(sandbox_client, method_name)(*args, timeout=3.5, **kwargs) + + assert mock_post.call_args.kwargs["timeout"] == 3.5, method_name + + +# --- Log redaction (0.6.1) ------------------------------------------------ + + +def test_credentials_and_pii_are_redacted_in_request_logs(caplog): + caplog.set_level(logging.DEBUG, logger="tappay.client") + client = Client( + is_sandbox=True, + partner_key="SECRET_PARTNER_KEY", + merchant_id="mid", + ) + card_holder = Models.CardHolderData( + phone_number="0912345678", + name="Wang Xiao Ming", + email="secret@example.com", + national_id="A123456789", + ) + + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post() + client.pay_by_prime( + prime="SECRET_PRIME", + amount=100, + details="Order #1", + card_holder_data=card_holder, + ) + + for secret in ( + "SECRET_PARTNER_KEY", + "SECRET_PRIME", + "0912345678", + "Wang Xiao Ming", + "secret@example.com", + "A123456789", + ): + assert secret not in caplog.text, f"{secret} leaked into logs" + + assert "***REDACTED***" in caplog.text + # Non-sensitive context is still useful for debugging. + assert "Order #1" in caplog.text + assert "sandbox.tappaysdk.com" in caplog.text + + +def test_card_secrets_are_redacted_in_response_logs(sandbox_client, caplog): + caplog.set_level(logging.DEBUG, logger="tappay.client") + + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post( + payload={ + "status": 0, + "rec_trade_id": "REC123", + "card_secret": { + "card_key": "SECRET_CARD_KEY", + "card_token": "SECRET_CARD_TOKEN", + }, + "card_info": {"bin_code": "424242", "last_four": "4242"}, + } + ) + sandbox_client.capture_today("rec") + + for secret in ("SECRET_CARD_KEY", "SECRET_CARD_TOKEN", "424242", "4242"): + assert secret not in caplog.text, f"{secret} leaked into logs" + + assert "REC123" in caplog.text + + +def test_redaction_is_skipped_when_debug_logging_is_off(sandbox_client, caplog): + """No response parsing work should happen when debug logging is disabled.""" + caplog.set_level(logging.INFO, logger="tappay.client") + + with patch("tappay.client.requests.post") as mock_post: + response = _mock_post() + mock_post.return_value = response + sandbox_client.capture_today("rec") + + # Parsed once to build the return value, never for logging. + assert response.json.call_count == 1 + + assert caplog.text == "" + + +def test_redact_leaves_unrelated_structures_intact(): + payload = {"amount": 100, "items": [{"sku": "A1", "name": "Widget"}]} + assert _redact(payload) == { + "amount": 100, + "items": [{"sku": "A1", "name": "***REDACTED***"}], + } + + +def test_redact_matches_keys_case_insensitively(): + assert _redact({"Partner_Key": "s", "X-API-KEY": "s"}) == { + "Partner_Key": "***REDACTED***", + "X-API-KEY": "***REDACTED***", + } + + +# --- Version single-sourcing (0.6.1) -------------------------------------- + + +def test_version_matches_installed_distribution_metadata(): + assert tappay.__version__ == importlib.metadata.version("tappay") + + +def test_client_version_constant_matches_package_version(): + assert VERSION == tappay.__version__ + + +def test_user_agent_reports_the_package_version(sandbox_client): + assert sandbox_client.headers["User-Agent"].startswith( + f"tappay-python/{tappay.__version__} python/" + ) + + +def test_user_agent_includes_app_name_and_version(): + client = Client( + is_sandbox=True, + partner_key="pk", + merchant_id="mid", + app_name="MyApp", + app_version="1.2.3", + ) + assert client.headers["User-Agent"].endswith(" MyApp/1.2.3") + + +def test_package_ships_a_py_typed_marker(): + marker = pathlib.Path(tappay.__file__).parent / "py.typed" + assert marker.is_file() + + +# --- Error surface (0.6.1) ------------------------------------------------ + + +def test_authentication_error_carries_a_message(sandbox_client): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post(status_code=401) + + with pytest.raises(AuthenticationError) as exc_info: + sandbox_client.capture_today("id") + + assert str(exc_info.value) == "401 response from sandbox.tappaysdk.com" + + +def test_no_content_response_returns_none(sandbox_client): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post(status_code=204) + + assert sandbox_client.capture_today("id") is None + + +def test_unexpected_status_code_raises_server_error(sandbox_client): + with patch("tappay.client.requests.post") as mock_post: + mock_post.return_value = _mock_post(status_code=302) + + with pytest.raises(ServerError) as exc_info: + sandbox_client.capture_today("id") + + assert "302" in str(exc_info.value) From fd412ca7517094d4eae98326c0a1c064591c405c Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Tue, 18 Aug 2026 09:07:44 +0800 Subject: [PATCH 3/5] Add pooled session, selective retries, error surface (0.7.0) Replace the module-level requests.post with a per-client requests.Session, so repeated calls reuse an established TLS connection instead of paying for a fresh handshake each time. Add close() and context manager support to release pooled connections. Mount retries per-URL rather than session-wide. requests resolves adapters by longest matching prefix, so the read-only query endpoints pick up a retrying adapter (2 attempts, exponential backoff, 429/500/502/503/504) while everything else falls back to a non-retrying one. Payment, refund, capture, bind and remove endpoints are excluded deliberately: TapPay exposes no idempotency key, so retrying a request that actually succeeded upstream would charge the cardholder twice. A test asserts this directly against every write path so the guarantee cannot silently regress. TapPay reports declines and other business failures with an HTTP 200 and a non-zero status field, invisible to HTTP-level error handling. Add raise_on_error=True to raise TapPayError carrying .status, .msg and the full .response. Defaults to False to preserve existing behaviour. Add a keyword-only currency argument to pay_by_prime, pay_by_token and bind_card, still defaulting to TWD. Models.Currencies becomes a str-backed enum of 15 currencies; members compare equal to their string form and serialize as "TWD" through json.dumps, and plain strings remain accepted. Tooling: - Add a dev optional dependency group, [tool.mypy] config, and a mypy step in CI. CI is split into a quality job and a test matrix because current mypy releases cannot target Python 3.8. - Add Python 3.13 to the matrix and classifiers. - Raise the requests floor to 2.26 and add urllib3>=1.26; both are needed for the Retry(allowed_methods=...) API. - Drop --cov from pytest addopts so a bare pytest works without pytest-cov. - Correct the README badge from black to ruff. Test coverage rises from 92% to 100% (223 statements, 73 tests). Tests now patch client.session.post rather than tappay.client.requests.post. BREAKING CHANGE: downstream tests that mock tappay.client.requests.post will no longer intercept anything; patch client.session.post instead. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 53 +++- CHANGELOG.md | 46 +++ README.md | 82 ++++- pyproject.toml | 25 +- tappay/__init__.py | 2 + tappay/client.py | 123 +++++++- tappay/exceptions.py | 30 ++ tappay/models.py | 28 +- tests/conftest.py | 26 ++ tests/test_client.py | 648 +++++++++++++++++++++++++++++++-------- 10 files changed, 900 insertions(+), 163 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 03f8463..dc35903 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,4 +1,3 @@ - name: CI on: @@ -8,33 +7,53 @@ on: branches: [ "master", "main" ] jobs: - build: + quality: + name: Lint and type check + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - name: Set up Python + uses: actions/setup-python@v5 + with: + python-version: "3.12" + + - name: Install dependencies + run: | + python -m pip install --upgrade pip + pip install -e ".[dev]" + + - name: Lint with Ruff + run: | + ruff check . + ruff format --check . + + # Runs on a single modern interpreter: current mypy releases cannot target + # Python 3.8, and type checking does not need to repeat across the matrix. + - name: Type check with mypy + run: mypy tappay + + test: + name: Test on Python ${{ matrix.python-version }} runs-on: ubuntu-latest strategy: + fail-fast: false matrix: - python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"] + python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] steps: - uses: actions/checkout@v4 - + - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - + - name: Install dependencies run: | python -m pip install --upgrade pip - pip install ruff pytest pytest-cov pip install -e . - - - name: Lint with Ruff - run: | - # stop the build if there are Python syntax errors or undefined names - ruff check . - # check formatting - ruff format --check . - + pip install pytest pytest-cov + - name: Test with pytest - run: | - pytest + run: pytest --cov=tappay --cov-report=term-missing diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f6e246..01cbbda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,52 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.7.0] - 2026-08-18 + +### Added +- **Connection pooling.** The client now holds a `requests.Session`, so repeated + calls reuse an established TLS connection instead of paying for a fresh + handshake each time. Added `Client.close()` and context manager support to + release pooled connections. +- **Selective retries.** The read-only endpoints (`get_records`, + `get_trade_history`) retry twice on connection errors and HTTP + 429/500/502/503/504 with exponential backoff. Payment, refund, capture, bind + and remove endpoints are deliberately excluded: TapPay exposes no idempotency + key, so retrying a request that actually succeeded upstream would charge the + cardholder twice. Configure with `max_retries=`, disable with `max_retries=0`. +- **`raise_on_error` and `TapPayError`.** TapPay reports declines and other + business failures with an HTTP 200 and a non-zero `status` field, which + HTTP-level error handling cannot see. Constructing the client with + `raise_on_error=True` now raises `TapPayError`, carrying `.status`, `.msg` and + the full `.response`. Defaults to `False` to preserve existing behaviour. +- **Currency selection.** `pay_by_prime`, `pay_by_token` and `bind_card` accept a + keyword-only `currency`, defaulting to TWD as before. `Models.Currencies` is + now a `str`-backed enum covering 15 currencies, and plain strings are accepted + so a newly supported currency is usable before this list catches up. +- `dev` optional dependency group (`pip install -e ".[dev]"`), a `[tool.mypy]` + configuration, and a mypy step in CI. +- Python 3.13 added to the CI test matrix and the package classifiers. + +### Changed +- `requests` floor raised to 2.26 and `urllib3>=1.26` added as an explicit + dependency; both are required for the `Retry(allowed_methods=...)` API. +- CI is split into a `quality` job (ruff and mypy on one interpreter) and a + `test` matrix. Current mypy releases cannot target Python 3.8, and type + checking does not need to repeat across every interpreter. +- `--cov=tappay` removed from pytest `addopts`, so a bare `pytest` no longer + fails when `pytest-cov` is absent. Coverage is requested explicitly in CI. +- README badge corrected from black to ruff, which is what the project uses. +- Test coverage is now 100% (223 statements), up from 92%. + +### Upgrade notes +- **If you mock `tappay.client.requests.post` in your tests, those mocks will no + longer intercept anything.** The client now issues requests through + `client.session.post`; patch that instead. +- `Models.Currencies` changed from a plain class to a `str` enum. Members still + compare equal to their string form and still serialize as `"TWD"` through + `json.dumps`, but `str()` and f-strings render them as `Currencies.TWD` on + Python 3.11+. Never interpolate a member into a request payload. + ## [0.6.1] - 2026-08-18 ### Security diff --git a/README.md b/README.md index 58843e3..d192b33 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ ![CI](https://github.com/shihweilo/tappay-python/workflows/CI/badge.svg) [![PyPI version](https://badge.fury.io/py/tappay.svg)](https://badge.fury.io/py/tappay) [![Python Versions](https://img.shields.io/pypi/pyversions/tappay.svg)](https://pypi.org/project/tappay/) -[![Code style: black](https://img.shields.io/badge/code%20style-black-000000.svg)](https://github.com/psf/black) +[![Code style: ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) > [!IMPORTANT] @@ -16,6 +16,9 @@ > [!NOTE] > **Typed**: As of version 0.6.1, this package ships a `py.typed` marker (PEP 561), so mypy, Pyright, and your IDE will use the library's own type hints. +> [!NOTE] +> **Connection reuse**: As of version 0.7.0, the client holds a pooled `requests.Session`, so repeated calls reuse an established TLS connection instead of renegotiating one each time. Close it with `client.close()` or use the client as a context manager. + This is the unofficial Python client library for TapPay's Backend API. To use it you'll need a TapPay account. Sign up at [tappaysdk.com](https://www.tappaysdk.com). ## Installation @@ -121,6 +124,72 @@ response = client.refund( ) ``` +### Currencies + +Payments settle in TWD by default. Pass `currency` to override it, using either a +`Models.Currencies` member or a plain currency string: + +```python +response = client.pay_by_prime( + prime="prime_token_from_frontend", + amount=100, + details="Order #123", + card_holder_data=card_holder, + currency=tappay.Models.Currencies.USD, +) +``` + +### Handling failures + +TapPay reports business failures such as a declined card with an HTTP 200 and a +non-zero `status` in the response body, so they are invisible to HTTP-level error +handling. By default the response is returned as-is and it is your job to check: + +```python +response = client.pay_by_prime(...) +if response["status"] != 0: + ... # declined, invalid argument, insufficient balance, and so on +``` + +Pass `raise_on_error=True` to have the client raise `TapPayError` instead: + +```python +client = tappay.Client(is_sandbox=False, raise_on_error=True) + +try: + response = client.pay_by_prime(...) +except tappay.TapPayError as exc: + print(exc.status, exc.msg, exc.response["rec_trade_id"]) +``` + +The exception hierarchy is: + +| Exception | Raised when | +| --- | --- | +| `AuthenticationError` | HTTP 401 | +| `ClientError` | HTTP 4xx | +| `ServerError` | HTTP 5xx, or an unexpected status code | +| `TapPayError` | Non-zero `status` in a 2xx body (only with `raise_on_error=True`) | + +All of them subclass `tappay.Error`. + +### Connection reuse and retries + +Each client owns a pooled session. Close it when you are done, or use the client +as a context manager: + +```python +with tappay.Client(is_sandbox=False) as client: + client.pay_by_prime(...) +``` + +The read-only query endpoints (`get_records`, `get_trade_history`) retry twice on +connection errors and on HTTP 429/500/502/503/504, with exponential backoff. +Payment, refund, capture, bind, and remove endpoints are **never** retried +automatically: TapPay exposes no idempotency key, so retrying a request that +actually succeeded upstream would charge the cardholder twice. Tune the retry +count with `max_retries=`, or disable it with `max_retries=0`. + For more API details, please refer to the [TapPay Backend API Documentation](https://docs.tappaysdk.com/tutorial/zh/back.html). ## Development @@ -139,8 +208,7 @@ cd tappay-python ```bash python3 -m venv .venv source .venv/bin/activate # On Windows: .venv\Scripts\activate -pip install -e . -pip install pytest pytest-cov ruff +pip install -e ".[dev]" ``` ### Testing @@ -154,7 +222,13 @@ pytest Run tests with coverage: ```bash -pytest --cov=tappay +pytest --cov=tappay --cov-report=term-missing +``` + +Type check with mypy: + +```bash +mypy tappay ``` ### Linting and Formatting diff --git a/pyproject.toml b/pyproject.toml index 6f4b298..bd6b9d2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tappay" -version = "0.6.1" +version = "0.7.0" authors = [ { name="Shih Wei Chris Lo", email="shihwei@gmail.com" }, ] @@ -18,6 +18,7 @@ classifiers = [ "Programming Language :: Python :: 3.10", "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", "License :: OSI Approved :: MIT License", "Operating System :: OS Independent", "Development Status :: 5 - Production/Stable", @@ -25,11 +26,21 @@ classifiers = [ "Topic :: Software Development :: Libraries :: Python Modules", ] dependencies = [ - "requests>=2.4.2", + "requests>=2.26", + "urllib3>=1.26", "pydantic>=2.0", "email-validator>=2.0", ] +[project.optional-dependencies] +dev = [ + "pytest>=7.0", + "pytest-cov>=4.0", + "ruff>=0.5", + "mypy>=1.8", + "types-requests", +] + [project.urls] "Homepage" = "https://github.com/shihweilo/tappay-python" "Bug Tracker" = "https://github.com/shihweilo/tappay-python/issues" @@ -58,7 +69,15 @@ docstring-code-format = true [tool.pytest.ini_options] minversion = "6.0" -addopts = "-ra -q --cov=tappay" +addopts = "-ra -q" testpaths = [ "tests", ] + +[tool.mypy] +warn_unused_configs = true +warn_redundant_casts = true +warn_unused_ignores = true +warn_return_any = true +disallow_untyped_defs = true +strict_equality = true diff --git a/tappay/__init__.py b/tappay/__init__.py index 0d0b873..79ac178 100644 --- a/tappay/__init__.py +++ b/tappay/__init__.py @@ -6,6 +6,7 @@ Error, Exceptions, ServerError, + TapPayError, ) from tappay.models import Models @@ -18,4 +19,5 @@ "ClientError", "ServerError", "AuthenticationError", + "TapPayError", ] diff --git a/tappay/client.py b/tappay/client.py index 36d907c..30e0180 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -1,9 +1,11 @@ import logging import os from platform import python_version -from typing import Any, Dict, Optional, Tuple, Union +from typing import Any, Dict, Optional, Tuple, Union, cast import requests +from requests.adapters import HTTPAdapter +from urllib3.util.retry import Retry from tappay._version import __version__ from tappay.exceptions import Exceptions @@ -23,6 +25,25 @@ #: "block indefinitely" and is strongly discouraged for server-side use. TimeoutType = Union[float, Tuple[float, float], None] +#: Endpoints that only read state and are therefore safe to retry. Payment, +#: refund, capture, bind and remove endpoints are deliberately excluded: TapPay +#: exposes no idempotency key, so a retried request that actually succeeded +#: upstream (but whose response was lost) would charge the cardholder twice. +RETRYABLE_PATHS: Tuple[str, ...] = ( + "/tpc/transaction/query", + "/tpc/transaction/trade-history", +) + +#: Transient conditions worth a second attempt on the read-only endpoints. +RETRY_STATUS_FORCELIST: Tuple[int, ...] = (429, 500, 502, 503, 504) + +#: Total retries attempted on the read-only endpoints, beyond the first try. +DEFAULT_MAX_RETRIES = 2 + +#: ``status`` value TapPay returns on success. Anything else is a failure +#: reported with an HTTP 200, which is why it needs explicit handling. +SUCCESS_STATUS = 0 + class _Unset: """Sentinel distinguishing "argument omitted" from an explicit ``None``.""" @@ -102,6 +123,8 @@ def __init__( app_name: Optional[str] = None, app_version: Optional[str] = None, timeout: TimeoutType = DEFAULT_TIMEOUT, + raise_on_error: bool = False, + max_retries: int = DEFAULT_MAX_RETRIES, ): """ Create a Client object to start making calls to TapPay APIs. @@ -114,6 +137,13 @@ def __init__( :param timeout: Default request timeout in seconds, as a float or a ``(connect, read)`` tuple. Passing ``None`` disables the timeout and lets a stalled request block the calling thread forever. + :param bool raise_on_error: When ``True``, raise + :class:`~tappay.exceptions.TapPayError` if TapPay reports a non-zero + ``status`` in the response body. Defaults to ``False`` for backward + compatibility, which means business failures such as a declined card + are returned as an ordinary dict and are easy to miss. + :param int max_retries: Retries for the read-only query endpoints. Write + endpoints are never retried; see :data:`RETRYABLE_PATHS`. """ if not isinstance(is_sandbox, bool): raise TypeError( @@ -129,6 +159,7 @@ def __init__( raise ValueError("Missing required value for `merchant_id`") self.timeout = timeout + self.raise_on_error = raise_on_error subdomain = "sandbox" if is_sandbox else "prod" self.api_host = f"{subdomain}.tappaysdk.com" @@ -144,6 +175,54 @@ def __init__( "x-api-key": self.partner_key, } + self.session = self._build_session(max_retries) + + def _build_session(self, max_retries: int) -> requests.Session: + """Create the pooled session and mount its retry policy. + + A single session keeps connections alive between calls, so each request + reuses an established TLS connection instead of paying for a fresh + handshake. Retries are mounted per-URL rather than session-wide: + ``requests`` resolves adapters by longest matching prefix, so the + read-only endpoints pick up the retrying adapter while everything else + falls back to the non-retrying one. + """ + session = requests.Session() + + no_retry = HTTPAdapter(max_retries=Retry(total=0, read=False)) + session.mount("https://", no_retry) + session.mount("http://", no_retry) + + if max_retries > 0: + retry = Retry( + total=max_retries, + connect=max_retries, + read=max_retries, + status=max_retries, + backoff_factor=0.5, + status_forcelist=RETRY_STATUS_FORCELIST, + # TapPay's read APIs are POST, which urllib3 excludes by default + # because POST is not idempotent in general. It is safe here + # only because RETRYABLE_PATHS is restricted to queries. + allowed_methods=frozenset({"POST"}), + raise_on_status=False, + ) + retry_adapter = HTTPAdapter(max_retries=retry) + for path in RETRYABLE_PATHS: + session.mount(f"https://{self.api_host}{path}", retry_adapter) + + return session + + def close(self) -> None: + """Close the underlying session and release pooled connections.""" + self.session.close() + + def __enter__(self) -> "Client": + return self + + def __exit__(self, *exc_info: Any) -> None: + self.close() + def pay_by_prime( self, prime: str, @@ -151,12 +230,16 @@ def pay_by_prime( details: str, card_holder_data: Models.CardHolderData, *, + currency: Union[str, Models.Currencies] = Models.Currencies.TWD, timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, ) -> Optional[Dict[str, Any]]: """ Make a payment using "prime" obtained from TapPay frontend SDK Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#pay-by-prime-api + + :param currency: Settlement currency, defaulting to TWD. Accepts a + :class:`Models.Currencies` member or a plain currency string. """ if not isinstance(card_holder_data, Models.CardHolderData): raise TypeError( @@ -167,7 +250,7 @@ def pay_by_prime( params = { "prime": prime, "amount": amount, - "currency": Models.Currencies.TWD, + "currency": currency, "details": details, "cardholder": card_holder_data.to_dict(), } @@ -186,18 +269,22 @@ def pay_by_token( amount: int, details: str, *, + currency: Union[str, Models.Currencies] = Models.Currencies.TWD, timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, ) -> Optional[Dict[str, Any]]: """ Make a payment using previously obtained card secrets (key & token) Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#pay-by-card-token-api + + :param currency: Settlement currency, defaulting to TWD. Accepts a + :class:`Models.Currencies` member or a plain currency string. """ params = { "card_key": card_key, "card_token": card_token, "amount": amount, - "currency": Models.Currencies.TWD, + "currency": currency, "details": details, } @@ -311,12 +398,16 @@ def bind_card( prime: str, card_holder_data: Models.CardHolderData, *, + currency: Union[str, Models.Currencies] = Models.Currencies.TWD, timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, ) -> Optional[Dict[str, Any]]: """ Bind new credit card Ref: https://docs.tappaysdk.com/tutorial/zh/advanced.html#bind-card-api + + :param currency: Settlement currency, defaulting to TWD. Accepts a + :class:`Models.Currencies` member or a plain currency string. """ if not isinstance(card_holder_data, Models.CardHolderData): raise TypeError( @@ -326,7 +417,7 @@ def bind_card( params = { "prime": prime, - "currency": Models.Currencies.TWD, + "currency": currency, "cardholder": card_holder_data.to_dict(), } @@ -414,7 +505,7 @@ def __post( logger.debug("POST params: %s", _redact(params)) logger.debug("POST timeout: %s", effective_timeout) - response = requests.post( + response = self.session.post( uri, json=params, headers=self.headers, timeout=effective_timeout ) return self.__parse(response) @@ -430,7 +521,11 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: elif response.status_code == 204: return None elif 200 <= response.status_code < 300: - return response.json() + # TapPay documents a JSON object for every 2xx; the cast records + # that assumption rather than widening the public return type. + data = cast(Optional[Dict[str, Any]], response.json()) + self.__raise_for_body_status(data) + return data elif 400 <= response.status_code < 500: message = f"{response.status_code} response from {self.api_host}" raise Exceptions.ClientError(message) @@ -441,3 +536,19 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: # Fallback for unexpected status codes message = f"Unexpected status code {response.status_code} from {self.api_host}" raise Exceptions.ServerError(message) + + def __raise_for_body_status(self, data: Any) -> None: + """Raise if TapPay reported a failure inside a 2xx response body. + + No-op unless the client was built with ``raise_on_error=True``. A body + without a ``status`` field is left alone, so responses that do not follow + the documented envelope are passed through to the caller untouched. + """ + if not self.raise_on_error or not isinstance(data, dict): + return + + status = data.get("status") + if status is None or status == SUCCESS_STATUS: + return + + raise Exceptions.TapPayError(status, data.get("msg"), data) diff --git a/tappay/exceptions.py b/tappay/exceptions.py index 3895a86..157b218 100644 --- a/tappay/exceptions.py +++ b/tappay/exceptions.py @@ -1,3 +1,6 @@ +from typing import Any, Dict, Optional + + class Error(Exception): """Base exception for all TapPay errors.""" @@ -22,6 +25,32 @@ class AuthenticationError(ClientError): pass +class TapPayError(Error): + """Error raised when TapPay reports a failure in the response body. + + TapPay signals business-level failures (declined cards, invalid arguments, + exhausted balances) with an HTTP 200 and a non-zero ``status`` field, so + these are invisible to HTTP-level error handling. Raised only when the + client is constructed with ``raise_on_error=True``. + + :ivar status: The non-zero ``status`` code returned by TapPay. + :ivar msg: The human-readable ``msg`` field returned by TapPay. + :ivar response: The full decoded response body, for callers that need + fields such as ``rec_trade_id`` or ``bank_result_code``. + """ + + def __init__( + self, + status: Any, + msg: Optional[str] = None, + response: Optional[Dict[str, Any]] = None, + ): + self.status = status + self.msg = msg + self.response = response if response is not None else {} + super().__init__(f"TapPay API error (status {status}): {msg}") + + class Exceptions: """Namespace for TapPay exceptions (for backward compatibility).""" @@ -29,3 +58,4 @@ class Exceptions: ClientError = ClientError ServerError = ServerError AuthenticationError = AuthenticationError + TapPayError = TapPayError diff --git a/tappay/models.py b/tappay/models.py index e61a652..a0296a5 100644 --- a/tappay/models.py +++ b/tappay/models.py @@ -1,3 +1,4 @@ +from enum import Enum from typing import Optional from pydantic import BaseModel, EmailStr, Field @@ -6,10 +7,33 @@ class Models: """Namespace for TapPay models.""" - class Currencies: - """Currency constants.""" + class Currencies(str, Enum): + """Currencies accepted by the TapPay APIs. + + A ``str``-backed enum, so members compare equal to their plain-string + form and serialize as ``"TWD"`` through ``json.dumps``. Note that + ``str()`` and f-strings render these as ``Currencies.TWD`` on Python + 3.11+, so never interpolate a member into a request payload. + + Methods accepting a currency also accept a plain string, so a currency + that TapPay adds before this list is updated remains usable. + """ TWD = "TWD" + USD = "USD" + JPY = "JPY" + HKD = "HKD" + GBP = "GBP" + AUD = "AUD" + EUR = "EUR" + CNY = "CNY" + KRW = "KRW" + SGD = "SGD" + MYR = "MYR" + THB = "THB" + PHP = "PHP" + IDR = "IDR" + VND = "VND" class CardHolderData(BaseModel): """Card holder data model using Pydantic v2. diff --git a/tests/conftest.py b/tests/conftest.py index 467cc12..8f2fc51 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,4 +1,8 @@ +from contextlib import contextmanager +from unittest.mock import Mock, patch + import pytest +import requests from tappay.client import Client, Models @@ -20,3 +24,25 @@ def card_holder(): return Models.CardHolderData( phone_number="0912345678", name="Wang Xiao Ming", email="test@example.com" ) + + +@contextmanager +def mock_post(client, status_code=200, payload=None): + """Patch a client's pooled session and yield the mocked ``post``. + + Patching the session rather than ``requests.post`` keeps these tests + honest about the transport the client actually uses. + """ + response = Mock(spec=requests.Response) + response.status_code = status_code + response.json.return_value = {"status": 0} if payload is None else payload + response.content = b"{}" + with patch.object(client.session, "post", return_value=response) as mocked: + yield mocked + + +@pytest.fixture +def post(sandbox_client): + """Mocked transport for the sandbox client, for the common 200 case.""" + with mock_post(sandbox_client) as mocked: + yield mocked diff --git a/tests/test_client.py b/tests/test_client.py index 0552825..32e648b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,152 +1,424 @@ import importlib.metadata +import json import logging import pathlib from unittest.mock import Mock, patch import pytest +import requests import tappay -from tappay.client import DEFAULT_TIMEOUT, VERSION, Client, Models, _redact -from tappay.exceptions import AuthenticationError, ClientError, ServerError +from tappay.client import ( + DEFAULT_MAX_RETRIES, + DEFAULT_TIMEOUT, + RETRYABLE_PATHS, + VERSION, + Client, + Models, + _redact, +) +from tappay.exceptions import ( + AuthenticationError, + ClientError, + ServerError, + TapPayError, +) +from tests.conftest import mock_post +WRITE_PATHS = ( + "/tpc/payment/pay-by-prime", + "/tpc/payment/pay-by-token", + "/tpc/transaction/refund", + "/tpc/transaction/refund/cancel", + "/tpc/transaction/cap", + "/tpc/transaction/cap/cancel", + "/tpc/card/bind", + "/tpc/card/remove", +) -def test_client_initialization_sandbox(): - client = Client(is_sandbox=True, partner_key="pk", merchant_id="mid") - assert client.api_host == "sandbox.tappaysdk.com" - assert client.partner_key == "pk" - assert client.merchant_id == "mid" +# --- Construction --------------------------------------------------------- -def test_client_initialization_production(): - client = Client(is_sandbox=False, partner_key="pk", merchant_id="mid") - assert client.api_host == "prod.tappaysdk.com" +def test_client_initialization_sandbox(sandbox_client): + assert sandbox_client.api_host == "sandbox.tappaysdk.com" + assert sandbox_client.partner_key == "partner_key" + assert sandbox_client.merchant_id == "merchant_id" -def test_pay_by_prime(sandbox_client, card_holder): - with patch("tappay.client.requests.post") as mock_post: - mock_response = Mock() - mock_response.status_code = 200 - mock_response.json.return_value = {"status": 0, "msg": "Success"} - mock_post.return_value = mock_response - response = sandbox_client.pay_by_prime( - prime="test_prime", amount=100, details="test", card_holder_data=card_holder - ) +def test_client_initialization_production(production_client): + assert production_client.api_host == "prod.tappaysdk.com" + + +def test_is_sandbox_must_be_a_bool(): + with pytest.raises(TypeError, match="expected bool"): + Client(is_sandbox="yes", partner_key="pk", merchant_id="mid") + + +def test_credentials_fall_back_to_environment(monkeypatch): + monkeypatch.setenv("TAPPAY_PARTNER_KEY", "env_pk") + monkeypatch.setenv("TAPPAY_MERCHANT_ID", "env_mid") + + client = Client(is_sandbox=True) + + assert client.partner_key == "env_pk" + assert client.merchant_id == "env_mid" + + +def test_explicit_credentials_win_over_environment(monkeypatch): + monkeypatch.setenv("TAPPAY_PARTNER_KEY", "env_pk") + monkeypatch.setenv("TAPPAY_MERCHANT_ID", "env_mid") + + client = Client(is_sandbox=True, partner_key="arg_pk", merchant_id="arg_mid") - assert response["status"] == 0 - mock_post.assert_called_once() - args, kwargs = mock_post.call_args - assert kwargs["json"]["prime"] == "test_prime" - assert kwargs["json"]["amount"] == 100 - assert kwargs["json"]["currency"] == "TWD" + assert client.partner_key == "arg_pk" + assert client.merchant_id == "arg_mid" -def test_pay_by_prime_invalid_cardholder(sandbox_client): - with pytest.raises(TypeError): +def test_missing_partner_key_is_rejected(monkeypatch): + monkeypatch.delenv("TAPPAY_PARTNER_KEY", raising=False) + with pytest.raises(ValueError, match="partner_key"): + Client(is_sandbox=True, merchant_id="mid") + + +def test_missing_merchant_id_is_rejected(monkeypatch): + monkeypatch.delenv("TAPPAY_MERCHANT_ID", raising=False) + with pytest.raises(ValueError, match="merchant_id"): + Client(is_sandbox=True, partner_key="pk") + + +def test_partner_key_is_sent_as_the_api_key_header(sandbox_client): + assert sandbox_client.headers["x-api-key"] == "partner_key" + assert sandbox_client.headers["Content-Type"] == "application/json" + + +# --- Request shape, per endpoint ----------------------------------------- + + +def test_pay_by_prime(sandbox_client, card_holder, post): + response = sandbox_client.pay_by_prime( + prime="test_prime", amount=100, details="test", card_holder_data=card_holder + ) + + assert response["status"] == 0 + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/payment/pay-by-prime" + assert body["prime"] == "test_prime" + assert body["amount"] == 100 + assert body["currency"] == "TWD" + assert body["details"] == "test" + assert body["cardholder"] == card_holder.to_dict() + # This endpoint is authenticated with both credentials. + assert body["partner_key"] == "partner_key" + assert body["merchant_id"] == "merchant_id" + + +def test_pay_by_prime_rejects_a_non_model_cardholder(sandbox_client): + with pytest.raises(TypeError, match="CardHolderData"): sandbox_client.pay_by_prime( prime="p", amount=100, details="d", card_holder_data={} ) -def test_api_error_handling_401(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_response = Mock() - mock_response.status_code = 401 - mock_post.return_value = mock_response +def test_pay_by_token(sandbox_client, post): + sandbox_client.pay_by_token( + card_key="ck", card_token="ct", amount=250, details="Subscription" + ) + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/payment/pay-by-token" + assert body["card_key"] == "ck" + assert body["card_token"] == "ct" + assert body["amount"] == 250 + assert body["details"] == "Subscription" + assert body["merchant_id"] == "merchant_id" + + +def test_refund(sandbox_client, post): + sandbox_client.refund(rec_trade_id="rec", amount=100) + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/transaction/refund" + assert body == {"rec_trade_id": "rec", "amount": 100, "partner_key": "partner_key"} + # Refunds authenticate with the partner key alone. + assert "merchant_id" not in body + + +def test_cancel_refund(sandbox_client, post): + sandbox_client.cancel_refund("rec") + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/transaction/refund/cancel" + assert body["rec_trade_id"] == "rec" + + +def test_capture_today(sandbox_client, post): + sandbox_client.capture_today("rec") + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/transaction/cap" + assert body["rec_trade_id"] == "rec" + + +def test_cancel_capture(sandbox_client, post): + sandbox_client.cancel_capture("rec") + + assert ( + post.call_args.args[0] + == "https://sandbox.tappaysdk.com/tpc/transaction/cap/cancel" + ) + + +def test_get_trade_history(sandbox_client, post): + sandbox_client.get_trade_history("rec") + + assert ( + post.call_args.args[0] + == "https://sandbox.tappaysdk.com/tpc/transaction/trade-history" + ) + + +def test_get_records_defaults(sandbox_client, post): + sandbox_client.get_records({"time": {"start_time": 1}}) - with pytest.raises(AuthenticationError): + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/transaction/query" + assert body["filters"] == {"time": {"start_time": 1}} + assert body["page"] == 0 + assert body["records_per_page"] == 50 + assert "order_by" not in body + + +def test_get_records_with_pagination_and_ordering(sandbox_client, post): + sandbox_client.get_records( + {"time": {}}, page=3, records_per_page=10, order_by_dict={"attribute": "time"} + ) + + body = post.call_args.kwargs["json"] + assert body["page"] == 3 + assert body["records_per_page"] == 10 + assert body["order_by"] == {"attribute": "time"} + + +def test_bind_card(sandbox_client, card_holder, post): + sandbox_client.bind_card(prime="p", card_holder_data=card_holder) + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/card/bind" + assert body["prime"] == "p" + assert body["cardholder"] == card_holder.to_dict() + assert body["merchant_id"] == "merchant_id" + + +def test_bind_card_rejects_a_non_model_cardholder(sandbox_client): + with pytest.raises(TypeError, match="CardHolderData"): + sandbox_client.bind_card(prime="p", card_holder_data={"name": "x"}) + + +def test_remove_card(sandbox_client, post): + sandbox_client.remove_card("ck", "ct") + + uri, body = post.call_args.args[0], post.call_args.kwargs["json"] + assert uri == "https://sandbox.tappaysdk.com/tpc/card/remove" + assert body["card_key"] == "ck" + assert body["card_token"] == "ct" + + +def test_extra_kwargs_are_merged_into_the_request_body(sandbox_client, post): + sandbox_client.refund("rec", 100, bank_refund_id="BR1") + + assert post.call_args.kwargs["json"]["bank_refund_id"] == "BR1" + + +def test_production_client_targets_the_production_host(production_client): + with mock_post(production_client) as mocked: + production_client.capture_today("rec") + + assert mocked.call_args.args[0].startswith("https://prod.tappaysdk.com/") + + +# --- Currency ------------------------------------------------------------- + + +def test_currency_defaults_to_twd(sandbox_client, card_holder, post): + sandbox_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder + ) + + assert post.call_args.kwargs["json"]["currency"] == "TWD" + + +@pytest.mark.parametrize( + "currency", [Models.Currencies.USD, "USD"], ids=["enum", "plain-string"] +) +def test_currency_can_be_overridden(sandbox_client, card_holder, post, currency): + sandbox_client.pay_by_prime( + prime="p", + amount=1, + details="d", + card_holder_data=card_holder, + currency=currency, + ) + + assert post.call_args.kwargs["json"]["currency"] == "USD" + + +def test_currency_applies_to_token_payments_and_card_binding( + sandbox_client, card_holder, post +): + sandbox_client.pay_by_token( + card_key="ck", card_token="ct", amount=1, details="d", currency="JPY" + ) + assert post.call_args.kwargs["json"]["currency"] == "JPY" + + sandbox_client.bind_card( + prime="p", card_holder_data=card_holder, currency=Models.Currencies.HKD + ) + assert post.call_args.kwargs["json"]["currency"] == "HKD" + + +def test_currency_enum_serializes_to_a_plain_code(): + """Guards against a `Currencies.TWD` repr reaching the API as the value.""" + encoded = json.dumps({"currency": Models.Currencies.TWD}) + + assert encoded == '{"currency": "TWD"}' + assert Models.Currencies.TWD == "TWD" + + +# --- Session, pooling and retries ---------------------------------------- + + +def test_requests_go_through_the_pooled_session(sandbox_client, card_holder): + """The module-level `requests.post` must no longer be used.""" + with patch.object(requests, "post") as module_post: + with mock_post(sandbox_client) as session_post: sandbox_client.pay_by_prime( - prime="p", - amount=1, - details="d", - card_holder_data=Models.CardHolderData( - phone_number="p", name="n", email="e@example.com" - ), + prime="p", amount=1, details="d", card_holder_data=card_holder ) + assert session_post.call_count == 1 + module_post.assert_not_called() -def test_api_error_handling_400(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_response = Mock() - mock_response.status_code = 400 - mock_post.return_value = mock_response - with pytest.raises(ClientError): - sandbox_client.capture_today("id") +def test_session_is_reused_across_calls(sandbox_client): + session = sandbox_client.session + with mock_post(sandbox_client): + sandbox_client.capture_today("a") + sandbox_client.capture_today("b") + assert sandbox_client.session is session -def test_api_error_handling_500(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_response = Mock() - mock_response.status_code = 500 - mock_post.return_value = mock_response - with pytest.raises(ServerError): - sandbox_client.capture_today("id") +def test_read_only_endpoints_retry(sandbox_client): + for path in RETRYABLE_PATHS: + adapter = sandbox_client.session.get_adapter( + f"https://{sandbox_client.api_host}{path}" + ) + assert adapter.max_retries.total == DEFAULT_MAX_RETRIES, path + + +def test_write_endpoints_never_retry(sandbox_client): + """A retried payment could double-charge, so these must stay at zero.""" + for path in WRITE_PATHS: + adapter = sandbox_client.session.get_adapter( + f"https://{sandbox_client.api_host}{path}" + ) + assert adapter.max_retries.total == 0, path + + +def test_retry_policy_permits_post_and_targets_transient_failures(sandbox_client): + adapter = sandbox_client.session.get_adapter( + f"https://{sandbox_client.api_host}{RETRYABLE_PATHS[0]}" + ) + retry = adapter.max_retries + + assert "POST" in retry.allowed_methods + assert 503 in retry.status_forcelist + assert 400 not in retry.status_forcelist + assert retry.backoff_factor > 0 + + +def test_retries_can_be_disabled(): + client = Client(is_sandbox=True, partner_key="pk", merchant_id="mid", max_retries=0) + + for path in RETRYABLE_PATHS: + adapter = client.session.get_adapter(f"https://{client.api_host}{path}") + assert adapter.max_retries.total == 0 + + +def test_retry_mounts_are_scoped_to_this_clients_host(sandbox_client): + """Sandbox retry mounts must not leak onto the production host.""" + adapter = sandbox_client.session.get_adapter( + f"https://prod.tappaysdk.com{RETRYABLE_PATHS[0]}" + ) + + assert adapter.max_retries.total == 0 + + +def test_close_releases_the_session(sandbox_client): + with mock_post(sandbox_client): + sandbox_client.capture_today("rec") + + sandbox_client.close() # must not raise, and is safe to call again + sandbox_client.close() + + +def test_client_works_as_a_context_manager(): + client = Client(is_sandbox=True, partner_key="pk", merchant_id="mid") + with patch.object(client.session, "close") as closer: + with client as entered: + assert entered is client + closer.assert_not_called() -# --- Timeout handling (0.6.1) --------------------------------------------- + closer.assert_called_once() -def _mock_post(status_code=200, payload=None): - """Build a patched `requests.post` returning a canned response.""" - mock_response = Mock() - mock_response.status_code = status_code - mock_response.json.return_value = payload if payload is not None else {"status": 0} - return mock_response +# --- Timeout handling ----------------------------------------------------- -def test_default_timeout_is_applied(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() - sandbox_client.capture_today("id") +def test_default_timeout_is_applied(sandbox_client, post): + sandbox_client.capture_today("id") - assert mock_post.call_args.kwargs["timeout"] == DEFAULT_TIMEOUT + assert post.call_args.kwargs["timeout"] == DEFAULT_TIMEOUT def test_client_level_timeout_override(card_holder): client = Client(is_sandbox=True, partner_key="pk", merchant_id="mid", timeout=5.0) - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() + with mock_post(client) as mocked: client.pay_by_prime( prime="p", amount=1, details="d", card_holder_data=card_holder ) - assert mock_post.call_args.kwargs["timeout"] == 5.0 + assert mocked.call_args.kwargs["timeout"] == 5.0 -def test_per_call_timeout_overrides_client_default(sandbox_client, card_holder): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() - sandbox_client.pay_by_prime( - prime="p", - amount=1, - details="d", - card_holder_data=card_holder, - timeout=(1.0, 2.0), - ) +def test_per_call_timeout_overrides_client_default(sandbox_client, card_holder, post): + sandbox_client.pay_by_prime( + prime="p", + amount=1, + details="d", + card_holder_data=card_holder, + timeout=(1.0, 2.0), + ) - assert mock_post.call_args.kwargs["timeout"] == (1.0, 2.0) + assert post.call_args.kwargs["timeout"] == (1.0, 2.0) -def test_explicit_none_timeout_is_preserved(sandbox_client): +def test_explicit_none_timeout_is_preserved(sandbox_client, post): """`None` means "no timeout" and must not be swallowed by the sentinel.""" - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() - sandbox_client.capture_today("id", timeout=None) + sandbox_client.capture_today("id", timeout=None) - assert mock_post.call_args.kwargs["timeout"] is None + assert post.call_args.kwargs["timeout"] is None -def test_timeout_is_not_forwarded_as_an_api_field(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() - sandbox_client.refund("rec", 100, timeout=9.0) +def test_timeout_is_not_forwarded_as_an_api_field(sandbox_client, post): + sandbox_client.refund("rec", 100, timeout=9.0) - assert "timeout" not in mock_post.call_args.kwargs["json"] + assert "timeout" not in post.call_args.kwargs["json"] -def test_timeout_propagates_to_every_endpoint(sandbox_client, card_holder): +def test_timeout_propagates_to_every_endpoint(sandbox_client, card_holder, post): """Guard against a new method forgetting to thread `timeout` through.""" calls = [ ("pay_by_token", ("ck", "ct", 100, "d"), {}), @@ -158,25 +430,25 @@ def test_timeout_propagates_to_every_endpoint(sandbox_client, card_holder): ("remove_card", ("ck", "ct"), {}), ("cancel_refund", ("rec",), {}), ("bind_card", ("prime",), {"card_holder_data": card_holder}), + ( + "pay_by_prime", + ("prime", 1, "d"), + {"card_holder_data": card_holder}, + ), ] for method_name, args, kwargs in calls: - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() - getattr(sandbox_client, method_name)(*args, timeout=3.5, **kwargs) - - assert mock_post.call_args.kwargs["timeout"] == 3.5, method_name + getattr(sandbox_client, method_name)(*args, timeout=3.5, **kwargs) + assert post.call_args.kwargs["timeout"] == 3.5, method_name -# --- Log redaction (0.6.1) ------------------------------------------------ +# --- Log redaction -------------------------------------------------------- def test_credentials_and_pii_are_redacted_in_request_logs(caplog): caplog.set_level(logging.DEBUG, logger="tappay.client") client = Client( - is_sandbox=True, - partner_key="SECRET_PARTNER_KEY", - merchant_id="mid", + is_sandbox=True, partner_key="SECRET_PARTNER_KEY", merchant_id="mid" ) card_holder = Models.CardHolderData( phone_number="0912345678", @@ -185,8 +457,7 @@ def test_credentials_and_pii_are_redacted_in_request_logs(caplog): national_id="A123456789", ) - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post() + with mock_post(client): client.pay_by_prime( prime="SECRET_PRIME", amount=100, @@ -213,18 +484,18 @@ def test_credentials_and_pii_are_redacted_in_request_logs(caplog): def test_card_secrets_are_redacted_in_response_logs(sandbox_client, caplog): caplog.set_level(logging.DEBUG, logger="tappay.client") - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post( - payload={ - "status": 0, - "rec_trade_id": "REC123", - "card_secret": { - "card_key": "SECRET_CARD_KEY", - "card_token": "SECRET_CARD_TOKEN", - }, - "card_info": {"bin_code": "424242", "last_four": "4242"}, - } - ) + with mock_post( + sandbox_client, + payload={ + "status": 0, + "rec_trade_id": "REC123", + "card_secret": { + "card_key": "SECRET_CARD_KEY", + "card_token": "SECRET_CARD_TOKEN", + }, + "card_info": {"bin_code": "424242", "last_four": "4242"}, + }, + ): sandbox_client.capture_today("rec") for secret in ("SECRET_CARD_KEY", "SECRET_CARD_TOKEN", "424242", "4242"): @@ -237,19 +508,17 @@ def test_redaction_is_skipped_when_debug_logging_is_off(sandbox_client, caplog): """No response parsing work should happen when debug logging is disabled.""" caplog.set_level(logging.INFO, logger="tappay.client") - with patch("tappay.client.requests.post") as mock_post: - response = _mock_post() - mock_post.return_value = response + with mock_post(sandbox_client) as mocked: sandbox_client.capture_today("rec") - # Parsed once to build the return value, never for logging. - assert response.json.call_count == 1 + assert mocked.return_value.json.call_count == 1 assert caplog.text == "" def test_redact_leaves_unrelated_structures_intact(): payload = {"amount": 100, "items": [{"sku": "A1", "name": "Widget"}]} + assert _redact(payload) == { "amount": 100, "items": [{"sku": "A1", "name": "***REDACTED***"}], @@ -263,7 +532,7 @@ def test_redact_matches_keys_case_insensitively(): } -# --- Version single-sourcing (0.6.1) -------------------------------------- +# --- Version and typing metadata ----------------------------------------- def test_version_matches_installed_distribution_metadata(): @@ -288,39 +557,156 @@ def test_user_agent_includes_app_name_and_version(): app_name="MyApp", app_version="1.2.3", ) + assert client.headers["User-Agent"].endswith(" MyApp/1.2.3") +def test_user_agent_omits_app_details_when_incomplete(): + client = Client( + is_sandbox=True, partner_key="pk", merchant_id="mid", app_name="MyApp" + ) + + assert "MyApp" not in client.headers["User-Agent"] + + def test_package_ships_a_py_typed_marker(): marker = pathlib.Path(tappay.__file__).parent / "py.typed" + assert marker.is_file() -# --- Error surface (0.6.1) ------------------------------------------------ +# --- HTTP-level error handling ------------------------------------------- def test_authentication_error_carries_a_message(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post(status_code=401) - + with mock_post(sandbox_client, status_code=401): with pytest.raises(AuthenticationError) as exc_info: sandbox_client.capture_today("id") assert str(exc_info.value) == "401 response from sandbox.tappaysdk.com" -def test_no_content_response_returns_none(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post(status_code=204) +def test_client_error_on_4xx(sandbox_client): + with mock_post(sandbox_client, status_code=400): + with pytest.raises(ClientError, match="400 response"): + sandbox_client.capture_today("id") + + +def test_server_error_on_5xx(sandbox_client): + with mock_post(sandbox_client, status_code=500): + with pytest.raises(ServerError, match="500 response"): + sandbox_client.capture_today("id") + +def test_no_content_response_returns_none(sandbox_client): + with mock_post(sandbox_client, status_code=204): assert sandbox_client.capture_today("id") is None def test_unexpected_status_code_raises_server_error(sandbox_client): - with patch("tappay.client.requests.post") as mock_post: - mock_post.return_value = _mock_post(status_code=302) - - with pytest.raises(ServerError) as exc_info: + with mock_post(sandbox_client, status_code=302): + with pytest.raises(ServerError, match="302"): sandbox_client.capture_today("id") - assert "302" in str(exc_info.value) + +# --- Body-level error handling (raise_on_error) --------------------------- + + +@pytest.fixture +def strict_client(): + return Client( + is_sandbox=True, partner_key="pk", merchant_id="mid", raise_on_error=True + ) + + +def test_non_zero_status_is_returned_silently_by_default(sandbox_client): + """The historical behaviour: a declined card looks like any other result.""" + with mock_post(sandbox_client, payload={"status": 3, "msg": "Card declined"}): + response = sandbox_client.capture_today("rec") + + assert response == {"status": 3, "msg": "Card declined"} + + +def test_non_zero_status_raises_when_strict(strict_client): + payload = {"status": 3, "msg": "Card declined", "rec_trade_id": "REC1"} + with mock_post(strict_client, payload=payload): + with pytest.raises(TapPayError) as exc_info: + strict_client.capture_today("rec") + + error = exc_info.value + assert error.status == 3 + assert error.msg == "Card declined" + assert error.response["rec_trade_id"] == "REC1" + assert "Card declined" in str(error) + + +def test_success_status_passes_through_when_strict(strict_client): + with mock_post(strict_client, payload={"status": 0, "msg": "Success"}): + assert strict_client.capture_today("rec")["status"] == 0 + + +def test_body_without_a_status_field_is_left_alone_when_strict(strict_client): + with mock_post(strict_client, payload={"records": []}): + assert strict_client.capture_today("rec") == {"records": []} + + +def test_non_dict_body_is_left_alone_when_strict(strict_client): + with mock_post(strict_client, payload=[1, 2, 3]): + assert strict_client.capture_today("rec") == [1, 2, 3] + + +def test_no_content_response_is_unaffected_when_strict(strict_client): + with mock_post(strict_client, status_code=204): + assert strict_client.capture_today("rec") is None + + +def test_tappay_error_is_catchable_as_the_base_error(strict_client): + with mock_post(strict_client, payload={"status": 3, "msg": "nope"}): + with pytest.raises(tappay.Error): + strict_client.capture_today("rec") + + +# --- Robustness of the logging path -------------------------------------- + + +def test_non_json_response_body_is_logged_without_crashing(sandbox_client, caplog): + """An HTML error page must not break the debug logger.""" + caplog.set_level(logging.DEBUG, logger="tappay.client") + + response = Mock(spec=requests.Response) + response.status_code = 204 + response.json.side_effect = ValueError("no JSON object could be decoded") + response.content = b"Gateway Timeout" + + with patch.object(sandbox_client.session, "post", return_value=response): + assert sandbox_client.capture_today("rec") is None + + assert "28 bytes, unparsed" in caplog.text + assert "Gateway Timeout" not in caplog.text + + +def test_unset_sentinel_has_a_readable_repr(): + from tappay.client import _UNSET + + assert repr(_UNSET) == "" + + +@pytest.mark.parametrize( + ("method_name", "args", "kwargs"), + [ + ("pay_by_prime", ("p", 1, "d"), {"card_holder_data": None}), + ("pay_by_token", ("ck", "ct", 1, "d"), {}), + ("bind_card", ("p",), {"card_holder_data": None}), + ("cancel_refund", ("rec",), {}), + ("refund", ("rec", 1), {}), + ], +) +def test_extra_kwargs_reach_the_body_on_every_method( + sandbox_client, card_holder, post, method_name, args, kwargs +): + if "card_holder_data" in kwargs: + kwargs["card_holder_data"] = card_holder + + getattr(sandbox_client, method_name)(*args, three_domain_secure=True, **kwargs) + + assert post.call_args.kwargs["json"]["three_domain_secure"] is True From 84076f63f538c9584415c1de74b4e2bccbfa5d42 Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Tue, 18 Aug 2026 09:11:25 +0800 Subject: [PATCH 4/5] Raise InvalidResponseError on a malformed 2xx body An intermediary such as a proxy, load balancer or WAF can answer with an HTML error page under a 2xx status, and an empty body decodes no better. Either case previously escaped as a bare json.JSONDecodeError raised from inside requests, with nothing to indicate which host or call produced it. Decode defensively and convert the failure into InvalidResponseError, reporting the status code, host, content type and body length, and keeping the original decode error as __cause__. It subclasses ServerError so that existing `except ServerError` handlers continue to catch it. The failure message deliberately omits the body. Exception text routinely reaches logs and error trackers, and an unparseable body cannot be redacted by key the way a JSON payload can, so a truncated preview would reopen the leak that 0.6.1 closed. A test asserts a malformed body containing a card token never appears in the message. Folded into the unreleased 0.7.0 rather than a new version. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 11 +++++- README.md | 7 ++++ tappay/__init__.py | 2 + tappay/client.py | 28 ++++++++++++-- tappay/exceptions.py | 16 ++++++++ tests/conftest.py | 17 ++++++++ tests/test_client.py | 92 +++++++++++++++++++++++++++++++++++++++++++- 7 files changed, 168 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01cbbda..42b7c8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 keyword-only `currency`, defaulting to TWD as before. `Models.Currencies` is now a `str`-backed enum covering 15 currencies, and plain strings are accepted so a newly supported currency is usable before this list catches up. +- **`InvalidResponseError`.** A 2xx response whose body is not valid JSON (an + HTML error page from a proxy or WAF, or an empty body) previously surfaced as + a bare `json.JSONDecodeError` from inside `requests`, with no indication of + which host or call produced it. It now raises `InvalidResponseError`, carrying + the status, host, content type and body length, and preserving the original + decode error as `__cause__`. It subclasses `ServerError`, so existing handlers + keep working. The body itself is never included in the message, because + exception text routinely reaches logs and error trackers and an unparseable + body cannot be redacted by key. - `dev` optional dependency group (`pip install -e ".[dev]"`), a `[tool.mypy]` configuration, and a mypy step in CI. - Python 3.13 added to the CI test matrix and the package classifiers. @@ -40,7 +49,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `--cov=tappay` removed from pytest `addopts`, so a bare `pytest` no longer fails when `pytest-cov` is absent. Coverage is requested explicitly in CI. - README badge corrected from black to ruff, which is what the project uses. -- Test coverage is now 100% (223 statements), up from 92%. +- Test coverage is now 100% (235 statements), up from 92%. ### Upgrade notes - **If you mock `tappay.client.requests.post` in your tests, those mocks will no diff --git a/README.md b/README.md index d192b33..71dd333 100644 --- a/README.md +++ b/README.md @@ -169,10 +169,17 @@ The exception hierarchy is: | `AuthenticationError` | HTTP 401 | | `ClientError` | HTTP 4xx | | `ServerError` | HTTP 5xx, or an unexpected status code | +| `InvalidResponseError` | A 2xx body that is not valid JSON (subclasses `ServerError`) | | `TapPayError` | Non-zero `status` in a 2xx body (only with `raise_on_error=True`) | All of them subclass `tappay.Error`. +An intermediary such as a proxy or WAF can answer with an HTML error page under a +2xx status. That previously surfaced as a bare `json.JSONDecodeError` from inside +`requests`; it now raises `InvalidResponseError`, reporting the status, host, +content type, and body length. The body itself is deliberately left out of the +message, since exception text tends to end up in logs and error trackers. + ### Connection reuse and retries Each client owns a pooled session. Close it when you are done, or use the client diff --git a/tappay/__init__.py b/tappay/__init__.py index 79ac178..269eafc 100644 --- a/tappay/__init__.py +++ b/tappay/__init__.py @@ -5,6 +5,7 @@ ClientError, Error, Exceptions, + InvalidResponseError, ServerError, TapPayError, ) @@ -18,6 +19,7 @@ "Error", "ClientError", "ServerError", + "InvalidResponseError", "AuthenticationError", "TapPayError", ] diff --git a/tappay/client.py b/tappay/client.py index 30e0180..dbe6f5f 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -521,9 +521,7 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: elif response.status_code == 204: return None elif 200 <= response.status_code < 300: - # TapPay documents a JSON object for every 2xx; the cast records - # that assumption rather than widening the public return type. - data = cast(Optional[Dict[str, Any]], response.json()) + data = self.__decode_json(response) self.__raise_for_body_status(data) return data elif 400 <= response.status_code < 500: @@ -537,6 +535,30 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: message = f"Unexpected status code {response.status_code} from {self.api_host}" raise Exceptions.ServerError(message) + def __decode_json(self, response: requests.Response) -> Optional[Dict[str, Any]]: + """Decode a 2xx body, converting a decode failure into a typed error. + + The failure message deliberately reports only the content type and + length, never the body itself: exception text routinely ends up in logs + and error trackers, and an unparseable body cannot be redacted by key + the way a JSON payload can. + """ + try: + # TapPay documents a JSON object for every 2xx; the cast records + # that assumption rather than widening the public return type. + return cast(Optional[Dict[str, Any]], response.json()) + except ValueError as exc: + content_type = response.headers.get("Content-Type", "unknown") + try: + length: Any = len(response.content) + except TypeError: # pragma: no cover - non-standard response object + length = "unknown" + message = ( + f"Malformed JSON in {response.status_code} response from " + f"{self.api_host} (content-type: {content_type}, {length} bytes)" + ) + raise Exceptions.InvalidResponseError(message) from exc + def __raise_for_body_status(self, data: Any) -> None: """Raise if TapPay reported a failure inside a 2xx response body. diff --git a/tappay/exceptions.py b/tappay/exceptions.py index 157b218..e0719e4 100644 --- a/tappay/exceptions.py +++ b/tappay/exceptions.py @@ -19,6 +19,21 @@ class ServerError(Error): pass +class InvalidResponseError(ServerError): + """Error raised when a 2xx response body is not valid JSON. + + An intermediary (proxy, load balancer, WAF) can answer with an HTML error + page under a 2xx status, and an empty body decodes no better. Without this, + such a response surfaced as a bare ``json.JSONDecodeError`` from deep inside + ``requests``, which gave no indication of which host or call produced it. + + Subclasses :class:`ServerError`, so existing handlers keep catching it. + The originating decode error is preserved as ``__cause__``. + """ + + pass + + class AuthenticationError(ClientError): """Error raised when authentication fails.""" @@ -57,5 +72,6 @@ class Exceptions: Error = Error ClientError = ClientError ServerError = ServerError + InvalidResponseError = InvalidResponseError AuthenticationError = AuthenticationError TapPayError = TapPayError diff --git a/tests/conftest.py b/tests/conftest.py index 8f2fc51..8e399db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -41,6 +41,23 @@ def mock_post(client, status_code=200, payload=None): yield mocked +@contextmanager +def mock_bad_json( + client, + status_code=200, + content=b"504 Gateway Time-out", + content_type="text/html", +): + """Patch the transport to return a 2xx whose body is not JSON.""" + response = Mock(spec=requests.Response) + response.status_code = status_code + response.json.side_effect = ValueError("Expecting value: line 1 column 1") + response.content = content + response.headers = {"Content-Type": content_type} + with patch.object(client.session, "post", return_value=response) as mocked: + yield mocked + + @pytest.fixture def post(sandbox_client): """Mocked transport for the sandbox client, for the common 200 case.""" diff --git a/tests/test_client.py b/tests/test_client.py index 32e648b..852c05e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -20,10 +20,11 @@ from tappay.exceptions import ( AuthenticationError, ClientError, + InvalidResponseError, ServerError, TapPayError, ) -from tests.conftest import mock_post +from tests.conftest import mock_bad_json, mock_post WRITE_PATHS = ( "/tpc/payment/pay-by-prime", @@ -710,3 +711,92 @@ def test_extra_kwargs_reach_the_body_on_every_method( getattr(sandbox_client, method_name)(*args, three_domain_secure=True, **kwargs) assert post.call_args.kwargs["json"]["three_domain_secure"] is True + + +# --- Malformed 2xx bodies ------------------------------------------------- + + +def test_non_json_success_body_raises_invalid_response(sandbox_client): + """A proxy's HTML error page under a 200 must not surface as a raw + JSONDecodeError from inside requests.""" + body = b"504 Gateway Time-out" + with mock_bad_json(sandbox_client, content=body): + with pytest.raises(InvalidResponseError) as exc_info: + sandbox_client.capture_today("rec") + + message = str(exc_info.value) + assert "Malformed JSON" in message + assert "200" in message + assert "sandbox.tappaysdk.com" in message + assert "text/html" in message + assert f"{len(body)} bytes" in message + assert "Gateway Time-out" not in message + + +def test_invalid_response_message_never_echoes_the_body(sandbox_client): + """Exception text reaches logs and error trackers, so it must stay clean.""" + body = b'{"card_token": "SECRET_CARD_TOKEN", "name": "Wang Xiao Ming"' + with mock_bad_json(sandbox_client, content=body, content_type="application/json"): + with pytest.raises(InvalidResponseError) as exc_info: + sandbox_client.capture_today("rec") + + message = str(exc_info.value) + assert "SECRET_CARD_TOKEN" not in message + assert "Wang Xiao Ming" not in message + assert f"{len(body)} bytes" in message + + +def test_empty_success_body_raises_invalid_response(sandbox_client): + with mock_bad_json(sandbox_client, content=b"", content_type="text/plain"): + with pytest.raises(InvalidResponseError, match="0 bytes"): + sandbox_client.capture_today("rec") + + +def test_invalid_response_preserves_the_decode_error_as_cause(sandbox_client): + with mock_bad_json(sandbox_client): + with pytest.raises(InvalidResponseError) as exc_info: + sandbox_client.capture_today("rec") + + assert isinstance(exc_info.value.__cause__, ValueError) + + +def test_invalid_response_is_catchable_as_a_server_error(sandbox_client): + """Subclassing ServerError keeps existing handlers working.""" + with mock_bad_json(sandbox_client): + with pytest.raises(ServerError): + sandbox_client.capture_today("rec") + + with mock_bad_json(sandbox_client): + with pytest.raises(tappay.Error): + sandbox_client.capture_today("rec") + + +def test_missing_content_type_header_is_tolerated(sandbox_client): + with mock_bad_json(sandbox_client) as mocked: + mocked.return_value.headers = {} + with pytest.raises(InvalidResponseError, match="content-type: unknown"): + sandbox_client.capture_today("rec") + + +def test_no_content_response_is_unaffected_by_json_decoding(sandbox_client): + """204 returns before any decoding is attempted.""" + with mock_bad_json(sandbox_client, status_code=204): + assert sandbox_client.capture_today("rec") is None + + +def test_malformed_body_raises_before_strict_status_checking(strict_client): + """raise_on_error must not mask the decode failure.""" + with mock_bad_json(strict_client): + with pytest.raises(InvalidResponseError): + strict_client.capture_today("rec") + + +def test_malformed_body_is_reported_for_every_endpoint(sandbox_client, card_holder): + with mock_bad_json(sandbox_client): + with pytest.raises(InvalidResponseError): + sandbox_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder + ) + with mock_bad_json(sandbox_client): + with pytest.raises(InvalidResponseError): + sandbox_client.get_records({"time": {}}) From 922c0cfce4f8e63ef068734ac9af02c60e366673 Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Tue, 18 Aug 2026 09:18:16 +0800 Subject: [PATCH 5/5] Fix CI: test collection under bare pytest, and Markdown formatting Two failures, both caused by verifying with commands that differed from the ones CI runs. Test collection failed on every matrix entry with "No module named 'tests'". `python -m pytest` puts the current directory on sys.path but a bare `pytest` does not, and the test module imports shared helpers from tests.conftest. Local runs used the former, CI the latter. Set pythonpath = ["."] in the pytest configuration so both resolve. `ruff format --check .` failed on README.md and CHANGELOG.md. Ruff 0.16 began formatting Python code blocks inside Markdown; the local environment had 0.14.9, which ignores them. Add magic trailing commas to the affected examples so the formatter keeps them exploded rather than collapsing them onto one line, and strip the trailing whitespace the originals carried. Pin ruff to >=0.16,<0.17 in the dev extra. An unpinned formatter means a new release can break CI with no change to the repository, which is what happened here. Also ignore .gemini/, which is tool scratch output that ruff was picking up locally. Verified by running the exact CI commands (bare ruff, mypy and pytest) in a clean virtualenv built from the dev extra. Co-Authored-By: Claude Opus 5 --- .gitignore | 3 +++ CHANGELOG.md | 2 +- README.md | 14 +++++++------- pyproject.toml | 5 ++++- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/.gitignore b/.gitignore index ce59eb3..ed21b1b 100644 --- a/.gitignore +++ b/.gitignore @@ -152,3 +152,6 @@ cython_debug/ # VS Code .vscode/ + +# AI tool scratch output +.gemini/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 42b7c8e..b8ff04f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -129,7 +129,7 @@ card_holder = Models.CardHolderData("0912345678", "Wang Xiao Ming", "test@exampl card_holder = Models.CardHolderData( phone_number="0912345678", name="Wang Xiao Ming", - email="test@example.com" + email="test@example.com", ) ``` diff --git a/README.md b/README.md index 71dd333..bb44e07 100644 --- a/README.md +++ b/README.md @@ -38,9 +38,9 @@ import tappay # Initialize the client client = tappay.Client( - is_sandbox=True, - partner_key="YOUR_PARTNER_KEY", - merchant_id="YOUR_MERCHANT_ID" + is_sandbox=True, + partner_key="YOUR_PARTNER_KEY", + merchant_id="YOUR_MERCHANT_ID", ) ``` @@ -91,7 +91,7 @@ logging.getLogger("tappay.client").setLevel(logging.DEBUG) card_holder = tappay.Models.CardHolderData( phone_number="0912345678", name="Wang Xiao Ming", - email="test@example.com" + email="test@example.com", ) # Make payment @@ -99,7 +99,7 @@ response = client.pay_by_prime( prime="prime_token_from_frontend", amount=100, details="Order #123", - card_holder_data=card_holder + card_holder_data=card_holder, ) print(response) ``` @@ -111,7 +111,7 @@ response = client.pay_by_token( card_key="card_key", card_token="card_token", amount=100, - details="Subscription" + details="Subscription", ) ``` @@ -120,7 +120,7 @@ response = client.pay_by_token( ```python response = client.refund( rec_trade_id="rec_trade_id", - amount=100 + amount=100, ) ``` diff --git a/pyproject.toml b/pyproject.toml index bd6b9d2..2c3633c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ dependencies = [ dev = [ "pytest>=7.0", "pytest-cov>=4.0", - "ruff>=0.5", + "ruff>=0.16,<0.17", "mypy>=1.8", "types-requests", ] @@ -70,6 +70,9 @@ docstring-code-format = true [tool.pytest.ini_options] minversion = "6.0" addopts = "-ra -q" +# Bare `pytest` does not add the rootdir to sys.path the way `python -m pytest` +# does; tests import shared helpers from tests.conftest. +pythonpath = ["."] testpaths = [ "tests", ]