Optionally raise instead of swallowing JSON decode errors

[jakobovski-arb]

The problem

_get and _paginate_iter both decode responses like this:

try:
    obj = self._decode(resp)
except ValueError as e:
    logger.error("Error decoding json response: %s", e)
    return []

A truncated or malformed body therefore becomes an empty result, which a caller cannot distinguish from "the API had nothing to return". The only trace is a log line, and the call itself succeeds.

In _paginate_iter this is worse than an empty result: the bare return ends the generator early, so a partial page silently truncates a result set that still looks complete to the caller.

This is not hypothetical. In a single multi-hour backfill over /v3/trades, we logged 12 of these:

RESTClient ERROR: Error decoding json response: Unterminated string starting at: line 1 column 570332 (char 570331)
RESTClient ERROR: Error decoding json response: Expecting ',' delimiter: line 1 column 190527 (char 190526)
...

Each one produced an empty page that our pipeline recorded as a real absence — "this ticker had no auction print that day" — and cached permanently, because an empty response from a working API is a legitimate answer in our domain. We only found out by reading the log afterwards. For data ordered newest-first, where the row of interest sits on the last page, a truncated early page can drop exactly the record you were querying for.

The change

RESTClient(raise_on_decode_error=True) raises a new ResponseDecodeError from both decode sites instead of returning []. The flag threads to VXClient; other BaseClient subclasses inherit it through RESTClient's super().__init__().

Default is False, so existing behaviour is unchanged and this is not a breaking change.

Caveats, in the interest of full disclosure

• We think raising should eventually be the default. Converting an undecodable body into a successful empty result is a correctness hazard for any consumer that treats "empty" as data, and there is currently no way to detect it programmatically at all. We made it opt-in only to keep the change non-breaking — if you'd prefer it default-on for a major version, or gated by an env var / module-level default instead of a constructor kwarg, we're happy to rework it.
• The _paginate_iter half is the more important one, and it is arguably a bug rather than a design choice: silently returning a truncated result set is different in kind from returning an empty one, and no flag setting makes a partially-consumed generator obvious to the caller. If you'd rather fix only that site, or always raise there while keeping _get lenient, that would still address the dangerous case.
• A retry would be the natural follow-up. A truncated body is usually transient, so the SDK could reasonably retry a failed decode the way urllib3 retries a failed connection, rather than pushing that on every caller. We didn't include it here to keep this diff to one concern.
• Naming is yours to pick. raise_on_decode_error / ResponseDecodeError seemed closest to the existing BadResponse, but rename freely.
• Test note: test_rest/models/test_requests.py::RequestTest::test_clint_headers_concat already fails on a clean master checkout in our environment (verified by stashing this change), so it is unrelated to this PR. The other 47 tests pass, including the 4 added here.

Tests

test_rest/test_decode_errors.py covers: the default still returns an empty result, the opt-in raises ResponseDecodeError, the flag reaches client.vx, and a well-formed body is unaffected either way.

---

Context: we're the authors of #1026 (maxsize on the connection pool). Both issues surfaced from the same workload — a high-concurrency historical backfill of /v3/trades — where silent data loss is expensive and hard to detect after the fact.

🤖 Generated with Claude Code