fix(transport): accept a successful empty 204 instead of raising (#103)

[karlwaldman]

The bug

Both clients parsed every 2xx with a bare response.json() — three sites: client.py:300, client.py:449 (request_with_headers) and async_client.py:262.

The Rails webhooks controller answers destroy with head :no_content, a 204 with an empty body. So:

client.webhooks.delete("fixture-id")
# json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

The deletion succeeded and the caller was told it failed. That is the worst shape of error for a mutation, because the obvious recovery is to retry a delete that already worked.

The fix

One shared decode_json_body helper in a new oilpriceapi/_body.py, backing all three decode sites. A single helper rather than three copies of the branch is the point — three copies drift, and this repo's sync/async parity is worth keeping.

It decodes first and inspects the body only when that fails:

try:
    return response.json()
except ValueError:
    if not (response.content or b"").strip():
        return {}
    raise

Checking .content up front was the first version. It worked, but it touched the body on every successful request and made the helper sensitive to how a response is represented rather than what it contains — the existing test_async_client.py mocks surfaced that as 7 RuntimeWarning: coroutine ... was never awaited. Decode-first has neither problem and the same semantics.

Deliberately narrow

Case	Result
204 / empty body on any 2xx	success, {}
whitespace-only body	success, {}
malformed non-empty 200	still raises
401 / 403	typed error, unchanged
204	not retried, retry budget untouched

The malformed-non-empty row is the one that matters. A real parse failure must not be laundered into an empty success — that would hide a broken response, which is the mirror image of the bug being fixed. Two tests pin it, one per client.

delete keeps its documented -> None contract.

Red

Tests written first, against unmodified main:

========================= short test summary info =========================
FAILED test_sync_delete_accepts_204_no_content
FAILED test_async_delete_accepts_204_no_content
FAILED test_sync_request_returns_empty_dict_for_204
FAILED test_async_request_returns_empty_dict_for_204
FAILED test_request_with_headers_accepts_204
FAILED test_any_2xx_with_an_empty_body_is_success[200]
FAILED test_any_2xx_with_an_empty_body_is_success[202]
FAILED test_any_2xx_with_an_empty_body_is_success[204]
FAILED test_whitespace_only_body_is_treated_as_empty
FAILED test_204_is_not_retried - json....
FAILED test_all_three_decode_sites_share_one_helper
========================= 11 failed, 5 passed in 0.34s ========================

Re-proved red-capable after the fix by restoring the pre-fix sources and deleting the new module (git checkout origin/main -- oilpriceapi/client.py oilpriceapi/async_client.py; rm oilpriceapi/_body.py) — that is the run above. The 5 that passed before the fix are the negative paths: malformed 200 and 401/403 already behaved correctly and must keep doing so.

Green

tests/unit/test_empty_204_response.py ................                   [100%]
============================== 16 passed in 0.14s ==============================

Sync and async parity

Every positive case is asserted on both clients: test_sync_delete_accepts_204_no_content / test_async_delete_accepts_204_no_content, test_sync_request_returns_empty_dict_for_204 / test_async_request_returns_empty_dict_for_204, and the malformed-200 and 401 negatives likewise.

test_all_three_decode_sites_share_one_helper pins it structurally: both modules must reference decode_json_body, and neither may contain a bare return response.json() or return response.json(), response.headers. A future edit that reintroduces a direct decode in one client fails the test.

Verified against the backend contract named in the issue: v1/webhooks_controller.rb destroy returns head :no_content. Tested against a disposable respx fixture — no production deletes.

A note on test mocking

The first push used respx, which is not in this repo's [dev] extra, so CI failed with ModuleNotFoundError on all five Python versions. A follow-up commit switches to patching httpx.Client.request / httpx.AsyncClient.request, which is this repo's existing convention (tests/unit/test_diesel_envelope.py). Same assertions, same red counts — no test dependency added for a handful of assertions.

Full suite

	passed	failed	skipped
before (origin/main)	864	3	63
after	880	3	63

+16 is exactly this PR's new tests. The 3 failures are tests/integration/test_demo_contract.py making live calls and getting HTTP 429 — environmental, identical before and after.

git diff --stat: 2 files changed, 5 insertions(+), 3 deletions(-), plus the new _body.py and the new test file. No CRLF normalisation of resources/alerts.py or resources/diesel.py.

Version deliberately left at 1.14.0; release decision is separate.

Closes #103.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ao5paex73xXvuM424Libo

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 087461c0-10e3-42dd-a48b-4e01f8ffc03c

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}