fix(url,retry,futures): un-break today's three regressions (#119, #118, #122)

All three shipped today and all three are reachable from the SDK's own code.

#119 -- `_url.py` rejected U+0020 SPACE. `range(0x21)` is 0x00-0x20 INCLUSIVE;
the comment above it names CR/LF request splitting and NUL, all below 0x20, so
the range over-reached by exactly one character. `/v1/prices?name=Brent Crude`
raised where it previously worked, and `resources/demo.py` builds exactly that
shape (`path = f"{path}?codes={','.join(codes)}"`). A space cannot introduce an
authority or split a request line -- httpx percent-encodes it -- so nothing the
guard exists to stop is enabled by allowing it. Now `range(0x20)`; 0x00-0x1F,
0x7F and backslash stay refused, pinned by tests.

#118 -- #115 keyed replay safety on the HTTP method, which is right, but no
audit of this SDK's own call sites was done, so its POST-shaped READS lost all
retries. A station lookup is idempotent by construction: same lat/lng/radius,
same answer, nothing created. Removing its retries was a pure availability
regression -- a call that used to ride out one bad gateway response now fails
on the first blip.

The rule used to classify them: a POST is read-shaped when the endpoint creates
or mutates no server-side resource AND the same request sent twice returns the
same answer. Six call sites qualify and now pass `idempotent=True` -- diesel
`get_stations`, `webhooks.test`, `alerts.test`, each in both clients.
`data_sources.test` deliberately does NOT: it triggers a fetch against the
customer's configured source and can write to its ingest log. `create()` and
`rotate_credentials()` are unchanged and still sent exactly once.

`test_every_post_call_site_declares_intent` walks the package with `ast` and
fails until every `method="POST"` call either passes `idempotent=` or is listed
as a known write with a reason, so the next POST-shaped read cannot quietly
lose its retries the same way.

#122 -- `normalize_futures_slug` raised a builtin `ValueError`, outside the SDK
hierarchy. That predates #111, but #111 turned 18 live catalog codes from
"resolved to a DIFFERENT instrument" into "raises" -- the right call -- so user
code written as `except OilPriceAPIError: fall_back()` went from a silently
wrong answer to an uncaught crash. #111's own comment argues a refusal is
recoverable; it is only recoverable if it is catchable.

New `FuturesContractError(ValidationError, ValueError)`: catchable through the
documented base class, and still a `ValueError` so existing callers are
unaffected. It renders its own message, which lists every valid slug and
contract code, so the guidance survives regardless of #117's merge order.

Tests: tests/unit/test_url_allows_space.py (10),
tests/unit/test_read_shaped_post_retries.py (12),
tests/unit/test_futures_contract_error.py (26). Proven red against
`origin/main` source: 28 failed / 23 passed.

Suite: 112 failed / 681 passed / 63 skipped, against a measured baseline of
112 failed / 630 passed / 63 skipped on clean main (the 112 are pre-existing,
all missing pytest-asyncio and respx in the environment).

Note for reviewers: `resources/alerts.py` and `resources/diesel.py` are CRLF
files in this repo. Their line endings are preserved -- the diff is 5 and 6
lines, not the whole file.

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

Author: karlwaldman

oilpriceapi/__init__.py

@@ -25,6 +25,7 @@
     RateLimitError,
     ServerError,
     TimeoutError,
+    FuturesContractError,
     ValidationError,
 )
 from oilpriceapi.models import (
@@ -60,6 +61,7 @@
     "RateLimitError",
     "DataNotFoundError",
     "ServerError",
+    "FuturesContractError",
     "ValidationError",
     "NetworkError",
     "TimeoutError",

oilpriceapi/_url.py

@@ -33,7 +33,12 @@
 # reference after this SDK has already decided it was a plain path.
 # Control characters (including CR/LF and NUL) can split a request line or
 # smuggle a header. Neither can appear in a legitimate API path.
-_FORBIDDEN_CHARS = frozenset("\\") | frozenset(chr(c) for c in range(0x21)) | {chr(0x7F)}
+#
+# The range stops BELOW 0x20: U+0020 SPACE is legitimate in a path or an inline
+# query string, httpx percent-encodes it, and it cannot introduce an authority
+# or split a request line. `range(0x21)` forbade it and broke every caller who
+# built a query inline -- including this SDK's own demo resource (#119).
+_FORBIDDEN_CHARS = frozenset("\\") | frozenset(chr(c) for c in range(0x20)) | {chr(0x7F)}
 
 _DEFAULT_PORTS = {"http": 80, "https": 443}
 

oilpriceapi/async_resources.py

@@ -61,9 +61,13 @@ async def get_stations(self, lat: float, lng: float, radius: Optional[float] = 8
                 raise ValidationError(
                     message="Radius must be between 0 and 50000 meters", field="radius", value=radius
                 )
+        # Read-shaped POST: a lat/lng/radius query that creates nothing, so
+        # repeating it has the same effect as doing it once and it keeps the
+        # retries #115 removed from genuine writes (#118).
         response = await self.client.request(
             method="POST", path="/v1/diesel-prices/stations",
-            json_data={"lat": lat, "lng": lng, "radius": radius}
+            json_data={"lat": lat, "lng": lng, "radius": radius},
+            idempotent=True
         )
         return DieselStationsResponse(**response)
 
@@ -263,7 +267,11 @@ async def test(self, alert_id: str) -> Dict[str, Any]:
             raise ValidationError(
                 message="Alert ID must be a non-empty string", field="alert_id", value=alert_id
             )
-        response = await self.client.request(method="POST", path=f"/v1/alerts/{alert_id}/test")
+        # Read-shaped POST: a simulated trigger that does not count against
+        # trigger limits and creates nothing, so it keeps its retries (#118).
+        response = await self.client.request(
+            method="POST", path=f"/v1/alerts/{alert_id}/test", idempotent=True
+        )
         if "data" in response:
             return response["data"]
         return response
@@ -1440,7 +1448,11 @@ async def delete(self, webhook_id: str) -> None:
         await self.client.request(method="DELETE", path=f"/v1/webhooks/{webhook_id}")
 
     async def test(self, webhook_id: str) -> Dict[str, Any]:
-        response = await self.client.request(method="POST", path=f"/v1/webhooks/{webhook_id}/test")
+        # Read-shaped POST: a diagnostic against an already-configured
+        # webhook. It creates nothing, so it keeps its retries (#118).
+        response = await self.client.request(
+            method="POST", path=f"/v1/webhooks/{webhook_id}/test", idempotent=True
+        )
         if "data" in response:
             return response["data"]
         return response

oilpriceapi/exceptions.py

@@ -342,6 +342,27 @@ def __str__(self) -> str:
         return message
 
 
+class FuturesContractError(ValidationError, ValueError):
+    """Raised when a futures contract or slug cannot be resolved (#122).
+
+    Two base classes, deliberately:
+
+    * ``ValidationError`` -- so ``except OilPriceAPIError`` catches it. Every
+      documented recovery path in this SDK is written against that base class,
+      and #111 turned 18 live catalog codes from "resolved to a DIFFERENT
+      instrument" into "raises". A refusal is only recoverable if it is
+      catchable, and a bare builtin ``ValueError`` was not.
+    * ``ValueError`` -- so code written against the pre-#111 ``raise
+      ValueError`` keeps working. This is not a breaking change.
+    """
+
+    def __str__(self) -> str:
+        # The message lists every valid slug and contract code: it IS the
+        # remediation, so render it whole rather than letting a field/value
+        # summary stand in for it.
+        return self.message
+
+
 class ServerError(OilPriceAPIError):
     """Raised when the server returns HTTP 5xx."""
 

oilpriceapi/resources/_futures_slug.py

@@ -24,6 +24,8 @@
 
 from typing import Dict, Set
 
+from ..exceptions import FuturesContractError
+
 # Canonical slugs emitted by the SDK for latest-curve routes.
 VALID_SLUGS: Set[str] = {
     "brent",
@@ -145,7 +147,14 @@ def normalize_futures_slug(contract: str) -> str:
 
     valid = ", ".join(sorted(VALID_SLUGS))
     codes = ", ".join(sorted(CONTRACT_CODE_TO_SLUG))
-    raise ValueError(
+    # FuturesContractError subclasses both ValidationError -- so the SDK's
+    # documented `except OilPriceAPIError` catches it -- and ValueError, so
+    # callers written against the old bare `raise ValueError` are unaffected.
+    # #111 made this path far more common: it turned 18 live catalog codes from
+    # a wrong answer into a refusal, and a refusal must be catchable (#122).
+    raise FuturesContractError(
         f"Unknown futures contract/slug {contract!r}. "
-        f"Pass a slug ({valid}) or a contract code ({codes})."
+        f"Pass a slug ({valid}) or a contract code ({codes}).",
+        field="contract",
+        value=contract,
     )

oilpriceapi/resources/alerts.py

@@ -535,9 +535,12 @@ def test(self, alert_id: str) -> Dict[str, Any]:
                 value=alert_id
             )
 
+        # Read-shaped POST: a simulated trigger that does not count against
+        # trigger limits and creates nothing, so it keeps its retries (#118).
         response = self.client.request(
             method="POST",
-            path=f"/v1/alerts/{alert_id}/test"
+            path=f"/v1/alerts/{alert_id}/test",
+            idempotent=True
         )
 
         # Parse response

oilpriceapi/resources/diesel.py

@@ -216,14 +216,18 @@ def get_stations(
                 )
 
         # Make API request (POST method for stations endpoint)
+        # Read-shaped POST: a lat/lng/radius query that creates nothing, so
+        # repeating it has the same effect as doing it once and it keeps the
+        # retries #115 removed from genuine writes (#118).
         response = self.client.request(
             method="POST",
             path="/v1/diesel-prices/stations",
             json_data={
                 "lat": lat,
                 "lng": lng,
                 "radius": radius
-            }
+            },
+            idempotent=True
         )
 
         # Same envelope as get_price: the stations block lives under `data`.

oilpriceapi/resources/webhooks.py

@@ -214,9 +214,12 @@ def test(self, webhook_id: str) -> Dict[str, Any]:
             >>> print(f"Test status: {result['status']}")
             >>> print(f"Response: {result['response']}")
         """
+        # Read-shaped POST: a diagnostic against an already-configured
+        # webhook. It creates nothing, so it keeps its retries (#118).
         response = self.client.request(
             method="POST",
-            path=f"/v1/webhooks/{webhook_id}/test"
+            path=f"/v1/webhooks/{webhook_id}/test",
+            idempotent=True
         )
 
         # Parse response

tests/unit/test_futures_contract_error.py

@@ -0,0 +1,85 @@
+"""An unresolvable futures contract must raise inside the SDK's hierarchy (#122).
+
+`normalize_futures_slug` raised a builtin `ValueError`, which is not an
+`OilPriceAPIError`. That predates #111, but #111 turned 18 live catalog codes
+from "resolved to a DIFFERENT instrument" into "raises" -- the right call -- so
+user code shaped like
+
+    try:
+        curve = client.futures.curve(code)
+    except OilPriceAPIError:
+        fall_back()
+
+went from a silently wrong answer to an uncaught crash. A refusal is only
+recoverable if it is catchable through the documented base class.
+"""
+
+import pytest
+
+from oilpriceapi import exceptions
+from oilpriceapi.exceptions import OilPriceAPIError, ValidationError
+from oilpriceapi.resources._futures_slug import normalize_futures_slug
+from oilpriceapi.resources.futures import FuturesResource
+
+# The catalog codes #111 moved from "wrong instrument" to "raises".
+REWRITTEN_BY_111 = [
+    "LNG_NW_EUROPE_EUR",
+    "WTI_MIDLAND_USD",
+    "BRENT_CRUDE_USD",
+    "GASOIL_USD",
+    "JKM_LNG_USD",
+]
+
+
+class _FakeClient:
+    def request(self, **kwargs):  # pragma: no cover - never reached
+        raise AssertionError("a refused contract must not reach the transport")
+
+
+@pytest.mark.parametrize("code", REWRITTEN_BY_111)
+def test_normalize_raises_inside_the_sdk_hierarchy(code):
+    with pytest.raises(OilPriceAPIError):
+        normalize_futures_slug(code)
+
+
+@pytest.mark.parametrize("code", REWRITTEN_BY_111)
+def test_it_is_a_validation_error_carrying_the_contract(code):
+    with pytest.raises(exceptions.FuturesContractError) as excinfo:
+        normalize_futures_slug(code)
+
+    error = excinfo.value
+    assert isinstance(error, ValidationError)
+    assert error.field == "contract"
+    assert error.value == code
+
+
+@pytest.mark.parametrize("code", REWRITTEN_BY_111)
+def test_value_error_callers_still_catch_it(code):
+    """Backwards compatible: code written against the old `raise ValueError`."""
+    with pytest.raises(ValueError):
+        normalize_futures_slug(code)
+
+
+def test_the_helpful_message_survives_str():
+    """The message IS the remediation: it lists every valid slug and code."""
+    with pytest.raises(exceptions.FuturesContractError) as excinfo:
+        normalize_futures_slug("WTI_MIDLAND_USD")
+
+    rendered = str(excinfo.value)
+    assert "Unknown futures contract/slug 'WTI_MIDLAND_USD'" in rendered
+    assert "Pass a slug (" in rendered
+    assert "brent" in rendered
+    assert "CL" in rendered
+
+
+@pytest.mark.parametrize("method", ["latest", "curve", "intraday", "historical", "ohlc"])
+def test_public_futures_methods_do_not_escape_the_base_class(method):
+    resource = FuturesResource(_FakeClient())
+
+    with pytest.raises(OilPriceAPIError):
+        getattr(resource, method)("WTI_MIDLAND_USD")
+
+
+@pytest.mark.parametrize("contract", ["brent", "wti", "CL", "BZ", "continuous/brent"])
+def test_valid_contracts_are_untouched(contract):
+    assert normalize_futures_slug(contract)