Basket discounts: what the API actually accepts, and error responses that survive an unexpected body - #13
Conversation
…hemas Neither schema accepts a basket item with a negative amount, so a discount cannot be sent as its own negative `voucher` line — the shape a consumer arrives at naturally. It belongs in a positive discount field of the item it reduces, and the two schemas differ in how much they verify: | | v1 | v3 | |---|---|---| | negative item amounts | `API.600.200.131` + `API.600.410.018` | `API.600.200.131` | | discount field | `amountDiscount`, per line | `amountDiscountPerUnitGross`, per unit | | total | not checked at all | exact to the cent, `API.600.410.062` | | `vat` per item | optional, stored as 0 | mandatory, `API.600.410.052` | | discount above the item | accepted | refused, the item may not go negative | `TestBasket` grows from 2 to 13 tests, one per row plus the per-unit semantics of the v3 discount. All of it measured against the sandbox, none of it derived from the documentation. `AGENTS.md` gets the same as prose. Also fixes the two existing basket tests: they passed `type="goods"`, but the parameter is `kind` — `BaseModel.__init__` swallows unknown kwargs, so `"type": ""` went out and no item type was ever exercised. A read-back assertion now pins it down.
Not every 4xx comes from the API. A gateway in front of it answers with an HTML error page, and `ErrorResponse.fromDict(r.json())` then raised a bare `JSONDecodeError` from inside the SDK — losing the status code and the body, and looking like an SDK bug rather than a refused request. The retry path already guarded against this; the client-error path did not. `_request` now falls back to an `ErrorResponse` carrying the status, the reason and the start of the raw body, so `statusCode` and `srcResponse` stay usable. Measured trigger, and the reason this surfaced: the gateway refuses any `returnUrl` pointing at `localhost`, `127.0.0.1` or a private IP with a 403 and an nginx page — which every local development server produces. Hostnames that merely resolve to 127.0.0.1 pass, so the check is on the string.
Review notesEverything below was reproduced against this branch's The direction is right and the basket tests are genuinely valuable. My concern is that the error-handling fix is mis-sized in both directions, and that the one branch it hardens is not the one that sees the most non-JSON bodies. 1.
|
timestamp |
result |
|---|---|
2026-08-21 10:15:32 |
ErrorResponse, 1 error — correct |
21.08.2026 10:15:32 |
ErrorResponse, 0 errors, "…with a non-JSON body: '{\"id\": \"s-err-1\"…'" |
2026-08-21T10:15:32 |
same |
2026-08-21 10:15:32.123 |
same |
So the message claims "non-JSON body" while quoting JSON, and errors is dropped. That is load-bearing: createOrUpdateCustomer (client.py:337) guards on if er.errors and er.statusCode == 400 and er.errors[0].code == "API.410.200.010" — with an empty list it re-raises instead of falling back to updateCustomer.
Note the SDK already owns a two-format parser, utils.parseDateTime, and payment.py:443, payment.py:723 and risk_check.py:52 all use it for the same field. ErrorResponse.fromDict is the only place that hard-codes one format, and it is now the one place whose failure is hidden.
Suggested: guard only the decode.
try:
data = r.json()
except ValueError:
errorResponse = ErrorResponse(...)
else:
errorResponse = ErrorResponse.fromDict(data)2. …and too narrow — a JSON body in a foreign shape still escapes
fromDict hard-indexes data["timestamp"], data["url"], data["errors"]. KeyError/TypeError are not ValueError, so they still leave _request bare — the exact failure class the commit message describes. Measured on a 403:
| body | escapes as |
|---|---|
{"message": "Forbidden"} |
KeyError: 'timestamp' |
null |
TypeError: 'NoneType' object is not subscriptable |
[] |
TypeError: list indices must be integers… |
"Forbidden" |
TypeError: string indices must be integers |
valid envelope, error entry {"code": "X"} |
TypeError: Error.__init__() missing 2 required positional arguments |
{"message": "Forbidden"} is the canonical AWS API Gateway 403; Cloudflare and Kong emit comparable shapes. The gateway in front of the API answers HTML today — nothing pins it there.
3. The retry-exhausted branch has the same defect, untouched
client.py:222-232 parses r.json() under its own except ValueError, then falls through to raise ErrorResponse("All request attempts failed", srcResponse=r) — which never sets statusCode. Measured with a 502 carrying an nginx page: statusCode == 0, no body, plus a full JSONDecodeError traceback at ERROR level. retryableMethods = ("GET", "HEAD"), and an HTML error page is at least as likely on 5xx as on 4xx, so this path probably sees the problem more often than the one that was fixed.
The two branches are now six near-identical lines with different behaviour. One _errorResponseFrom(r, message) helper called from both would fix this for free — and would also serve client.py:547 and client.py:650, which still call fromDict on a 2xx isError body. Worth noting that tests/fixtures/charge.json has none of timestamp/url/errors, and tests/test_client.py:112-116 injects them by hand to make that path assert.
4. r is never reset after a transport failure (pre-existing)
r = None is set once before the loop (client.py:171); the except (Timeout, ConnectionError) clause does continue without clearing it. A sequence 500 → timeout → timeout → timeout therefore builds the final ErrorResponse from attempt 1's response. That is worse than the empty statusCode above, because statusCode and srcResponse are populated and look trustworthy. One line in the except clause fixes it.
5. The 2xx path still parses unguarded — and twice
if 200 <= r.status_code <= 201:
logger.debug("Response[%s %s]: %r", r.status_code, r.reason, r.json())
return r.json()Response.json() does not memoise, and the %-style deferral only defers formatting, not argument evaluation — so every successful call pays two decodes plus two json.loads, with DEBUG off. Bind it once.
Separately, <= 201 routes every other 2xx into the client-error branch. Measured: a 204 No Content now raises ErrorResponse("HTTP 204 No Content with a non-JSON body: ''") with statusCode = 204. Before the PR that was an obvious JSONDecodeError; now a successful deleteCustomer is indistinguishable from a real rejection. I did not verify whether the API currently answers 204 anywhere — the three DELETE methods read data["id"], so today they get a body — but the misclassification is structural.
6. The new diagnostic does not reach any consumer
The fallback puts everything in the exception message and leaves errors == []. But ErrorResponse.__repr__ (model/error.py:93) renders only url, errorId, traceId, errors — all empty here. Measured:
repr: unzer.model.error.ErrorResponse(url=None, errorId=None, traceId=None, errors=[])
%r is this SDK's own logging style (client.py:178/179/197/201/206), and the documented downstream consumer logs exactly that and then iterates .errors. README.md:141 and examples/03_installment_plans.py:85 do the same. So for a .errors-reading caller the new error carries less than the JSONDecodeError it replaces.
Two options, either is fine: synthesize one Error entry (code = the HTTP status, merchantMessage = the body excerpt) so a raised ErrorResponse never has an empty list; or include args[0] and statusCode in __repr__. Both would also give callers a way to tell "the gateway refused this" from "the API refused this", which today is only reachable by string-matching the message.
Minor, same line: r.text[:200] decodes the full body to keep 200 chars, and for a text/html body without a charset requests decodes as ISO-8859-1 per RFC 2616 — a UTF-8 gateway page comes out as mojibake. r.content[:200] avoids both.
7. BasketItem.serialize does not round, Basket.serialize does
This one matters because of the docs this PR adds. basket.py:92-97 wraps every basket amount in roundAmount; basketItem.py:116-125 wraps none. Measured:
basket amountTotalGross: 0.3
item amountGross : 0.30000000000000004
item amountDiscount : 8.881784197001252e-16
roundAmount's own docstring says the API truncates past four decimals and does not accept scientific notation. The new AGENTS.md row requires totalValueGross == sum((amountPerUnitGross - amountDiscountPerUnitGross) * quantity) exact to the cent, i.e. it asks consumers to do exactly the arithmetic whose residue is no longer scrubbed. Spreading a 90.78 discount over 7 units gives 12.968571428571428 on the wire against a rounded total.
Tests
test_v3_discount_must_not_exceed_the_unit_pricedoes not isolate its claim. It sendstotalValueGross=self.GROSS - 1000.0, i.e. a negative basket total, and assertsAPI.600.200.131"Amount has to be positive" — the same codetest_v3_rejects_negative_item_amountsasserts for a negative item. The negative total alone explains the rejection. If the API dropped the per-item cap tomorrow the test stays green and the AGENTS.md rule derived from it silently becomes false. Adding a second, larger item keeps the total positive so only the one line goes negative.test_non_json_client_error_raises_error_responseis tautological.assert "403" in str(error) and "403 Forbidden" in str(error)— the fixture body itself contains<title>403 Forbidden</title>, so both substrings match even ifr.status_code,r.reasonand the body excerpt were all dropped from the message. Also the first clause is implied by the second.assert str(error).startswith("HTTP 403 Forbidden")plus a body-only substring would actually pin the format. Nothing covers the[:200]truncation.- The two vat tests cannot show what they claim.
serialize()emits every key of the chosen schema, so both send"vat": null— never an absent field. They prove "the API rejects an explicit null" and "v1 coerces null to 0", which is not the same as optional/mandatory. goods_v1/goods_v3are not "overridable per test". The defaults are passed as explicit keywords next to**overrides, so overriding one is aTypeError, not an override. Measured:amountDiscountPerUnitGross=works,vat=Noneandquantity=3raisegot multiple values for keyword argument. That is the mechanical reason five of the new tests build their item inline, and why the two pre-existing tests were edited rather than migrated. The repo already has the idiom — dict-merge with|, used atclient.py:146andbasketItem.py:115/120: build the defaults as a dict and callBasketItem(**defaults | overrides).- Hard-coded
orderIds. CONTRIBUTING.md:86 says "generate unique ids per run"; the 11 new baskets use fixed ids. The same file does it right 90 lines above (f"sdk-test-{uuid.uuid4().hex[:12]}", with a docstring explaining why), anduuidis already imported. Two pre-existing tests already deviate, so this extends rather than invents the deviation — but it also leaves eight permanent baskets on the account per run. - Three redundant round trips.
createBasketends withreturn self.getBasket(data["id"], api_version=…), so the returnedBasketalready is the read-back. Lines 296, 357 and 380 GET it again, same URL. Asserting onbasketdirectly keeps the docstrings true. - The
kind/typefix is not protected by CI. The only new assertion for it sits in the sandbox suite, whichpyproject.toml(addopts = "-m 'not sandbox'") and the workflow both deselect. A two-line mocked test onBasketItem(kind="goods").serialize()["type"]would guard it where it runs.
Docs
examples/03_installment_plans.py:57still hastype="goods". The commit fixes the two tests and names the root cause, but leaves the only public example of building a basket item shipping"type": "". That is what a downstream user copies.BaseModel.__init__swallows unknown kwargs without a word (model/base.py:33), whileError.__init__andErrorResponse.__init__bothlogger.warningon extras. Measured:BasketItem(type="goods", quantitiy=3)yieldskind=None,"type": ""andquantity: None, silently. Given the camelCase→snake_case rename announced for 2.0, this will bite again. Mirroring theErrorwarning inBaseModelwould have surfaced the test bug the day it was written (it needsBasketItem.fromDict/Basket.fromDictto pop the keys they deliberately pass through).- AGENTS.md:215 contradicts
basketItem.py:47. The table says v1amountDiscountis "per line"; the docstring says "Discount amount for the basket item (multiplied by thequantity)", which reads as per unit. Every v1 discount test usesquantity=1, where the two are indistinguishable — while the v3 side does get the discriminating test. Since v1 reconciles nothing, the basket endpoint cannot settle this; per AGENTS.md:87 it should either be measured downstream or marked unverified. basketItem.py:50saysvatis(optional)unconditionally, which AGENTS.md:218 and the new test now contradict for v3. Every other schema-specific field in that docstring carries a(v1)/(v3)tag.- AGENTS.md:52 is in the "confirmed by real sandbox calls" table but is not covered by a test. The row ends "…and a charge does not compare the basket to the payment amount either". The test docstring it comes from says "(measured with iDEAL). That is not tested here", and the reason given (it would create a payment per run) is sound. What I could not resolve from the repo is how it was measured: both
tests/sandbox/test_live_api.py:10-16and AGENTS.md:130-135 state that iDEAL cannot be exercised server-side. A manually createdtypeIdfrom the Payment Page would explain it and would not be a contradiction — but as written a reader cannot tell. Worth a sentence, because this is the row a consumer leans on to skip validating a basket against a charge amount. - The discount rules live only in AGENTS.md. Its own "Where documentation goes" table sends user-facing detail to
README.md/docs/*.md;docs/payment-methods.mdstill describes basket construction with no discount guidance. The named downstream consumer is exactly the audience that models a voucher as a negative line item, and has no reason to open a file headed "Guidance for AI coding agents".
Follow-up: sandbox measurements, and two corrections to my own reviewI went and measured the things above that I had taken from a docstring. Two of my points were wrong, and the measurements turned up something the PR should probably cover. Correction 1: the
|
| payload | POST /v1/baskets |
POST /v3/baskets |
|---|---|---|
| v1 shape | 201, stored correctly | 400 API.600.410.051 |
| v3 shape | 201 — every amount stored as 0.0000 |
201, stored correctly |
So a basket built as Basket(amountTotalGross=…, basketItems=[BasketItem(amountPerUnitGross=…)]) — v1 total, v3 item — serializes to the v1 shape with amountGross: null, goes to /v1/baskets, and comes back as a basket worth 0.0000 with no error anywhere. Given the model happily accepts any mix of v1 and v3 attributes, and given that BaseModel.__init__ also swallows unknown kwargs, this is easy to hit and impossible to notice.
A validation in Basket.serialize() (or validateBeforeRequest) that refuses a basket mixing v1 and v3 item fields would close it, and unlike a returnUrl blocklist it does not encode any policy that Unzer could relax later — the two schemas are mutually exclusive by construction.
New: a basket can only be used once
API.330.200.152 "Resources: basket was used." on the second charge against the same basketId. Worth a row in the AGENTS.md table: a consumer that retries a failed authorize has to create a new basket, which is not obvious and is exactly what a checkout retry does.
The charge/basket claim (AGENTS.md:52) is true — and now testable
Four charges against four identical baskets whose amountTotalGross is 817.02:
amount 817.02 ACCEPTED booked=817.02
amount 726.24 ACCEPTED booked=726.24
amount 907.80 ACCEPTED booked=907.80
amount 1.00 ACCEPTED booked=1.00
So the charge really does not reconcile against the basket. The reason the row could not be tested was given as "it would create a payment on the account for every run" — but this used Prepayment, which is created server-side, so unlike the iDEAL measurement it can live in the sandbox suite. That would turn the one unmeasured row in the table into a measured one, and remove the tension with the two places that say iDEAL cannot be driven server-side.
…oes not fit
The `except ValueError` guarding `ErrorResponse.fromDict(r.json())` was wrong in
both directions.
Too narrow: `fromDict` indexes `data["timestamp"]`, `data["url"]` and
`data["errors"]` and builds `Error` from required positionals, so a 4xx whose
body *is* JSON but is not this API's envelope raised `KeyError`/`TypeError` --
neither of them a `ValueError`. Measured, all of these still escaped `_request`
bare: `{"message": "Forbidden"}` (the canonical API-gateway shape), `null`,
`[]`, `"Forbidden"`, and an error entry missing a field. That is the same
failure the commit set out to remove, one body shape over.
Too wide: `fromDict` parses the timestamp with a single hard-coded format, so a
well-formed API error with any other one was reported as "non-JSON body" --
while quoting JSON -- and lost its `errors` list on the way. The root of that
belongs in `fromDict`; here it at least no longer claims the body was not JSON,
and it is logged instead of swallowed.
Only `r.json()` is guarded now, and the two causes are reported separately.
`test_v3_discount_must_not_exceed_the_unit_price` sent an over-discounted item *and* a negative `totalValueGross`, then asserted `API.600.200.131` "Amount has to be positive" -- the same code `test_v3_rejects_negative_item_amounts` asserts for a negative item. The negative total alone explains it, so the test could not show that the item was what the API refused. Measured with a second, larger item keeping the total positive: the API then answers `API.600.410.064` and names the line, "Basket item i1 'amountDiscountPerUnitGross' does not equal to 'amountPerUnitGross'". So the cap is real, but the test was asserting the wrong code for the wrong reason. Also correct the module docstring's source for "a charge does not validate the basket against the payment amount": that was attributed to a measurement with iDEAL, which the same file states cannot be exercised server-side. Re-measured with Prepayment, which can: against one basket worth 817.02 the amounts 817.02, 726.24, 907.80 and 1.00 are all accepted and booked at face value.
Four claims measured against the sandbox, three of them wrong: - **The Pay later methods do not require v3.** A sandbox authorize with `PaylaterInstallment` and with Klarna succeeds on a v1 basket carrying `amountDiscount` just as it does on v3, booking the requested amount unchanged. The `Basket` docstring asserted the opposite. - **Mixing the schemas fails silently in one direction.** A v3 item in a v1 basket is accepted with a 201 and every item amount stored as `0.0000`; the reverse is refused loudly with `API.600.410.051`. A basket is also only readable through the schema it was created with (`API.600.410.024`). - **`vat` is mandatory in v3**, not "optional" as `BasketItem` claimed -- `API.600.410.052`, which this branch's own test already showed. - **A basket is single use**: `API.330.200.152 "Resources: basket was used."` on a second charge, so a retry after a failed authorize needs a new one. Also record the error code the per-item discount cap really produces (`API.600.410.064`, and only when the basket total stays positive), and replace the iDEAL attribution on the charge-does-not-check-the-basket row with the Prepayment measurement, which is reproducible server-side.
Applied on the branchScope kept to what this PR touches; everything else is filed.
|
`ErrorResponse.fromDict` was the strictest parser in the package, on the one path that runs when something has already gone wrong: it indexed `timestamp`, `url` and `errors`, parsed the timestamp with a single hard-coded format, and built `Error` from three required positionals. Any of those failing cost the whole error. That matters because the codes are the part a caller acts on -- `UnzerClient.createOrUpdateCustomer` falls back to `updateCustomer` on `errors[0].code == "API.410.200.010"`, which is on the live checkout path of every Unzer provider in viur-shop. Since the previous commit a parse failure no longer crashes, so the loss had become silent: the guard saw an empty list and re-raised. Now only `errors` is required -- it is what identifies the body as this API's error envelope -- and everything else missing or unreadable costs that field alone. The timestamp goes through `utils.parseDateTime`, which knows both formats the API uses, and falls back to `None` with a warning rather than taking the codes down with it. `Error` gained defaults, so an entry missing `merchantMessage` keeps its code; it already tolerated *extra* keys, so refusing over a missing one was the wrong way round. A body that is not an envelope still raises, so the caller keeps its "the API refused this" versus "something else answered" distinction. Measured across `2026-08-21 10:15:32`, `21.08.2026 10:15:32`, `2026-08-21T10:15:32`, `2026-08-21 10:15:32.123` and a missing timestamp: the codes arrive in all five and the `createOrUpdateCustomer` fallback fires again. Closes #14
The table claimed `amountDiscount` is per line while `basketItem.py:47` documents
it as per unit ("multiplied by the `quantity`"). Nothing here settles it: every v1
discount test uses `quantity=1`, where the two are indistinguishable, and v1
reconciles nothing, so the basket endpoint cannot be made to answer. Only the
rendered basket or the partner system can -- until then this repository's own rule
applies and it is labelled rather than asserted.
`Error.__init__` gained its defaults here and `fromDict`/`_parseTimestamp` were rewritten, so they fall under "type hints everywhere" -- the surrounding older models being unannotated is not a reason to add more of them.
The module-level `import datetime` had exactly one user left after this branch replaced the `strptime` call: the type hint on `_parseTimestamp`. Matches the existing `from datetime import datetime as dt` in `additional_transaction_data`.
Four statements did not survive the sandbox runs on this branch: - "Amounts are rounded to four decimals on serialisation" holds for the basket, not for the items -- `BasketItem.serialize` rounds nothing, so an item goes out as `0.30000000000000004` next to a total rounded to `0.3` (#18). Worth stating precisely right here, because the v3 reconciliation this file documents is exact to the cent. - The schema table read as if each method required a particular version. Measured, none of them does; the column records what was seen to work. - Mixing the schemas within one basket was warned about in the `Basket` docstring but not here, and only one direction fails loudly: a v3 item in a v1 basket is accepted with a 201 and every item amount stored as `0.0000` (#16). - `TestBasket` no longer covers "all of this": the v1 per-line/per-unit question is labelled unverified and no test can settle it.
Chased the per-line/per-unit question 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. No consumer evaluates the field: the v1 endpoint stores it and reconciles nothing, a charge ignores the basket, Klarna lists the item by title and takes its total from the authorize amount, and the Hosted Payment Page renders the line as `3x T-Shirt € 300,00` -- the undiscounted gross -- with its total taken from the request as well. The distinction is therefore undecidable *and* inconsequential, which is a more useful thing to record than the guess it replaces. It also means a discounted line is shown to the customer at full price next to a lower total, with nothing naming the difference.
…egative items (#202) A basket-domain discount was sent to Unzer as its own line item with negative amounts. Unzer refuses that, so every Klarna or Paylater-Installment checkout with a discount in the cart failed. ## The problem `build_discount_item()` emitted the discount as `kind="voucher"` with `amountPerUnit`/`amountNet`/`amountGross` set to `-amount`. Both basket schemas reject negative item amounts: ``` POST /v1/baskets → 400 API.600.200.131 Amount -90.78 has to be positive (per amount field) API.600.410.018 Basket item has negative gross amount ``` The shape is the obvious one — one item per article, one for the voucher, the grosses adding up to the order total — which is why it looked right. It fails deterministically, just rarely, because a basket discount and a BNPL method seldom meet. ## Changes - **`spread_discount()`** puts the discount where the API wants it: a positive `amountDiscount` on the items it reduces. Since the field hangs off an item, a node discount has to be split, and the split is done in whole cents — each item gets the floor of its exact share, the cents left over by rounding go one each to the items with the largest fractional part. The parts add up to the discount exactly, which the v3 endpoint requires. Weights are what is left of an item (gross minus discounts already spread on it by a nested node), so no item can be pushed below zero; a discount exceeding the whole basket raises instead of producing a silently wrong basket. - **`build_node_items()`** replaces the flat queue walk and mirrors the `total_discount_price` computation of `CartNodeSkel` per node: the discount applies to the subtree, the node's own shipping is added afterwards and stays undiscounted. - The discount **amount** is no longer recomputed from the discount skeleton but taken as the difference between the items and the node's stored `total_discount_price`. An ordered cart is frozen, so that value is what the customer is charged even if the discount changed afterwards — and the basket keeps reconciling whatever the discount type does. A difference without a basket discount is logged rather than skewing the basket silently. - **`has_basket_discount()` reads the frozen discount**, for the same reason. The bone is `RelationalConsistency.SetNull` and core's `update_relations` refreshes it, so deleting the discount entity — or re-scoping its condition to another `application_domain` — after the order was frozen turned a reduction the customer is still charged for into an error log and a basket overstating the order by the full discount. Note the trap this needs to avoid: `freeze_cart` stores the *dumped* relation, where `application_domain` is a plain string, while the live relation yields the enum member — and `ApplicationDomain` is a plain `enum.Enum`, so `"basket" == ApplicationDomain.BASKET` is `False`. Reading `frozen_values` without normalising would have reported "no basket discount" for every frozen cart, i.e. dropped the discount entirely. - `get_basket_id()` reports the resulting `amountTotalDiscount`. `build_discount_item()` is gone, `has_basket_discount()` is new. ## Verification Measured against the Unzer sandbox, since neither the documentation nor a `201` from `POST /baskets` settles what a payment method does with a basket: - **Real authorizes with the exact shape this PR builds** — v1, `amountTotalGross` already reduced, `amountTotalDiscount` reported, the discount as a positive `amountDiscount` on the items: ``` Klarna, v1 + amountDiscount AUTHORIZED booked=817.02 pending=True Paylater Installment, v1 + amountDiscount AUTHORIZED success=True Paylater Installment, v3 equivalent AUTHORIZED success=True ``` `booked` is the authorize amount unchanged, so the discount is not applied twice. This also disproves the claim in the SDK's `Basket` docstring that the Pay later methods require the v3 schema — corrected upstream in mausbrand/unzer-python-sdk#13. - **A charge does not reconcile against its basket at all**: charges of 817.02, 726.24, 907.80 and 1.00 against four identical baskets worth 817.02 were all accepted at face value. So nothing on the Unzer side would have caught a wrong basket — which is why the authorizes above, not the basket creation, are the evidence that matters. - **The cent distribution** in `spread_discount` was checked by brute force over 200k randomized carts: the shares always sum to the amount, no item is pushed below zero, the loop terminates. - Eight real carts of a shop running this code, rebuilt with the patched `spread_discount` and sent to the sandbox: accepted in both schemas, item sums equal to the order total in all eight. The previously refused basket now passes, its 90.78 split into 59.89 and 30.89. - `pycodestyle` with the project config: no finding in the changed lines. The unit tests on the spreading arithmetic that this description previously claimed are **not in the repository** — it has no test directory, no runner in `Pipfile [dev-packages]` and no CI test job. `spread_discount` is a pure `@staticmethod` over plain `BasketItem` objects and is the best possible candidate for the first one; filed as #205. ## Known gaps, deliberately not addressed here - **The discount loses its name in the customer-visible basket** — measured, not assumed. The removed voucher item carried `title=discount["dest"]["name"] or "Discount"`; `amountDiscount` has no label and nothing replaces it. I put an item of `quantity=3`, gross 300.00 and `amountDiscount=10.00` through both renderers: - Unzer's Hosted Payment Page shows the line as `3x T-Shirt € 300,00` — the **undiscounted** gross — next to a total of `€ 290,00`, with nothing explaining the 20.00 difference. - Klarna's checkout lists the item by title only, with no price at all, and takes its total from the authorize amount. So neither renderer evaluates `amountDiscount`; the customer sees full-price lines above a lower total. Whatever we decide, it should be a decision rather than a side effect. - **`BASKET_ITEM_VOUCHER` is left behind with no producer.** Nothing emits a voucher line any more, so the constant is the last remnant of the removed builder. Dropping it would be the tidier end state; kept out of this diff to hold it to the fix. - **`spread_discount()` raises `ValueError`** when the discount exceeds the items, which surfaces as an HTTP 500 on the customer's "order now" after the order UID is assigned. Kept deliberately: such a cart is a data error and should stop the checkout rather than ship a knowingly wrong basket. Reachable because `DiscountSkel.absolute` has no upper bound and `Price.apply_discount` does not clamp at zero. - Basket item titles reach the provider HTML-escaped. An earlier commit on this branch unescaped them at the call site; that was reverted, because it assumes every project's article bone escapes, which `ArticleAbstractSkel` does not require. Filed as #204 with a sketch for reconciling it in `Cart.copy_article_values`, which is the one place that sees both bone definitions. ## Related #203 (the basket builders read live prices and shipping against a frozen cart total — same class as the `has_basket_discount` bug fixed here), #204 (escaping), #205 (no tests), #206 (pre-discount VAT and the unset `amountTotalVat`), #200 (`DiscountType.FREE_SHIPPING` raises in `Price.apply_discount()` — no longer reachable from this path, since the amount now comes from the stored total), #201 (cart discounts are never revalidated).
Three findings from tracking down a failing Klarna checkout, all measured against the
sandbox.
How a discount has to reach a basket
Neither schema accepts a basket item with a negative amount — so a discount cannot be
sent as its own negative
voucherline, which is the shape a consumer arrives atnaturally (one item per article, one for the voucher, the grosses adding up to the
order total). It belongs in a positive discount field of the item it reduces, and the
two schemas verify very different amounts of it:
API.600.200.131+API.600.410.018API.600.200.131amountDiscount(per line or per unit is unverified, see below)amountDiscountPerUnitGross, per unitamountGrossstays the pre-discount grossamountPerUnitGrossminus the discount must stay positiveAPI.600.410.062vatper itemAPI.600.410.052API.600.410.064TestBasketgrows from 2 to 13 tests, one per row plus the per-unit semantics of thev3 discount.
AGENTS.mdcarries the same as prose, with the v1 tolerance marked as awarning rather than a licence: a charge does not compare the basket to the payment
amount either, but a method that hands the basket to a partner system may well be
stricter.
Two things about the last row are worth stating precisely, because a first version of
it was wrong. The per-item cap only shows itself when the basket total stays positive:
with a single over-discounted line the total goes negative too and the API answers the
generic
API.600.200.131"Amount has to be positive", which the negative total aloneexplains. With a second, larger item it names the line instead —
API.600.410.064,"Basket item i1 'amountDiscountPerUnitGross' does not equal to 'amountPerUnitGross'".
And the charge-does-not-check-the-basket claim is now backed by Prepayment rather than
iDEAL, which this repository states cannot be driven server-side: against one basket
worth 817.02, charges of 817.02, 726.24, 907.80 and 1.00 were all accepted and
booked at face value.
The two existing basket tests are fixed along the way: they passed
type="goods", butthe parameter is
kind.BaseModel.__init__swallows unknown kwargs, so"type": ""went out and no item type was ever exercised.
Error responses that survive an unexpected body
Not every 4xx comes from the API. A gateway in front of it answers with an HTML page,
and parsing that as JSON raised a bare
JSONDecodeErrorfrom inside the SDK — losingthe status code and the body, and looking like an SDK bug rather than a refused
request.
Guarding that turned out to need three steps rather than one, because the first
attempt was both too narrow and too wide:
fromDictindexedtimestamp,urlanderrors, so a 4xx whosebody is JSON but is not this API's envelope raised
KeyError/TypeError— not aValueError. Measured, these all still escaped_requestbare:{"message": "Forbidden"}(the canonical API-gateway body),null,[],"Forbidden".fromDictparsed the timestamp with one hard-coded format while theAPI is known to use two, and that
ValueErrorlanded in the same handler. Awell-formed API error with a different format was reported as "non-JSON body" —
while quoting JSON — and arrived with an empty
errorslist.The second is the one that mattered:
UnzerClient.createOrUpdateCustomerfalls back toupdateCustomeronerrors[0].code == "API.410.200.010", so an emptied list turns arecoverable checkout into a failed one, silently. That path is exercised routinely in
production.
So
_requestnow guards onlyr.json()and reports a body that decodes but does notfit the schema separately, and
ErrorResponse.fromDicttreats onlyerrorsasrequired — it is what identifies the body as an error envelope. Everything else
missing or unreadable costs that field alone: the timestamp goes through
utils.parseDateTimeand falls back toNonewith a warning, andErrorgaineddefaults so an entry missing
merchantMessagekeeps its code. A body that is not anenvelope still raises, so callers keep the distinction between "the API refused this"
and "something else answered".
Measured across
2026-08-21 10:15:32,21.08.2026 10:15:32,2026-08-21T10:15:32,2026-08-21 10:15:32.123and a missing timestamp: the codes arrive in all five and thecreateOrUpdateCustomerfallback fires again. Closes #14.The trigger that surfaced all this is worth knowing for anyone developing locally:
returnUrlhttp://localhost:8080/…http://127.0.0.1:8080/…,https://localhost:8080/…http://192.168.1.5:8080/…https://example.com/…http://lvh.me:8080/…,http://localtest.me:8080/…,http://127-0-0-1.nip.io:8080/…The gateway refuses localhost and private IP literals before the API sees the request.
Hostnames that merely resolve to 127.0.0.1 pass, so the check is on the string. A
rate limit is ruled out:
localhoststill gets a 403 right after a successful callwith a public host.
Documentation claims corrected by measurement
Three assertions in the docstrings and in
AGENTS.mddid not survive being checked:PaylaterInstallmentand with Klarna succeeds on a v1 basket carryingamountDiscountexactly as it does on v3, booking the requested amount unchanged.The
Basketdocstring and the schema table both claimed otherwise.vatis mandatory in v3, not "optional" asBasketItemsaid.accepted with a 201 and every item amount stored as
0.0000; the reverse is refusedloudly with
API.600.410.051. A basket is also only readable through the schema itwas created with (
API.600.410.024), and single use —API.330.200.152on a secondcharge, so a retry after a failed authorize needs a new one. The silent-zeroing case
is filed as A v3 item in a v1 basket is accepted and silently stored as
0.0000#16, since enforcing it belongs inBasket.serialize.The v1
amountDiscountsemantics are now labelled unverified rather than asserted:the table said per line,
basketItem.py:47says per unit, every v1 discount test usesquantity=1where the two are indistinguishable, and v1 reconciles nothing — so thebasket endpoint cannot settle it. Only the rendered basket or the partner system can.
Verification
pytest -m sandbox tests/sandbox/test_live_api.py::TestBasket— 13 passed againstthe real sandbox.
pytest— 291 passed, including the new mocked cases for the HTML body, the fiveforeign-JSON shapes, the five timestamp formats and the incomplete error entry.
pycodestyle --diff— clean.stashing):
TestPaymentPage::test_redirect_url_points_at_the_sandboxexpectssbx-in the redirect URL, the API now answers
payment.test.unzer.com.Filed rather than fixed here
#15 (the remaining
_requestbranches: retry-exhaustedstatusCode == 0, theunguarded and doubled
r.json()on 2xx,<= 201sending a 204 into the error branch,rnot reset after a transport failure), #16 (schema mix), #17 (BaseModelswallowsunknown kwargs —
examples/03_installment_plans.py:57still carries thetype=bugthis branch fixed in the tests), #18 (
BasketItem.serializedoes not round itsamounts while
Basket.serializedoes).Note on the target branch
Against
develop, notmain, although the commits are patch level:maincarriesneither
tests/norAGENTS.mdyet — #8 through #12 are still only ondevelop, sothe files this branch changes do not exist there.