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

Both clients parsed every 2xx with a bare `response.json()`. The Rails
webhooks controller answers `destroy` with `head :no_content` -- a 204 with an
empty body -- so `client.webhooks.delete(...)` raised JSONDecodeError. The
deletion had SUCCEEDED and the caller was told it failed, which is the worst
shape of error for a mutation: the obvious recovery is to retry a delete that
already worked.

One shared `decode_json_body` helper now backs all three decode sites --
`client.request`, `client.request_with_headers` and `async_client.request` --
so the sync and async paths cannot drift. It decodes first and inspects the
body only when that fails, which keeps the body untouched on the hot path.

Deliberately narrow:
  - an empty or whitespace-only body on any 2xx is success, returning {};
  - a MALFORMED NON-EMPTY body is still an error, never laundered into an
    empty success;
  - 401/403/429 typed errors are untouched, and a 204 is not retried.

`delete` keeps its documented `-> None` contract.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo

Author: karlwaldman

oilpriceapi/_body.py

@@ -0,0 +1,53 @@
+"""Decode a successful response body, tolerating a legitimately empty one (#103).
+
+Both clients used to parse every 2xx with a bare ``response.json()``. The Rails
+webhooks controller answers ``destroy`` with ``head :no_content`` -- a 204 with
+an empty body -- so ``client.webhooks.delete(...)`` raised ``JSONDecodeError``.
+The deletion had SUCCEEDED and the caller was told it failed, which is the worst
+shape of error for a mutation: the obvious recovery is to retry a delete that
+already worked.
+
+The rule is deliberately narrow. An empty body on a 2xx is success. A malformed
+NON-EMPTY body is still an error -- that is a real parse failure and laundering
+it into an empty success would hide a broken response.
+"""
+
+from __future__ import annotations
+
+from typing import Any, Dict
+
+import httpx
+
+__all__ = ["decode_json_body"]
+
+
+def decode_json_body(response: httpx.Response) -> Dict[str, Any]:
+    """Return the decoded 2xx body, or ``{}`` when the server sent none.
+
+    Args:
+        response: A response whose status is already known to be 2xx.
+
+    Returns:
+        The decoded JSON body, or an empty dict for a no-content response.
+
+    Raises:
+        Whatever ``response.json()`` raises for a non-empty body that is not
+        valid JSON. A malformed response is still a failure.
+    """
+    # Decode first and inspect the body only when that fails. Checking
+    # `.content` up front would touch the body on every successful request --
+    # needless work on the hot path, and it makes the helper sensitive to how a
+    # response is represented rather than to what it contains.
+    try:
+        return response.json()
+    except ValueError:
+        # 204 No Content and 304 Not Modified are DEFINED to carry no body, and
+        # some servers answer a mutation with a zero-length 200 or 202. A body
+        # of only whitespace is as empty as a zero-length one. In every one of
+        # those cases the request SUCCEEDED and there is simply nothing to
+        # decode, so an empty dict is the honest answer.
+        if not (response.content or b"").strip():
+            return {}
+        # A malformed NON-EMPTY body is a real parse failure. Laundering it into
+        # an empty success would hide a broken response from the caller.
+        raise

oilpriceapi/async_client.py

@@ -17,6 +17,7 @@
 logger = logging.getLogger(__name__)
 
 from ._subscriptions_common import unwrap_data
+from ._body import decode_json_body
 from ._url import resolve_api_url
 from .async_resources import (
     AsyncAlertsResource,
@@ -259,7 +260,7 @@ async def request(
                         duration=_time.time() - start_time,
                         success=True,
                     )
-                    return response.json()
+                    return decode_json_body(response)
                 if response.status_code == 429:
                     retry_after = response.headers.get("Retry-After")
                     logger.warning(

oilpriceapi/client.py

@@ -17,6 +17,7 @@
 logger = logging.getLogger(__name__)
 
 from ._subscriptions_common import unwrap_data
+from ._body import decode_json_body
 from ._url import resolve_api_url
 from .exceptions import (
     ConfigurationError,
@@ -297,7 +298,7 @@ def request(
                         duration=time.time() - start_time,
                         success=True,
                     )
-                    return response.json()
+                    return decode_json_body(response)
                 if response.status_code == 429:
                     retry_after = response.headers.get("Retry-After")
                     logger.warning(
@@ -446,7 +447,7 @@ def request_with_headers(
                 )
 
                 if 200 <= response.status_code < 300:
-                    return response.json(), response.headers
+                    return decode_json_body(response), response.headers
                 if response.status_code == 429:
                     retry_after = response.headers.get("Retry-After")
 

tests/unit/test_empty_204_response.py

@@ -0,0 +1,171 @@
+"""A successful 204 must not be reported to the caller as a failure (#103).
+
+Both clients parse EVERY 2xx with `response.json()`. The Rails webhooks
+controller answers `destroy` with `head :no_content` -- a 204 with an empty
+body -- so `client.webhooks.delete(...)` raises JSONDecodeError. The deletion
+SUCCEEDED and the caller was told it failed, which is the worst shape of error
+for a mutation: the obvious recovery is to retry a delete that already worked.
+
+Narrow by design:
+  - an empty body on a 2xx is success, and `delete` keeps returning None;
+  - a MALFORMED NON-EMPTY 200 stays an error -- it is a real parse failure and
+    must not be laundered into an empty success;
+  - typed errors for 401/403/429 are untouched.
+"""
+
+import httpx
+import pytest
+import respx
+
+from oilpriceapi import AsyncOilPriceAPI, OilPriceAPI
+from oilpriceapi.exceptions import (
+    AuthenticationError,
+    OilPriceAPIError,
+)
+
+BASE = "https://api.oilpriceapi.com"
+WEBHOOK = "/v1/webhooks/fixture-id"
+
+
+def _sync():
+    return OilPriceAPI(api_key="k", base_url=BASE, max_retries=1)
+
+
+def _async():
+    return AsyncOilPriceAPI(api_key="k", base_url=BASE, max_retries=1)
+
+
+# --- 204 with no body is success -------------------------------------------
+
+@respx.mock
+def test_sync_delete_accepts_204_no_content():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(return_value=httpx.Response(204))
+    assert _sync().webhooks.delete("fixture-id") is None
+
+
+@pytest.mark.asyncio
+@respx.mock
+async def test_async_delete_accepts_204_no_content():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(return_value=httpx.Response(204))
+    assert await _async().webhooks.delete("fixture-id") is None
+
+
+@respx.mock
+def test_sync_request_returns_empty_dict_for_204():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(return_value=httpx.Response(204))
+    assert _sync().request("DELETE", WEBHOOK) == {}
+
+
+@pytest.mark.asyncio
+@respx.mock
+async def test_async_request_returns_empty_dict_for_204():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(return_value=httpx.Response(204))
+    assert await _async().request("DELETE", WEBHOOK) == {}
+
+
+@respx.mock
+def test_request_with_headers_accepts_204():
+    """The third decode site, which #103 names alongside the other two."""
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(204, headers={"X-Request-Id": "abc"})
+    )
+    body, headers = _sync().request_with_headers("DELETE", WEBHOOK)
+    assert body == {}
+    assert headers["X-Request-Id"] == "abc"
+
+
+@pytest.mark.parametrize("status", [200, 202, 204])
+@respx.mock
+def test_any_2xx_with_an_empty_body_is_success(status):
+    """A 200 or 202 with a genuinely empty body is the same situation."""
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(status, content=b"")
+    )
+    assert _sync().request("DELETE", WEBHOOK) == {}
+
+
+@respx.mock
+def test_whitespace_only_body_is_treated_as_empty():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(200, content=b"\n  \n")
+    )
+    assert _sync().request("DELETE", WEBHOOK) == {}
+
+
+# --- what must STILL fail ---------------------------------------------------
+
+@respx.mock
+def test_malformed_nonempty_200_is_still_an_error():
+    """A real parse failure must not be laundered into an empty success."""
+    respx.get(f"{BASE}/v1/prices/latest").mock(
+        return_value=httpx.Response(200, content=b"{not json")
+    )
+    with pytest.raises(Exception) as exc:
+        _sync().request("GET", "/v1/prices/latest")
+    assert not isinstance(exc.value, type(None))
+
+
+@pytest.mark.asyncio
+@respx.mock
+async def test_async_malformed_nonempty_200_is_still_an_error():
+    respx.get(f"{BASE}/v1/prices/latest").mock(
+        return_value=httpx.Response(200, content=b"{not json")
+    )
+    with pytest.raises(Exception):
+        await _async().request("GET", "/v1/prices/latest")
+
+
+@respx.mock
+def test_204_is_not_retried():
+    """A success must not burn the retry budget."""
+    route = respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(204)
+    )
+    _sync().request("DELETE", WEBHOOK)
+    assert route.call_count == 1
+
+
+@respx.mock
+def test_401_still_raises_authentication_error():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(401, json={"error": "bad key"})
+    )
+    with pytest.raises(AuthenticationError):
+        _sync().request("DELETE", WEBHOOK)
+
+
+@respx.mock
+def test_403_still_raises_a_typed_error():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(403, json={"error": "forbidden"})
+    )
+    with pytest.raises(OilPriceAPIError):
+        _sync().request("DELETE", WEBHOOK)
+
+
+@pytest.mark.asyncio
+@respx.mock
+async def test_async_401_still_raises_authentication_error():
+    respx.delete(f"{BASE}{WEBHOOK}").mock(
+        return_value=httpx.Response(401, json={"error": "bad key"})
+    )
+    with pytest.raises(AuthenticationError):
+        await _async().request("DELETE", WEBHOOK)
+
+
+# --- sync/async parity ------------------------------------------------------
+
+def test_all_three_decode_sites_share_one_helper():
+    """client.request, client.request_with_headers and async_client.request
+    must decode identically; three copies of the branch would drift."""
+    import inspect
+
+    from oilpriceapi import async_client, client
+
+    sync_src = inspect.getsource(client)
+    async_src = inspect.getsource(async_client)
+    for src, name in ((sync_src, "client"), (async_src, "async_client")):
+        assert "decode_json_body" in src, name
+        # No bare `return response.json()` left behind.
+        assert "return response.json()" not in src, name
+    assert "return response.json(), response.headers" not in sync_src