diff --git a/AGENTS.md b/AGENTS.md index da57647..62daa83 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -48,6 +48,9 @@ had been believed: | `expiresAt` of the installment plans is a unix timestamp in seconds (`1735689599`, per the API reference) | It is **milliseconds** (13 digits). Reading it as seconds lands in the year 58608 and raises | | The installment plans response is documented with seconds throughout | Mixed: `expiresAt` in ms, transaction dates as `YYYY-MM-DD HH:MM:SS` strings, rate dates as `YYYY-MM-DD` | | One response uses one type per boolean | `card3ds` is a bool while `billingAddressRequired` is the string `"false"` — same response | +| A discount can be sent as its own negative basket item (a `voucher` line) | Both schemas reject negative item amounts — `API.600.200.131`, plus `API.600.410.018` on v1. A discount belongs in `amountDiscount` (v1) or `amountDiscountPerUnitGross` (v3), positive | +| The basket endpoint checks its own arithmetic | Only v3 does, to the cent (`API.600.410.062`). v1 accepts items that contradict `amountTotalGross`, and a charge does not compare the basket to the payment amount either — measured with Prepayment: amounts of 817.02, 726.24, 907.80 and 1.00 are all accepted against the same basket worth 817.02 | +| Any `returnUrl` the API accepts is fine for local development | A gateway in front of the API refuses `localhost`, `127.0.0.1` and private IPs with a **403 and an nginx HTML page** — the API never sees the request. Hostnames that merely *resolve* to 127.0.0.1 (`lvh.me`, `localtest.me`, `127-0-0-1.nip.io`) pass, so the block is on the string, not the resolved address | ### How to verify @@ -182,25 +185,84 @@ from its source, the docstring says so and why — `PaymentTypes` drops the `UNK the Java SDK uses as a parsing fallback. Deviating silently is how `Action` came to be missing a type in the first place. -**Amounts are rounded to four decimals on serialisation** (`unzer.utils.roundAmount`). The API -takes `Decimal{10,4}`, and float arithmetic does not cooperate: `12.3 - 10.0 - 2.3` is -`8.88e-16`, which `json.dumps` writes in scientific notation — not a number the API accepts. -Amounts come back from the API as *strings* with four decimals (`"5.5500"`). +**Basket-level amounts are rounded to four decimals on serialisation** +(`unzer.utils.roundAmount`). The API takes `Decimal{10,4}`, and float arithmetic does not +cooperate: `12.3 - 10.0 - 2.3` is `8.88e-16`, which `json.dumps` writes in scientific +notation — not a number the API accepts. Amounts come back from the API as *strings* with +four decimals (`"5.5500"`). + +`BasketItem.serialize` does **not** do this — measured, an item goes out as +`0.30000000000000004` next to a basket total rounded to `0.3`. That is an inconsistency, +not a design decision (#18), and it matters most where v3 reconciles the total against the +items to the cent. **`Basket` intentionally supports two incompatible schemas.** v1 uses `amountTotalGross`, v3 uses `totalValueGross`; setting the latter switches the basket to the v3 endpoint. Do not "clean this up" into one schema. Which version a payment method needs is a recurring source of confusion, and the -documentation is not reliable here: +documentation is not reliable here. Measured, the answer so far is that none of them +*needs* a particular one — what the column records is which schemas were seen to work, +not a requirement: | Method | Verified in practice | What the docs claim | |---|---|---| -| `paylater-installment` | v3 | v3 | -| `klarna` | **v1 works** — in production use via viur-shop | v2 | +| `paylater-installment` | **both** — sandbox authorize succeeds on v1 and on v3 | v3 | +| `klarna` | **both** — v1 in production use via viur-shop, v3 measured | v2 | | `paylater-invoice` | not verified | v2 | | everything else | v1 | v1 | +**Do not mix the schemas within one basket** — and note that only one direction fails +loudly. Measured: a v3 item in a v1 basket is accepted with a 201 and every item amount +stored as `0.0000`, while the basket-level total survives, so the basket looks plausible +and every line is worth nothing. A v1 item in a v3 basket is refused with +`API.600.410.051`. `Basket.isV3()` and `BasketItem.isV3()` decide independently and +nothing checks that they agree (#16). A basket is also only readable through the schema it +was created with — `API.600.410.024` otherwise — and the ids differ visibly: v1 gives +`s-bsk-72`, v3 a UUID. + +**A basket can only be used once.** A second charge against the same `basketId` is refused with `API.330.200.152 "Resources: basket was used."`, so a retry after a failed authorize needs a new basket. + +**Discounts belong in a discount field, not in a negative line item.** The obvious +shape — one item per article plus a `voucher` item carrying the negative discount, the +grosses adding up to the order total — is refused by both schemas +(`API.600.200.131 "Amount … has to be positive"`, and `API.600.410.018` on v1). Send the +discount as a positive value instead: + +| | v1 | v3 | +|---|---|---| +| Field | `amountDiscount` — per line or per unit is **undecidable, and so far inconsequential**: nothing measured reads the value back out (see below) | `amountDiscountPerUnitGross`, **per unit** | +| Item amount | `amountGross` stays the pre-discount gross; the API stores both untouched | `amountPerUnitGross` minus the discount must stay positive | +| Total | not checked at all | `totalValueGross == sum((amountPerUnitGross - amountDiscountPerUnitGross) * quantity)`, exact to the cent (`API.600.410.062`) | +| `vat` per item | optional | mandatory (`API.600.410.052`) | + +So a discount that exceeds a single line has to be spread over several items in v3, +while v1 swallows it. v3 names the offending line in `API.600.410.064` ("Basket item i1 +'amountDiscountPerUnitGross' does not equal to 'amountPerUnitGross'") — but only if the +basket total stays positive; otherwise the negative total is refused first with the +generic `API.600.200.131`, which says nothing about the item. The v1 tolerance is not a +licence: a charge does not compare the basket to the payment amount, but the methods +that forward the basket to a partner system may. `tests/sandbox/test_live_api.py::TestBasket` holds all of this as executable evidence, +except the v1 per-line/per-unit question above. + +**Nothing measured consumes `amountDiscount`.** The question was chased down the whole +chain with an item of `quantity=3`, gross 300.00 and `amountDiscount=10.00`, where the +two readings differ by 20.00: + +* the v1 basket endpoint stores it and reconciles nothing; +* a charge does not compare the basket to the amount at all; +* Klarna's checkout lists the item by title only and takes its total from the authorize + `amount` — 290.00 and 270.00 were both accepted and both displayed as sent; +* Unzer's Hosted Payment Page renders the line as `3x T-Shirt € 300,00`, i.e. plain + `amountGross` with the discount nowhere, and its total likewise comes from the request, + not from the basket. + +So `amountDiscount` is stored and forwarded but not evaluated by anything reachable from +here, and the per-line/per-unit distinction has no observable effect. Worth knowing for +two reasons: the value cannot be used to make a basket "add up" for the customer, and a +line reduced by a discount is shown at its **undiscounted** gross next to a lower total, +with no label explaining the difference. + Basket v2 is not implemented (`Basket.apiVersion` only returns `v1` or `v3`), and so far nothing has needed it. v2 and v3 share one schema, so if a method ever does require v2, the v3 model can build the body — only the endpoint version differs. @@ -304,5 +366,9 @@ minimal dicts. affects parsing. - Errors can arrive with a 2xx status code and an `errors` list in the body — Unzer documents this explicitly. Do not treat HTTP 200 as success without checking `isError`. +- Not every 4xx body is JSON. The gateway in front of the API answers with an HTML page, + so `_request` builds an `ErrorResponse` from the status and the raw text instead of + parsing it — a bare `JSONDecodeError` from inside the SDK hides both and reads like an + SDK bug. The known trigger is a `returnUrl` on localhost or a private IP. - `logger.debug` output contains full request payloads, including IBANs and dates of birth. Do not add payload logging above DEBUG level. diff --git a/src/unzer/client.py b/src/unzer/client.py index 91536be..e163215 100644 --- a/src/unzer/client.py +++ b/src/unzer/client.py @@ -204,7 +204,29 @@ def _request(self, url: str, method: str, continue logger.debug("Client error") logger.debug("Response[%s %s]: %r", r.status_code, r.reason, r.text) - errorResponse = ErrorResponse.fromDict(r.json()) + # Not every 4xx comes from the API itself, and a body the SDK cannot map + # onto its error model used to escape as a bare exception from inside + # `fromDict` -- hiding the status code and the body, so the caller could + # not tell a refused request from an SDK bug. The two causes are kept + # apart because they need different things done about them. + try: + data = r.json() + except ValueError: + # Measured: a gateway in front of the API refuses a returnUrl on + # localhost or a private IP with a 403 and an nginx HTML page. + errorResponse = ErrorResponse( + f"HTTP {r.status_code} {r.reason} with a non-JSON body: {r.text[:200]!r}") + else: + try: + errorResponse = ErrorResponse.fromDict(data) + except (KeyError, TypeError, ValueError): + # Valid JSON, but not this API's error envelope -- the shape an + # API gateway or WAF sends (`{"message": "Forbidden"}`), and the + # shape `fromDict` produces for a timestamp it cannot parse. + logger.exception(f"Cannot read {data!r} as an ErrorResponse") + errorResponse = ErrorResponse( + f"HTTP {r.status_code} {r.reason} with a body the error schema " + f"does not fit: {r.text[:200]!r}") errorResponse.statusCode = r.status_code errorResponse.srcResponse = r raise errorResponse diff --git a/src/unzer/model/basket.py b/src/unzer/model/basket.py index e26b2a5..4aeeba9 100644 --- a/src/unzer/model/basket.py +++ b/src/unzer/model/basket.py @@ -13,9 +13,17 @@ class Basket(BaseModel): with gross amounts, otherwise the v1 endpoint with :attr:`amountTotalGross` is used. The basket items follow the same rule on their own (see :class:`unzer.model.BasketItem`), so don't mix the schemas within one basket. + Measured: a v3 item in a v1 basket is accepted with a 201 and every item amount + stored as ``0.0000``, silently, while a v1 item in a v3 basket is refused with + ``API.600.410.051``. A basket is also only readable through the schema it was + created with, ``API.600.410.024`` otherwise. - The Pay later payment methods (e.g. :class:`unzer.model.PaylaterInstallment`) - require the v3 schema. + Both schemas are accepted by every payment method measured so far -- a sandbox + authorize with Klarna and with :class:`unzer.model.PaylaterInstallment` succeeds + on either. The documentation claims the Pay later methods require v3; the API does + not enforce it. What does differ is validation: v3 reconciles + :attr:`totalValueGross` against the items to the cent (``API.600.410.062``), + v1 checks nothing at all. Note that the v2 and v3 endpoints share the same schema, so the newer v3 is used here. """ diff --git a/src/unzer/model/basketItem.py b/src/unzer/model/basketItem.py index 690f316..b1b27d9 100644 --- a/src/unzer/model/basketItem.py +++ b/src/unzer/model/basketItem.py @@ -47,7 +47,8 @@ def __init__( :param amountDiscount: (optional) (v1) Discount amount for the basket item (multiplied by the :attr:`quantity`) format: float :type amountDiscount: float - :param vat: (optional) Integer Vat value for the basket item in percent (0-100) format: int32 + :param vat: (optional in v1, mandatory in v3 -- ``API.600.410.052``) Integer + Vat value for the basket item in percent (0-100) format: int32 :type vat: int :param amountGross: (optional) (v1) Gross amount (= amountNet + amountVat) in the specified currency. Equals amountNet if vat value is 0 format: float diff --git a/src/unzer/model/error.py b/src/unzer/model/error.py index 16cc3b0..94b8f3c 100644 --- a/src/unzer/model/error.py +++ b/src/unzer/model/error.py @@ -1,5 +1,8 @@ -import datetime import logging +import typing as t +from datetime import datetime as dt + +from ..utils import parseDateTime logger = logging.getLogger("unzer-sdk").getChild(__name__) @@ -14,7 +17,13 @@ class Error: .. seealso:: https://docs.unzer.com/server-side-integration/api-basics/error-handling/ """ - def __init__(self, code, merchantMessage, customerMessage, **kwargs): + def __init__( + self, + code: str | None = None, + merchantMessage: str | None = None, + customerMessage: str | None = None, + **kwargs: t.Any, + ) -> None: self.code = code self.merchantMessage = merchantMessage self.customerMessage = customerMessage @@ -77,12 +86,28 @@ def __init__( logger.warning("ErrorResponse got additional unhandled data: %r", kwargs) @classmethod - def fromDict(cls, data, message="Unzer Error"): + def fromDict(cls, data: t.Any, message: str = "Unzer Error") -> "ErrorResponse": + """Build an ErrorResponse from a decoded API error body. + + Only ``errors`` is treated as required, because it is what identifies the + body as this API's error envelope and it is the one part a caller acts on -- + `UnzerClient.createOrUpdateCustomer` branches on ``errors[0].code``. Anything + else missing or unreadable costs that field alone, never the list: an error + that cannot be reported is worse than one reported without its timestamp. + + :param data: The decoded body. + :param message: The exception message. + :raises ValueError: If ``data`` is not an error envelope, so the caller can + tell "the API refused this" from "something else answered". + :return: The error response. + """ + if not isinstance(data, dict) or "errors" not in data: + raise ValueError(f"Not an API error envelope: {data!r}") return cls( message, - timestamp=datetime.datetime.strptime(data["timestamp"], "%Y-%m-%d %H:%M:%S"), - url=data["url"], - errors=[Error(**error) for error in data["errors"]], + timestamp=cls._parseTimestamp(data.get("timestamp")), + url=data.get("url"), + errors=[Error(**error) for error in data.get("errors") or []], errorId=data.get("id"), traceId=data.get("traceId"), isError=data.get("isError"), @@ -90,6 +115,22 @@ def fromDict(cls, data, message="Unzer Error"): isSuccess=data.get("isSuccess"), ) + @staticmethod + def _parseTimestamp(value: str | dt | None) -> dt | None: + """Read the error timestamp, tolerating a format the SDK does not know. + + The API is known to use two formats and has been seen with others; losing the + error codes over the one field nobody branches on is not a trade worth making. + + :param value: The raw ``timestamp`` value. + :return: The parsed timestamp, or ``None`` if it cannot be read. + """ + try: + return parseDateTime(value) + except (TypeError, ValueError): + logger.warning(f"Cannot parse the error timestamp {value!r}") + return None + def __repr__(self): return ( f"{self.__class__.__module__}.{self.__class__.__name__}(" diff --git a/tests/sandbox/test_live_api.py b/tests/sandbox/test_live_api.py index ac435c8..67bd4df 100644 --- a/tests/sandbox/test_live_api.py +++ b/tests/sandbox/test_live_api.py @@ -187,6 +187,43 @@ def test_create_or_update_recovers_from_a_duplicate(self, sandbox_client): class TestBasket: + """Baskets, and how a discount has to be expressed in each of the two schemas. + + Both schemas reject line items with negative amounts, so a discount cannot be sent + as its own negative "voucher" item -- it belongs in the positive discount field of + the item it reduces (``amountDiscount`` in v1, ``amountDiscountPerUnitGross`` in + v3). What the schemas do not share is how much of the arithmetic the API checks: v3 + reconciles the total to the cent, v1 checks nothing at all. + + Note that a *charge* does not validate the basket against the payment amount + either -- measured with Prepayment against one basket worth 817.02, the amounts + 817.02, 726.24, 907.80 and 1.00 were all accepted and booked at face value. That + is not tested here, because it would create a payment on the account for every + run, and it says nothing about the payment methods that hand the basket on to a + partner system, which may well be stricter. + """ + + # 19 % VAT: 907.80 gross == 762.86 net + 144.94 VAT. A 10 % basket discount of + # 90.78 leaves a total of 817.02. + VAT_PERCENT = 19 + GROSS = 907.80 + NET = 762.86 + VAT_AMOUNT = 144.94 + DISCOUNT = 90.78 + TOTAL = 817.02 + + def goods_v1(self, **overrides): + """A v1 line item, overridable per test.""" + return BasketItem( + basketItemReferenceId="item-1", title="T-Shirt", quantity=1, kind="goods", + vat=self.VAT_PERCENT, amountPerUnit=self.NET, amountNet=self.NET, + amountVat=self.VAT_AMOUNT, amountGross=self.GROSS, **overrides) + + def goods_v3(self, **overrides): + """A v3 line item, overridable per test.""" + return BasketItem( + basketItemReferenceId="item-1", title="T-Shirt", quantity=1, kind="goods", + vat=self.VAT_PERCENT, amountPerUnitGross=self.GROSS, **overrides) def test_v1_basket(self, sandbox_client): basket = sandbox_client.createBasket(Basket( @@ -194,7 +231,7 @@ def test_v1_basket(self, sandbox_client): currencyCode="EUR", orderId="sdk-test-basket-v1", basketItems=[BasketItem( title="T-Shirt", quantity=1, vat=19, amountGross=100.0, amountPerUnit=100.0, - amountNet=84.03, amountVat=15.97, basketItemReferenceId="item-1", type="goods")], + amountNet=84.03, amountVat=15.97, basketItemReferenceId="item-1", kind="goods")], )) assert basket.key assert not basket.isV3() @@ -204,7 +241,7 @@ def test_v3_basket(self, sandbox_client): totalValueGross=100.0, currencyCode="EUR", orderId="sdk-test-basket-v3", basketItems=[BasketItem( title="T-Shirt", quantity=1, vat=19, amountPerUnitGross=100.0, - basketItemReferenceId="item-1", type="goods")], + basketItemReferenceId="item-1", kind="goods")], )) assert basket.key # v3 ids are UUIDs while v1 ids are short counters -- a cheap way to tell @@ -212,6 +249,177 @@ def test_v3_basket(self, sandbox_client): assert basket.isV3() assert len(basket.key) > len("s-bsk-999") + def test_v1_rejects_negative_item_amounts(self, sandbox_client): + """A discount as its own negative line item is refused, not merely discouraged. + + This is the shape a consumer arrives at naturally -- one item per article, one + item for the voucher, the grosses adding up to the order total -- and it fails + for every basket that carries a discount. + """ + with pytest.raises(ErrorResponse) as excinfo: + sandbox_client.createBasket(Basket( + amountTotalGross=self.TOTAL, currencyCode="EUR", + orderId="sdk-test-basket-v1-negative", + basketItems=[self.goods_v1(), BasketItem( + basketItemReferenceId="discount-1", title="Voucher", quantity=1, + kind="voucher", vat=0, amountPerUnit=-self.DISCOUNT, + amountNet=-self.DISCOUNT, amountVat=0.0, amountGross=-self.DISCOUNT)], + )) + codes = {error.code for error in excinfo.value.errors} + assert "API.600.410.018" in codes, codes # basket item has negative amount gross + assert "API.600.200.131" in codes, codes # amount has to be positive + + def test_v3_rejects_negative_item_amounts(self, sandbox_client): + """v3 refuses them as well, so the schema switch alone is no way around it.""" + with pytest.raises(ErrorResponse) as excinfo: + sandbox_client.createBasket(Basket( + totalValueGross=self.TOTAL, currencyCode="EUR", + orderId="sdk-test-basket-v3-negative", + basketItems=[self.goods_v3(), BasketItem( + basketItemReferenceId="discount-1", title="Voucher", quantity=1, + kind="voucher", vat=0, amountPerUnitGross=-self.DISCOUNT)], + )) + assert "API.600.200.131" in {error.code for error in excinfo.value.errors} + + def test_v1_discount_goes_into_amount_discount(self, sandbox_client): + """The v1 way: a positive ``amountDiscount`` on the item it reduces. + + Reading the basket back shows that the API stores both values untouched -- + ``amountGross`` stays the pre-discount gross, so the reduced value is + ``amountGross - amountDiscount`` and the caller owns that arithmetic. + """ + basket = sandbox_client.createBasket(Basket( + amountTotalGross=self.TOTAL, amountTotalDiscount=self.DISCOUNT, + currencyCode="EUR", orderId="sdk-test-basket-v1-discount", + basketItems=[self.goods_v1(amountDiscount=self.DISCOUNT)], + )) + assert basket.key + stored = sandbox_client.getBasket(basket.key) + assert stored.amountTotalGross == self.TOTAL + assert stored.amountTotalDiscount == self.DISCOUNT + item = stored.basketItems[0] + assert item.amountDiscount == self.DISCOUNT + assert item.amountGross == self.GROSS, "the API does not subtract the discount" + assert item.kind == "goods", "the item type is sent as `type`, not as `kind`" + + def test_v3_discount_goes_into_amount_discount_per_unit_gross(self, sandbox_client): + """The v3 way: a positive ``amountDiscountPerUnitGross``, per unit.""" + basket = sandbox_client.createBasket(Basket( + totalValueGross=self.TOTAL, currencyCode="EUR", + orderId="sdk-test-basket-v3-discount", + basketItems=[self.goods_v3(amountDiscountPerUnitGross=self.DISCOUNT)], + )) + assert basket.key + assert basket.isV3() + + def test_v3_multiplies_the_discount_by_the_quantity(self, sandbox_client): + """``amountDiscountPerUnitGross`` is per unit, not per line. + + Three units at 100.00 with a per-unit discount of 10.00 reconcile against a + total of 270.00 -- if the discount counted once per line, the total would have + to be 290.00 and this call would fail. + """ + basket = sandbox_client.createBasket(Basket( + totalValueGross=3 * (100.0 - 10.0), currencyCode="EUR", + orderId="sdk-test-basket-v3-quantity", + basketItems=[BasketItem( + basketItemReferenceId="item-1", title="T-Shirt", quantity=3, kind="goods", + vat=self.VAT_PERCENT, amountPerUnitGross=100.0, + amountDiscountPerUnitGross=10.0)], + )) + assert basket.key + + def test_v3_reconciles_the_total_to_the_cent(self, sandbox_client): + """v3 enforces ``totalValueGross == sum((perUnit - discount) * quantity)``. + + A single cent is enough to be refused, so a discount spread over several items + has to be rounded so that the parts add up exactly. + """ + with pytest.raises(ErrorResponse) as excinfo: + sandbox_client.createBasket(Basket( + totalValueGross=self.TOTAL + 0.01, currencyCode="EUR", + orderId="sdk-test-basket-v3-off-by-a-cent", + basketItems=[self.goods_v3(amountDiscountPerUnitGross=self.DISCOUNT)], + )) + assert "API.600.410.062" in {error.code for error in excinfo.value.errors} + + def test_v1_does_not_reconcile_the_total(self, sandbox_client): + """v1 accepts a basket whose items contradict its own total. + + Documented as a warning, not as a licence: the value is passed on to the + payment method, and the ones that forward the basket to a partner system may + be stricter than the basket endpoint is. + """ + basket = sandbox_client.createBasket(Basket( + amountTotalGross=1.00, currencyCode="EUR", + orderId="sdk-test-basket-v1-wrong-total", basketItems=[self.goods_v1()], + )) + assert basket.key + assert sandbox_client.getBasket(basket.key).amountTotalGross == 1.00 + + def test_v3_requires_vat_on_every_item(self, sandbox_client): + """``vat`` is mandatory in v3 -- the v1 endpoint takes items without it.""" + with pytest.raises(ErrorResponse) as excinfo: + sandbox_client.createBasket(Basket( + totalValueGross=self.GROSS, currencyCode="EUR", + orderId="sdk-test-basket-v3-no-vat", + basketItems=[BasketItem( + basketItemReferenceId="item-1", title="T-Shirt", quantity=1, + kind="goods", amountPerUnitGross=self.GROSS)], + )) + assert "API.600.410.052" in {error.code for error in excinfo.value.errors} + + def test_v1_takes_items_without_vat(self, sandbox_client): + """The counterpart: v1 accepts the same item without ``vat`` and stores 0.""" + basket = sandbox_client.createBasket(Basket( + amountTotalGross=self.GROSS, currencyCode="EUR", + orderId="sdk-test-basket-v1-no-vat", + basketItems=[BasketItem( + basketItemReferenceId="item-1", title="T-Shirt", quantity=1, kind="goods", + amountPerUnit=self.NET, amountNet=self.NET, amountGross=self.GROSS)], + )) + assert sandbox_client.getBasket(basket.key).basketItems[0].vat == 0.0 + + def test_v3_discount_must_not_exceed_the_unit_price(self, sandbox_client): + """The per-item result must stay positive, which caps the discount per item. + + A discount bigger than the item it sits on therefore has to be spread across + several items in v3. + + A second, larger item keeps ``totalValueGross`` positive on purpose. With only + the over-discounted line the basket total is negative too, and the API answers + ``API.600.200.131`` "Amount has to be positive" -- which the negative total + alone explains, so such a basket cannot show that the *item* is what was + refused. Isolated like this the API names the item instead, in + ``API.600.410.064``: "Basket item i1 'amountDiscountPerUnitGross' does not + equal to 'amountPerUnitGross'". + """ + with pytest.raises(ErrorResponse) as excinfo: + sandbox_client.createBasket(Basket( + totalValueGross=(self.GROSS - 1000.0) + 2000.0, currencyCode="EUR", + orderId="sdk-test-basket-v3-discount-too-large", + basketItems=[ + self.goods_v3(amountDiscountPerUnitGross=1000.0), + BasketItem( + basketItemReferenceId="item-2", title="T-Shirt", quantity=1, + kind="goods", vat=self.VAT_PERCENT, amountPerUnitGross=2000.0), + ], + )) + assert "API.600.410.064" in {error.code for error in excinfo.value.errors} + + def test_v1_accepts_a_discount_larger_than_its_item(self, sandbox_client): + """v1 does not cap it, the counterpart to the v3 test above. + + Another consequence of v1 checking nothing: the item is left at an effective + -92.20 and the endpoint still answers 201. + """ + basket = sandbox_client.createBasket(Basket( + amountTotalGross=self.TOTAL, currencyCode="EUR", + orderId="sdk-test-basket-v1-discount-too-large", + basketItems=[self.goods_v1(amountDiscount=1000.0)], + )) + assert basket.key + class TestPaymentPage: """Paypage v1 is tagged [Deprecated] in the spec but still works.""" diff --git a/tests/test_client.py b/tests/test_client.py index da66d09..e1e9946 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -77,6 +77,103 @@ def test_client_error_raises_error_response(self, client, fixture_json): assert [e.code for e in error.errors] == ["API.320.200.145"] assert error.errors[0].merchantMessage == "Basket is already in use." + @responses.activate + def test_non_json_client_error_raises_error_response(self, client): + """A gateway in front of the API answers 4xx with an HTML page. + + That used to surface as a bare JSONDecodeError from inside the SDK, which hid + the status code and the body and looked like an SDK bug. Measured against the + live gateway: a returnUrl pointing at localhost or a private IP is refused with + a 403 and an nginx error page. + """ + html = ("\r\n403 Forbidden\r\n" + "\r\n

403 Forbidden

\r\n\r\n\r\n") + responses.add(responses.GET, f"{BASE}/payments/s-pay-1", body=html, status=403, + content_type="text/html") + with pytest.raises(ErrorResponse) as excinfo: + client.getPayment("s-pay-1") + error = excinfo.value + assert error.statusCode == 403 + assert not error.errors, "there is no error list to parse in an HTML body" + # Assert on the prefix, not on "403 Forbidden": that string also occurs twice + # inside the fixture body, so a message that dropped the status line entirely + # would still satisfy it. + assert str(error).startswith("HTTP 403 Forbidden with a non-JSON body:") + assert "403 Forbidden" in str(error), "the body is quoted" + assert error.srcResponse is not None + + # Raw JSON strings rather than `json=`, so the body is exactly what is written + # here -- `responses` turns `json=None` into an empty body, not into `null`. + @pytest.mark.parametrize("body", [ + pytest.param('{"message": "Forbidden"}', id="gateway-envelope"), + pytest.param("null", id="null"), + pytest.param("[]", id="list"), + pytest.param('"Forbidden"', id="bare-string"), + pytest.param('{"timestamp": "2026-08-21 10:15:32", "url": "u"}', + id="envelope-without-errors"), + ]) + @responses.activate + def test_client_error_with_a_foreign_json_body(self, client, body): + """A 4xx carrying JSON that is not this API's error envelope. + + An API gateway or WAF in front of the API answers in its own shape -- + ``{"message": "Forbidden"}`` is the canonical one. ``ErrorResponse.fromDict`` + indexes ``timestamp``/``url``/``errors`` and builds ``Error`` from required + positionals, so these used to escape as a bare ``KeyError``/``TypeError`` + from inside the SDK: the same failure the HTML case above was fixed for. + """ + responses.add(responses.GET, f"{BASE}/payments/s-pay-1", body=body, status=403, + content_type="application/json") + with pytest.raises(ErrorResponse) as excinfo: + client.getPayment("s-pay-1") + error = excinfo.value + assert error.statusCode == 403 + assert str(error).startswith("HTTP 403 Forbidden with a body the error schema") + + @pytest.mark.parametrize("timestamp", [ + pytest.param("2026-08-21 10:15:32", id="iso"), + pytest.param("21.08.2026 10:15:32", id="european"), + pytest.param("2026-08-21T10:15:32", id="iso-with-t"), + pytest.param("2026-08-21 10:15:32.123", id="milliseconds"), + pytest.param(None, id="missing"), + ]) + @responses.activate + def test_the_error_codes_survive_an_unreadable_timestamp(self, client, timestamp): + """The one field nobody branches on must not cost the ones they do. + + `createOrUpdateCustomer` decides on `errors[0].code`, so an error body whose + timestamp the SDK cannot read still has to arrive with its codes. Only the + first two formats parse; the rest leave `timestamp` at None. + """ + responses.add(responses.GET, f"{BASE}/payments/s-pay-1", status=400, json={ + "id": "s-err-1", "url": "u", "timestamp": timestamp, + "errors": [{"code": "API.320.200.145", "merchantMessage": "m", + "customerMessage": "c"}], + }) + with pytest.raises(ErrorResponse) as excinfo: + client.getPayment("s-pay-1") + error = excinfo.value + assert [e.code for e in error.errors] == ["API.320.200.145"] + assert error.errorId == "s-err-1" + assert error.statusCode == 400 + + @responses.activate + def test_an_incomplete_error_entry_keeps_its_code(self, client): + """An entry missing `merchantMessage`/`customerMessage` used to raise TypeError. + + `Error` tolerates *extra* keys with a warning, so refusing to build one over a + missing key was the wrong way round -- and it cost the whole list. + """ + responses.add(responses.GET, f"{BASE}/payments/s-pay-1", status=400, json={ + "id": "s-err-1", "url": "u", "timestamp": "2026-08-21 10:15:32", + "errors": [{"code": "API.320.200.145"}], + }) + with pytest.raises(ErrorResponse) as excinfo: + client.getPayment("s-pay-1") + error = excinfo.value + assert [e.code for e in error.errors] == ["API.320.200.145"] + assert error.errors[0].merchantMessage is None + @responses.activate def test_error_response_keeps_the_source_response(self, client, fixture_json): responses.add(responses.GET, f"{BASE}/payments/s-pay-1",