From 4900e3faf29b09a01ab2babb01dcef9b903786dd Mon Sep 17 00:00:00 2001 From: Chris Lo Date: Wed, 9 Sep 2026 14:15:15 +0800 Subject: [PATCH] Correct SDK against current TapPay docs (0.7.1) Four fixes found by checking the SDK against the current TapPay documentation. Two are defects introduced in 0.7.0, which was tagged but never published to PyPI, so no released version exposed them. Models.Currencies listed 11 currencies TapPay does not accept. 0.7.0 shipped JPY, GBP, AUD, EUR, CNY, KRW, SGD, THB, PHP, IDR and VND on assumption rather than from the docs. They autocompleted in an IDE and looked sanctioned, so they would have been shipped to production and rejected at the API. TapPay documents exactly four, each tied to an acquirer: TWD, HKD (Bank of China), MYR (RAZER PAY) and USD (Global Payments). Removed rather than deprecated, since keeping them keeps suggesting values that fail. Plain currency strings still pass through, so a currency TapPay adds later remains usable. raise_on_error=True raised on normal end-of-pagination. The Record API answers status 2 for "end of list, no more records under the given filter", which is how paging terminates. Every non-zero status was treated as a failure, so paging to the end of any query raised TapPayError. Benign statuses are now scoped per endpoint through BENIGN_STATUSES: 2 is benign for /tpc/transaction/query only, and still raises everywhere else. Full refunds were unreachable. refund() required amount, but TapPay documents it as needed only for a partial refund; omitting it refunds the whole transaction. amount is now optional and omitted from the request when None. Passing 0 still sends 0, because a zero-value partial refund is not the same as a full one. The default read timeout was 27.0s, taken from a generic requests convention. TapPay documents 30s because banks take longer at peak, so the shorter value could report a timeout for a transaction that succeeded. DEFAULT_TIMEOUT is now (3.05, 30.0). Verified against the Backend, Advanced Features and Reference pages as of 2026-09-09. 102 tests, coverage remains 100%. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 43 +++++++++++++++ README.md | 25 ++++++++- pyproject.toml | 2 +- tappay/client.py | 46 ++++++++++++---- tappay/models.py | 23 ++++---- tests/test_client.py | 123 ++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 232 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8ff04f..8e939f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,49 @@ 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.1] - 2026-09-09 + +Corrections found by checking the SDK against the current TapPay documentation. +Two of these are defects introduced in 0.7.0. 0.7.0 was tagged but never +published to PyPI, so no released version ever exposed them. + +### Fixed +- **`Models.Currencies` listed 11 currencies TapPay does not accept.** 0.7.0 + shipped JPY, GBP, AUD, EUR, CNY, KRW, SGD, THB, PHP, IDR and VND, none of + which are documented. They autocompleted in an IDE and looked sanctioned. + TapPay documents exactly four, each tied to an acquirer: TWD (most acquirers), + HKD (Bank of China), MYR (RAZER PAY) and USD (Global Payments). The removed + members are gone rather than deprecated, since keeping them would keep + suggesting currencies that fail at the API. Plain currency strings are still + accepted, so a currency TapPay adds later remains usable. +- **`raise_on_error=True` raised on a normal end-of-pagination.** The Record API + answers `status: 2` for "end of list, no more records under the given filter", + which is how paging terminates. 0.7.0 treated every non-zero status as a + failure, so paging to the end of any query raised `TapPayError`. Benign + statuses are now scoped per endpoint via `BENIGN_STATUSES`; `2` is benign for + `/tpc/transaction/query` only and still raises everywhere else. +- **Full refunds were unreachable.** `refund()` required `amount`, but TapPay + documents it as needed "only for partial refund" — omitting it refunds the + whole transaction. `amount` is now optional and dropped from the request when + `None`. Passing `0` still sends `0`, since a zero-value partial refund is not + the same as a full one. +- **The default read timeout was below TapPay's documented minimum.** It was + 27.0s, chosen from a generic `requests` convention; TapPay documents 30s + because banks take longer at peak. A short timeout reports failure for a + transaction that may have succeeded. `DEFAULT_TIMEOUT` is now `(3.05, 30.0)`. + +### Notes +- Verified against the TapPay Backend, Advanced Features and Reference pages as + of 2026-09-09. TapPay publishes no API changelog, so these are differences + observed against the current documentation, not a dated diff. +- Still outstanding and deferred to 0.8.0: `pay_by_token` accepts no cardholder + (whose `phone_number` and `email` are required for 3D-secure token payments); + `CardHolderData` is missing `name_en`, `phone_number_country_code`, + `member_id` and `bank_member_id`, and validates none of the documented field + lengths; `filters` and `details` are required by this SDK but optional in the + API; and nine documented endpoints remain unimplemented, including + reconciliation, card metadata and update-cardholder. + ## [0.7.0] - 2026-08-18 ### Added diff --git a/README.md b/README.md index bb44e07..f54bd36 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,13 @@ response = client.pay_by_token( ### Refunds +Omit `amount` to refund the transaction in full; pass it for a partial refund: + ```python +# Full refund +response = client.refund(rec_trade_id="rec_trade_id") + +# Partial refund response = client.refund( rec_trade_id="rec_trade_id", amount=100, @@ -126,8 +132,19 @@ 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: +Payments settle in TWD by default. TapPay documents four currencies, each tied to +the acquirer configured for your merchant account: + +| Currency | Acquirer | +| --- | --- | +| `TWD` | Most acquirers | +| `HKD` | Bank of China | +| `MYR` | RAZER PAY | +| `USD` | Global Payments | + +Pass `currency` to override the default, using either a `Models.Currencies` +member or a plain currency string (strings are accepted so a currency TapPay +adds before this list is updated remains usable): ```python response = client.pay_by_prime( @@ -162,6 +179,10 @@ except tappay.TapPayError as exc: print(exc.status, exc.msg, exc.response["rec_trade_id"]) ``` +`get_records` is exempt from one case: the Record API answers `status: 2` for +"no more records under this filter", which is how pagination terminates. That is +treated as a normal result, not an error, even with `raise_on_error=True`. + The exception hierarchy is: | Exception | Raised when | diff --git a/pyproject.toml b/pyproject.toml index 2c3633c..6671094 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "tappay" -version = "0.7.0" +version = "0.7.1" authors = [ { name="Shih Wei Chris Lo", email="shihwei@gmail.com" }, ] diff --git a/tappay/client.py b/tappay/client.py index dbe6f5f..d543b85 100644 --- a/tappay/client.py +++ b/tappay/client.py @@ -1,7 +1,7 @@ import logging import os from platform import python_version -from typing import Any, Dict, Optional, Tuple, Union, cast +from typing import Any, Dict, FrozenSet, Optional, Tuple, Union, cast import requests from requests.adapters import HTTPAdapter @@ -18,8 +18,10 @@ #: 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) +#: the ``requests`` documentation. The read value is the 30 seconds TapPay +#: documents: banks can take longer at peak, and timing out early reports a +#: failure for a transaction that may well have succeeded. +DEFAULT_TIMEOUT: Tuple[float, float] = (3.05, 30.0) #: Anything ``requests`` accepts for its ``timeout`` argument. ``None`` means #: "block indefinitely" and is strongly discouraged for server-side use. @@ -44,6 +46,15 @@ #: reported with an HTTP 200, which is why it needs explicit handling. SUCCESS_STATUS = 0 +#: Non-zero ``status`` values that are documented as normal outcomes rather +#: than failures, keyed by endpoint. The Record API answers ``2`` for "end of +#: list, no more records under the given filter", which is how pagination +#: terminates -- treating it as an error would make paging with +#: ``raise_on_error=True`` raise on the last page of every query. +BENIGN_STATUSES: Dict[str, FrozenSet[int]] = { + "/tpc/transaction/query": frozenset({2}), +} + class _Unset: """Sentinel distinguishing "argument omitted" from an explicit ``None``.""" @@ -298,7 +309,7 @@ def pay_by_token( def refund( self, rec_trade_id: str, - amount: int, + amount: Optional[int] = None, *, timeout: Union[TimeoutType, _Unset] = _UNSET, **kwargs: Any, @@ -306,12 +317,19 @@ def refund( """ Refund a payment Ref: https://docs.tappaysdk.com/tutorial/zh/back.html#refund-api + + :param amount: Amount to refund. TapPay documents this as required only + for a partial refund; omit it to refund the transaction in full. + Sending ``0`` is a zero-value partial refund, not a full one, so the + field is dropped from the request when ``amount`` is ``None``. """ - params = { + params: Dict[str, Any] = { "rec_trade_id": rec_trade_id, - "amount": amount, } + if amount is not None: + params["amount"] = amount + if kwargs: params.update(**kwargs) @@ -508,9 +526,11 @@ def __post( response = self.session.post( uri, json=params, headers=self.headers, timeout=effective_timeout ) - return self.__parse(response) + return self.__parse(response, request_uri) - def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: + def __parse( + self, response: requests.Response, request_uri: str = "" + ) -> Optional[Dict[str, Any]]: if logger.isEnabledFor(logging.DEBUG): logger.debug("response status: %s", response.status_code) logger.debug("response content: %s", _redacted_body(response)) @@ -522,7 +542,7 @@ def __parse(self, response: requests.Response) -> Optional[Dict[str, Any]]: return None elif 200 <= response.status_code < 300: data = self.__decode_json(response) - self.__raise_for_body_status(data) + self.__raise_for_body_status(data, request_uri) return data elif 400 <= response.status_code < 500: message = f"{response.status_code} response from {self.api_host}" @@ -559,12 +579,14 @@ def __decode_json(self, response: requests.Response) -> Optional[Dict[str, Any]] ) raise Exceptions.InvalidResponseError(message) from exc - def __raise_for_body_status(self, data: Any) -> None: + def __raise_for_body_status(self, data: Any, request_uri: str = "") -> 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. + the documented envelope are passed through to the caller untouched, and + statuses listed in :data:`BENIGN_STATUSES` for the endpoint are treated + as normal outcomes rather than failures. """ if not self.raise_on_error or not isinstance(data, dict): return @@ -572,5 +594,7 @@ def __raise_for_body_status(self, data: Any) -> None: status = data.get("status") if status is None or status == SUCCESS_STATUS: return + if status in BENIGN_STATUSES.get(request_uri, frozenset()): + return raise Exceptions.TapPayError(status, data.get("msg"), data) diff --git a/tappay/models.py b/tappay/models.py index a0296a5..057143f 100644 --- a/tappay/models.py +++ b/tappay/models.py @@ -8,7 +8,13 @@ class Models: """Namespace for TapPay models.""" class Currencies(str, Enum): - """Currencies accepted by the TapPay APIs. + """Currencies documented as accepted by the TapPay APIs. + + Only these four are documented, and each depends on the acquirer + configured for your merchant account: TWD is supported by most + acquirers, HKD by Bank of China, MYR by RAZER PAY, and USD by Global + Payments. Confirm your own acquirer supports a currency before using + it, since an unsupported one is rejected at the API rather than here. A ``str``-backed enum, so members compare equal to their plain-string form and serialize as ``"TWD"`` through ``json.dumps``. Note that @@ -16,24 +22,13 @@ class Currencies(str, Enum): 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. + 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" + USD = "USD" class CardHolderData(BaseModel): """Card holder data model using Pydantic v2. diff --git a/tests/test_client.py b/tests/test_client.py index 852c05e..c5e8834 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -9,6 +9,7 @@ import tappay from tappay.client import ( + BENIGN_STATUSES, DEFAULT_MAX_RETRIES, DEFAULT_TIMEOUT, RETRYABLE_PATHS, @@ -268,9 +269,9 @@ 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" + card_key="ck", card_token="ct", amount=1, details="d", currency="MYR" ) - assert post.call_args.kwargs["json"]["currency"] == "JPY" + assert post.call_args.kwargs["json"]["currency"] == "MYR" sandbox_client.bind_card( prime="p", card_holder_data=card_holder, currency=Models.Currencies.HKD @@ -800,3 +801,121 @@ def test_malformed_body_is_reported_for_every_endpoint(sandbox_client, card_hold with mock_bad_json(sandbox_client): with pytest.raises(InvalidResponseError): sandbox_client.get_records({"time": {}}) + + +# --- Documented currency set (0.7.1) ------------------------------------- + + +def test_currencies_match_what_tappay_documents(): + """0.7.0 shipped 11 currencies TapPay does not accept. Only these four are + documented: TWD (most acquirers), HKD (Bank of China), MYR (RAZER PAY), + USD (Global Payments).""" + assert {c.value for c in Models.Currencies} == {"TWD", "HKD", "MYR", "USD"} + + +@pytest.mark.parametrize("code", ["JPY", "EUR", "GBP", "KRW", "CNY", "SGD", "VND"]) +def test_undocumented_currencies_are_not_offered_as_members(code): + """These autocompleted in an IDE and looked sanctioned; they were not.""" + assert not hasattr(Models.Currencies, code) + + +def test_unlisted_currency_strings_still_pass_through( + sandbox_client, card_holder, post +): + """The enum narrows what we *suggest*, not what the caller may send.""" + sandbox_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder, currency="JPY" + ) + + assert post.call_args.kwargs["json"]["currency"] == "JPY" + + +# --- Timeout matches TapPay's guidance (0.7.1) --------------------------- + + +def test_read_timeout_matches_tappays_documented_30_seconds(): + """TapPay documents a 30s timeout because banks are slow at peak; a shorter + one reports failure for a transaction that may have succeeded.""" + connect, read = DEFAULT_TIMEOUT + + assert read == 30.0 + assert connect > 3.0 + + +# --- Benign statuses are per-endpoint (0.7.1) ---------------------------- + + +def test_record_api_end_of_list_is_not_an_error(strict_client): + """status 2 from the Record API means "no more records", which is how + pagination terminates -- it must not raise under raise_on_error.""" + payload = {"status": 2, "msg": "Success", "number_of_transactions": 0} + with mock_post(strict_client, payload=payload): + response = strict_client.get_records({"time": {}}, page=99) + + assert response["status"] == 2 + + +def test_same_status_still_raises_on_a_payment_endpoint(strict_client, card_holder): + """2 is benign only for the Record API; it is not whitelisted globally.""" + with mock_post(strict_client, payload={"status": 2, "msg": "nope"}): + with pytest.raises(TapPayError) as exc_info: + strict_client.pay_by_prime( + prime="p", amount=1, details="d", card_holder_data=card_holder + ) + + assert exc_info.value.status == 2 + + +def test_other_non_zero_statuses_still_raise_on_the_record_api(strict_client): + with mock_post(strict_client, payload={"status": 3, "msg": "Card declined"}): + with pytest.raises(TapPayError): + strict_client.get_records({"time": {}}) + + +def test_benign_statuses_are_scoped_to_known_endpoints(): + assert BENIGN_STATUSES == {"/tpc/transaction/query": frozenset({2})} + + +def test_end_of_list_is_returned_unchanged_without_strict_mode(sandbox_client): + with mock_post(sandbox_client, payload={"status": 2}): + assert sandbox_client.get_records({"time": {}})["status"] == 2 + + +# --- Full refunds (0.7.1) ------------------------------------------------- + + +def test_full_refund_omits_the_amount_field(sandbox_client, post): + """TapPay refunds the whole transaction when `amount` is absent; sending + an amount makes it a partial refund instead.""" + sandbox_client.refund("rec") + + body = post.call_args.kwargs["json"] + assert body == {"rec_trade_id": "rec", "partner_key": "partner_key"} + assert "amount" not in body + + +def test_partial_refund_still_sends_the_amount(sandbox_client, post): + sandbox_client.refund("rec", 100) + + assert post.call_args.kwargs["json"]["amount"] == 100 + + +def test_zero_amount_refund_is_sent_rather_than_treated_as_absent(sandbox_client, post): + """0 is falsy but means a zero-value partial refund, not a full one.""" + sandbox_client.refund("rec", 0) + + assert post.call_args.kwargs["json"]["amount"] == 0 + + +def test_explicit_none_amount_means_a_full_refund(sandbox_client, post): + sandbox_client.refund("rec", amount=None) + + assert "amount" not in post.call_args.kwargs["json"] + + +def test_full_refund_still_accepts_extra_fields(sandbox_client, post): + sandbox_client.refund("rec", bank_refund_id="BR1") + + body = post.call_args.kwargs["json"] + assert body["bank_refund_id"] == "BR1" + assert "amount" not in body