fix(config): honour timeout=0 and base_url; stop crashing on max_retries=0 (#120, #121) (#126)

Both halves of the constructor argument handling #115 started and did not
finish, plus the version bump and changelog that change needed.

#120 -- #115 replaced two of the three `or` defaults and left the third, on the
line directly above its own fix, in both clients:

    self.base_url = (base_url or self.DEFAULT_BASE_URL).rstrip("/")   # unchanged
    self.timeout  = timeout or self.DEFAULT_TIMEOUT                   # unchanged
    self.max_retries = (... if max_retries is None else ...)          # fixed
    self.retry_on = (... if retry_on is None else ...)                # fixed

`timeout=0` is a real httpx timeout meaning "fail immediately" and is what
`float(os.getenv("OPA_TIMEOUT", "0"))` produces. It silently became 30, and the
resulting hang is very hard to attribute back to the constructor. A negative
timeout was passed down to httpx unvalidated. `base_url=""` pointed the client
at PRODUCTION, which also pins the #113 origin guard to an origin the caller
did not choose -- the worst available answer for an input nobody meant.

Both are now explicit None checks with validation, matching the two lines
below them. `timeout=None` and `base_url=None` still take the defaults.

#121 -- `max_retries=0` and `max_retries=3.0` started raising
`ConfigurationError` AT CLIENT CONSTRUCTION, having constructed fine in 1.13.0,
with no version bump and no changelog. The validation is right; the delivery
was not. This fails at startup, so it takes a whole process down rather than
degrading one call, and `int(os.getenv("OPA_MAX_RETRIES", "0"))` is the common
way to produce it.

Recommendation taken: accept them again, do not break.

  0    -> 1 attempt, with a DeprecationWarning saying the argument counts
          total ATTEMPTS, not retries after the first
  3.0  -> 3, with a DeprecationWarning
  -1   -> still ConfigurationError
  2.5  -> still ConfigurationError (not a whole number of attempts)
  "3"  -> still ConfigurationError
  True -> still ConfigurationError (a typo hazard; it would mean 1)

The bug #104 fixed does not come back: 0 resolves to ONE attempt, never 3, and
`test_zero_max_retries_does_not_go_back_to_three` counts what reaches the
transport.

Version bumped 1.13.0 -> 1.14.0 in `version.py` and `pyproject.toml`, with a
CHANGELOG entry covering both, including an "Upgrading" note: nothing that
worked in 1.13.0 raises in 1.14.0, but `timeout=0` and `max_retries=0` now mean
what they say instead of 30 and 3.

Two tests added in #115 are updated rather than deleted:
`test_invalid_max_retries_fails_loudly` drops `0` from its parametrize and
keeps the negatives; `test_async_invalid_max_retries_fails_loudly` uses -1. The
new contract for 0 is pinned in
tests/unit/test_constructor_config_validation.py (43 tests, every one run
against BOTH clients).

Proven red against `origin/main` source: 22 failed / 21 passed.

Suite: 112 failed / 673 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).


Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: karlwaldman

CHANGELOG.md

@@ -2,6 +2,51 @@
 
 All notable changes to the OilPriceAPI Python SDK will be documented in this file.
 
+## [1.14.0] - Unreleased
+
+### Fixed
+
+- **`timeout=0` is honoured instead of silently becoming 30.** The constructor
+  used `timeout or self.DEFAULT_TIMEOUT`, so an explicit zero -- a real httpx
+  timeout meaning "fail immediately", and what
+  `float(os.getenv("OPA_TIMEOUT", "0"))` produces -- was discarded. The caller
+  got a 30-second timeout and a hang that is very hard to attribute back to the
+  constructor. Same defect, and the same line, as the `or` defaults fixed in
+  1.13.x for `max_retries` and `retry_on`. Both clients.
+- **`base_url=""` no longer points the client at production.** It now raises
+  `ConfigurationError`. Whatever an empty string meant, silently talking to the
+  live API -- and pinning the request-origin guard to an origin the caller did
+  not choose -- was the worst available answer.
+
+### Changed
+
+- **`max_retries=0` is accepted again, with a `DeprecationWarning`, and means
+  one attempt.** It was briefly rejected with `ConfigurationError` at client
+  construction, which takes a process down at startup for a value that
+  constructed fine in 1.13.0. `max_retries` counts total **attempts**, not
+  retries after the first, so `0` now resolves to `1` -- one attempt, no
+  retries -- and the warning says so. It does **not** go back to silently
+  meaning 3. `0` will be refused in the next major version; pass
+  `max_retries=1` to say "no retries" explicitly.
+- **An integral float `max_retries` (`3.0`) is coerced with a
+  `DeprecationWarning`** instead of raising. That is what a JSON or YAML config
+  round-trip produces for an integer.
+- Still refused, because no coercion is obviously right: a negative
+  `max_retries`, a non-integral float (`2.5`), a string, and `bool` (`True`
+  would silently mean one attempt).
+
+### Added
+
+- `timeout` is validated: a negative or non-numeric value raises
+  `ConfigurationError` instead of being passed down to httpx unvalidated.
+
+### Upgrading
+
+Nothing that worked in 1.13.0 raises in 1.14.0. Two silent behaviours change:
+`timeout=0` now means zero rather than 30, and `max_retries=0` now means one
+attempt rather than three. If you were relying on either of those defaults,
+pass the value you want explicitly.
+
 ## [1.13.0] - 2026-08-23
 
 ### Fixed

oilpriceapi/async_client.py

@@ -44,7 +44,13 @@
 )
 from .models import HistoricalPrice, HistoricalResponse, MarketBrief, Price
 from .resource_validators import format_date
-from .retry import RetryStrategy, mark_ambiguous_write, validated_max_retries
+from .retry import (
+    RetryStrategy,
+    mark_ambiguous_write,
+    validated_base_url,
+    validated_max_retries,
+    validated_timeout,
+)
 
 
 class AsyncOilPriceAPI:
@@ -97,8 +103,13 @@ def __init__(
             )
 
         # Configuration
-        self.base_url = (base_url or self.DEFAULT_BASE_URL).rstrip("/")
-        self.timeout = timeout or self.DEFAULT_TIMEOUT
+        # Explicit None checks, not `or`, on every one of these four lines.
+        # #115 fixed the two below and left these two, so `timeout=0` silently
+        # became 30 and `base_url=""` silently became production (#120).
+        self.base_url = (
+            self.DEFAULT_BASE_URL if base_url is None else validated_base_url(base_url)
+        )
+        self.timeout = self.DEFAULT_TIMEOUT if timeout is None else validated_timeout(timeout)
         # Explicit None checks, not `or` (#104) -- see OilPriceAPI.__init__.
         self.max_retries = (
             self.DEFAULT_MAX_RETRIES if max_retries is None else validated_max_retries(max_retries)

oilpriceapi/client.py

@@ -45,7 +45,13 @@
 from .resources.subscriptions import SubscriptionsResource
 from .resources.webhooks import WebhooksResource
 from .resources.well_production import WellProductionResource
-from .retry import RetryStrategy, mark_ambiguous_write, validated_max_retries
+from .retry import (
+    RetryStrategy,
+    mark_ambiguous_write,
+    validated_base_url,
+    validated_max_retries,
+    validated_timeout,
+)
 
 
 class OilPriceAPI:
@@ -118,11 +124,14 @@ def __init__(
             )
 
         # Configuration
-        self.base_url = (base_url or self.DEFAULT_BASE_URL).rstrip("/")
-        self.timeout = timeout or self.DEFAULT_TIMEOUT
-        # Explicit None checks, not `or`: an explicit max_retries=0 used to
-        # become 3 and an explicit retry_on=[] used to become the default status
-        # list, silently discarding what the caller asked for (#104).
+        # Explicit None checks, not `or`, on every one of these four lines.
+        # #115 fixed the two below and left these two, so `timeout=0` silently
+        # became 30 and `base_url=""` silently became production (#120).
+        self.base_url = (
+            self.DEFAULT_BASE_URL if base_url is None else validated_base_url(base_url)
+        )
+        self.timeout = self.DEFAULT_TIMEOUT if timeout is None else validated_timeout(timeout)
+        # max_retries=0 and retry_on=[] used to be discarded the same way (#104).
         self.max_retries = (
             self.DEFAULT_MAX_RETRIES if max_retries is None else validated_max_retries(max_retries)
         )

oilpriceapi/retry.py

@@ -2,6 +2,7 @@
 
 import logging
 import random
+import warnings
 from typing import List, Mapping, Optional
 
 logger = logging.getLogger(__name__)
@@ -22,15 +23,61 @@ def validated_max_retries(value: object) -> int:
     (``for attempt in range(self.max_retries)``). One attempt is the minimum;
     zero attempts would send nothing. This used to be swallowed by an ``or``
     default, which silently turned an explicit 0 into 3.
+
+    ``0`` and an integral float are accepted with a ``DeprecationWarning``
+    rather than refused (#121). Both constructed fine in 1.13.0 -- ``0`` is the
+    natural spelling of "do not retry" and ``3.0`` is what a JSON or YAML
+    config round-trip produces for an integer -- and this failure happens at
+    CLIENT CONSTRUCTION, so refusing them takes the whole process down at
+    startup rather than degrading one call. The warning says what the argument
+    actually counts, which is the thing the caller needs to learn; the silent
+    ``0 -> 3`` that #104 removed does not come back.
+
+    Still refused, because no coercion is obviously right: negatives,
+    non-numerics, ``bool`` (a typo hazard: ``True`` would mean one attempt),
+    and a non-integral float.
     """
     from .exceptions import ConfigurationError
 
-    if isinstance(value, bool) or not isinstance(value, int):
+    if isinstance(value, bool):
+        raise ConfigurationError(
+            "max_retries must be an int counting total attempts, got bool. "
+            "Pass max_retries=1 for a single attempt with no retries."
+        )
+
+    if isinstance(value, float):
+        if not value.is_integer():
+            raise ConfigurationError(
+                f"max_retries counts total attempts and must be a whole number, "
+                f"got {value}. Pass max_retries=1 for a single attempt with no "
+                f"retries."
+            )
+        warnings.warn(
+            f"max_retries={value!r} is a float; it counts total ATTEMPTS and is "
+            f"being used as {int(value)}. Pass an int.",
+            DeprecationWarning,
+            stacklevel=3,
+        )
+        value = int(value)
+
+    if not isinstance(value, int):
         raise ConfigurationError(
             f"max_retries must be an int counting total attempts, got "
             f"{type(value).__name__}. Pass max_retries=1 for a single attempt "
             f"with no retries."
         )
+
+    if value == 0:
+        warnings.warn(
+            "max_retries counts total ATTEMPTS, not retries after the first, so "
+            "max_retries=0 is being treated as 1 -- one attempt, no retries. "
+            "Pass max_retries=1 to say that explicitly; 0 will be refused in a "
+            "future major version.",
+            DeprecationWarning,
+            stacklevel=3,
+        )
+        return 1
+
     if value < 1:
         raise ConfigurationError(
             f"max_retries counts total attempts and must be at least 1, got {value}. "
@@ -39,6 +86,58 @@ def validated_max_retries(value: object) -> int:
     return value
 
 
+def validated_timeout(value: object) -> float:
+    """Validate an explicit ``timeout`` (seconds).
+
+    ``0`` is a real request timeout to httpx -- fail immediately rather than
+    wait -- and is what ``float(os.getenv("OPA_TIMEOUT", "0"))`` produces. It
+    used to be swallowed by ``timeout or self.DEFAULT_TIMEOUT`` and silently
+    became 30, and the resulting hang is very hard to attribute back to the
+    constructor (#120).
+
+    A negative value used to be passed straight down to httpx unvalidated.
+    """
+    from .exceptions import ConfigurationError
+
+    if isinstance(value, bool) or not isinstance(value, (int, float)):
+        raise ConfigurationError(
+            f"timeout must be a number of seconds, got {type(value).__name__}. "
+            f"Pass timeout=None for the {30}s default."
+        )
+    if value != value:  # NaN
+        raise ConfigurationError("timeout must be a number of seconds, got NaN.")
+    if value < 0:
+        raise ConfigurationError(
+            f"timeout must be zero or positive, got {value}. Use timeout=0 to "
+            f"fail immediately, or timeout=None for the default."
+        )
+    return value
+
+
+def validated_base_url(value: object) -> str:
+    """Validate an explicit ``base_url``.
+
+    ``(base_url or DEFAULT).rstrip("/")`` sent a client constructed with
+    ``base_url=""`` to PRODUCTION. Whatever the caller meant, that is the worst
+    available answer -- and every request-origin guard downstream then pins to
+    an origin they did not choose (#120).
+    """
+    from .exceptions import ConfigurationError
+
+    if not isinstance(value, str):
+        raise ConfigurationError(
+            f"base_url must be a string, got {type(value).__name__}. "
+            f"Pass base_url=None for the default."
+        )
+    trimmed = value.strip().rstrip("/")
+    if not trimmed:
+        raise ConfigurationError(
+            "base_url is empty. Pass base_url=None for the default "
+            "(https://api.oilpriceapi.com) rather than an empty string."
+        )
+    return trimmed
+
+
 def mark_ambiguous_write(error, method: Optional[str]):
     """Tell the caller the write was sent once and its outcome is unknown."""
     error.ambiguous_write = True

oilpriceapi/version.py

@@ -5,6 +5,6 @@
 Used in __init__.py, client.py, and async_client.py.
 """
 
-__version__ = "1.13.0"
+__version__ = "1.14.0"
 SDK_VERSION = __version__
 SDK_NAME = "oilpriceapi-python"

pyproject.toml

@@ -6,7 +6,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "oilpriceapi"
-version = "1.13.0"
+version = "1.14.0"
 description = "Official Python SDK for source-timestamped OilPriceAPI energy data"
 authors = [
     {name = "OilPriceAPI", email = "support@oilpriceapi.com"}