Raise on any mid-pagination failure instead of silently truncating

`_walk_pages` and `get_stats_data`'s pagination loops have, since
PR DOI-USGS#273, logged failures correctly but preserved a "best effort"
contract of returning whatever pages had been collected when a
follow-up page failed. The waterdata API exposes no resume cursor
once a paginated walk is interrupted, so the partial DataFrame
couldn't reliably be extended — silently returning it handed
callers truncated data they had no way to know was truncated.

Both loops now wrap any mid-pagination exception (429, 5xx,
network error) in a ``RuntimeError`` carrying:

- the number of pages successfully collected,
- the upstream cause (as both the message text and ``__cause__``
  for programmatic inspection),
- a short menu of recovery actions (wait and retry, reduce
  request size, or obtain an API token).

The shared helper ``_paginated_failure_message`` builds the user-
facing string so both loops stay aligned.

Behavior change: callers that previously consumed partial
DataFrames on transient upstream blips will now see an exception
they need to handle (typically: retry, possibly with a smaller
``limit`` or narrower query). Called out in NEWS.

Tests:
- Replaced the prior best-effort-preserves-partial assertions
  with raises-with-cause-chain assertions for all three failure
  modes (connection error, 5xx, 429), in both ``_walk_pages``
  and ``get_stats_data`` variants.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: thodson-usgs

NEWS.md

@@ -1,3 +1,5 @@
+**05/16/2026:** Paginated `waterdata` request loops (`_walk_pages` and `get_stats_data`) now raise on *any* mid-pagination failure (HTTP 429, 5xx, network error, …) instead of logging a warning and returning the rows accumulated up to the failure. The API exposes no resume cursor once a paginated walk is interrupted, so the prior best-effort return-partial behavior silently handed callers truncated DataFrames they couldn't reliably extend — a data-correctness footgun. The wrapper `RuntimeError` carries the upstream cause via `__cause__` plus a short menu of recovery actions (wait and retry, reduce the request, or obtain an API token). **Behavior change**: callers that previously consumed partial DataFrames on transient upstream blips will now see an exception; retry the call (possibly with a smaller `limit` or narrower query).
+
 **05/14/2026:** Fixed two latent bugs in the paginated `waterdata` request loop (`_walk_pages` and `get_stats_data`). Previously, when `requests.Session.request(...)` itself raised mid-pagination (network error, timeout), the except block called `_error_body()` on the *prior page's* response, so the logged "error" described the wrong request and could itself crash on non-JSON bodies. Separately, no status-code check was performed on subsequent paginated responses, so a 5xx body that didn't include `numberReturned` was silently treated as an empty page — pagination quietly stopped and the user got truncated data with no error logged. The loop now status-checks each page like the initial request and reports the actual exception. The "best-effort" behavior (return whatever pages were collected) is preserved.
 
 **05/07/2026:** Bumped the declared minimum Python version from **3.8** to **3.9** (`pyproject.toml`'s `requires-python` and the ruff target). This brings the manifest in line with what was already being tested — CI's matrix has long covered only 3.9, 3.13, and 3.14, the `waterdata` test module already skipped itself on Python < 3.10, and several modules already use 3.9-only stdlib (e.g. `zoneinfo`). Users on 3.8 will no longer be able to install the package; please upgrade.

dataretrieval/waterdata/utils.py

@@ -422,6 +422,24 @@ def _raise_for_non_200(resp: requests.Response) -> None:
         raise RuntimeError(_error_body(resp))
 
 
+def _paginated_failure_message(pages_collected: int, cause: BaseException) -> str:
+    """Build the user-facing message for a mid-pagination failure.
+
+    Wraps the upstream error with the page count we got through and a
+    short menu of recovery actions, since the API offers no resume
+    cursor: a partial result here cannot be reliably extended, so the
+    caller's next step is always to retry the whole call (possibly
+    smaller, possibly later, possibly with an API token).
+    """
+    return (
+        f"Paginated request failed after collecting {pages_collected} "
+        f"page(s): {str(cause).rstrip('.')}. To recover: wait for the "
+        f"rate-limit window to reset and retry, reduce the request size "
+        f"(e.g. fewer locations, a shorter time range, or a smaller "
+        f"``limit``), or obtain an API token."
+    )
+
+
 def _construct_api_requests(
     service: str,
     properties: list[str] | None = None,
@@ -647,8 +665,16 @@ def _walk_pages(
 
     Raises
     ------
-    Exception
-        If a request fails/returns a non-200 status code.
+    RuntimeError
+        On any failure during pagination (non-200 status on any page, or
+        any exception from the underlying ``client.request``). Because
+        the API exposes no resume cursor once a paginated walk is
+        interrupted, returning the partial pages would hand the caller
+        truncated data with no reliable way to fill the gap. The
+        exception message reports how many pages were collected, the
+        upstream cause, and recovery options (wait, reduce the request,
+        or obtain an API token); the original exception is set as
+        ``__cause__``.
     """
     logger.info("Requesting: %s", req.url)
 
@@ -694,7 +720,7 @@ def _walk_pages(
                 logger.warning(
                     "Request failed for URL: %s. Data download interrupted.", curr_url
                 )
-                curr_url = None
+                raise RuntimeError(_paginated_failure_message(len(dfs), e)) from e
 
         # Concatenate all pages at once for efficiency
         return pd.concat(dfs, ignore_index=True), initial_response
@@ -1161,7 +1187,7 @@ def get_stats_data(
                     url,
                     next_token,
                 )
-                next_token = None
+                raise RuntimeError(_paginated_failure_message(len(all_dfs), e)) from e
 
         dfs = pd.concat(all_dfs, ignore_index=True) if len(all_dfs) > 1 else all_dfs[0]
 

tests/waterdata_utils_test.py

@@ -2,6 +2,7 @@
 from unittest import mock
 
 import pandas as pd
+import pytest
 import requests
 
 import dataretrieval.waterdata.utils as _utils_module
@@ -131,21 +132,28 @@ def _walk_pages_with_failure(failure_resp_or_exc):
     return _walk_pages(geopd=False, req=mock_req, client=mock_client)
 
 
-def test_walk_pages_logs_actual_exception_when_request_raises(caplog):
-    """Exception from client.request() must be logged with its actual message."""
+def test_walk_pages_raises_on_connection_error_mid_pagination(caplog):
+    """A connection error mid-pagination must raise with the upstream cause
+    chained, and the wrapper message must include recovery guidance."""
     caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
 
-    df, _ = _walk_pages_with_failure(requests.ConnectionError("boom"))
+    with pytest.raises(RuntimeError, match="Paginated request failed") as excinfo:
+        _walk_pages_with_failure(requests.ConnectionError("boom"))
+
+    # Original exception preserved via chaining for callers who need it.
+    assert isinstance(excinfo.value.__cause__, requests.ConnectionError)
+    assert "boom" in str(excinfo.value)
+    # And the actionable advice is present in the message.
+    assert "API token" in str(excinfo.value)
 
-    # First page's data is preserved (best-effort behavior).
-    assert list(df["val"]) == ["a"]
-    # Logged error mentions the actual ConnectionError, not a stale page body.
     messages = _error_log_messages(caplog)
     assert any("boom" in m for m in messages), messages
 
 
-def test_walk_pages_surfaces_5xx_mid_pagination(caplog):
-    """A non-200 mid-pagination response must be logged, not silently swallowed."""
+def test_walk_pages_raises_on_5xx_mid_pagination(caplog):
+    """A 5xx mid-pagination must raise — partial data is no longer returned
+    because the API has no resume cursor, so silently truncating is the
+    wrong default."""
     caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
 
     page2_503 = mock.MagicMock()
@@ -156,13 +164,30 @@ def test_walk_pages_surfaces_5xx_mid_pagination(caplog):
     }
     page2_503.url = "https://example.com/page2"
 
-    df, _ = _walk_pages_with_failure(page2_503)
+    with pytest.raises(RuntimeError, match="Paginated request failed"):
+        _walk_pages_with_failure(page2_503)
 
-    assert list(df["val"]) == ["a"]
     messages = _error_log_messages(caplog)
     assert any("503" in m or "ServiceUnavailable" in m for m in messages), messages
 
 
+def test_walk_pages_raises_on_mid_pagination_429(caplog):
+    """A 429 mid-pagination must raise. Specific status code is preserved in
+    the chained cause so callers can branch on rate-limit vs other failures."""
+    caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
+
+    page2_429 = mock.MagicMock()
+    page2_429.status_code = 429
+    page2_429.url = "https://example.com/page2"
+
+    with pytest.raises(RuntimeError, match="Paginated request failed") as excinfo:
+        _walk_pages_with_failure(page2_429)
+
+    assert "429" in str(excinfo.value)
+    messages = _error_log_messages(caplog)
+    assert any("429" in m for m in messages), messages
+
+
 def _stats_initial_ok():
     """A 200-OK initial stats response: empty data list, signals one more page."""
     resp = mock.MagicMock()
@@ -205,21 +230,25 @@ def _run_get_stats_data_with_failure(failure_resp_or_exc, monkeypatch):
     )
 
 
-def test_get_stats_data_logs_actual_exception_when_request_raises(caplog, monkeypatch):
-    """get_stats_data variant of the connection-error scenario."""
+def test_get_stats_data_raises_on_connection_error_mid_pagination(caplog, monkeypatch):
+    """get_stats_data variant of the connection-error-raises contract."""
     caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
 
-    _run_get_stats_data_with_failure(
-        requests.ConnectionError("stats-boom"),
-        monkeypatch,
-    )
+    with pytest.raises(RuntimeError, match="Paginated request failed") as excinfo:
+        _run_get_stats_data_with_failure(
+            requests.ConnectionError("stats-boom"),
+            monkeypatch,
+        )
+
+    assert isinstance(excinfo.value.__cause__, requests.ConnectionError)
+    assert "stats-boom" in str(excinfo.value)
 
     messages = _error_log_messages(caplog)
     assert any("stats-boom" in m for m in messages), messages
 
 
-def test_get_stats_data_surfaces_5xx_mid_pagination(caplog, monkeypatch):
-    """get_stats_data variant of the mid-pagination 5xx scenario."""
+def test_get_stats_data_raises_on_5xx_mid_pagination(caplog, monkeypatch):
+    """get_stats_data variant of the 5xx-raises contract."""
     caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
 
     page2_503 = mock.MagicMock()
@@ -230,12 +259,29 @@ def test_get_stats_data_surfaces_5xx_mid_pagination(caplog, monkeypatch):
     }
     page2_503.url = "https://example.com/stats?service=foo&next_token=tok2"
 
-    _run_get_stats_data_with_failure(page2_503, monkeypatch)
+    with pytest.raises(RuntimeError, match="Paginated request failed"):
+        _run_get_stats_data_with_failure(page2_503, monkeypatch)
 
     messages = _error_log_messages(caplog)
     assert any("503" in m or "ServiceUnavailable" in m for m in messages), messages
 
 
+def test_get_stats_data_raises_on_mid_pagination_429(caplog, monkeypatch):
+    """get_stats_data variant of the 429-raises contract."""
+    caplog.set_level(logging.ERROR, logger=_LOGGER_NAME)
+
+    page2_429 = mock.MagicMock()
+    page2_429.status_code = 429
+    page2_429.url = "https://example.com/stats?service=foo&next_token=tok2"
+
+    with pytest.raises(RuntimeError, match="Paginated request failed") as excinfo:
+        _run_get_stats_data_with_failure(page2_429, monkeypatch)
+
+    assert "429" in str(excinfo.value)
+    messages = _error_log_messages(caplog)
+    assert any("429" in m for m in messages), messages
+
+
 def test_get_stats_data_warning_includes_next_token(caplog, monkeypatch):
     """The pagination-failure warning includes the next_token so operators
     can identify which page in the sequence failed. (Addresses Copilot's
@@ -249,7 +295,8 @@ def test_get_stats_data_warning_includes_next_token(caplog, monkeypatch):
         "description": "upstream timeout",
     }
 
-    _run_get_stats_data_with_failure(page2_503, monkeypatch)
+    with pytest.raises(RuntimeError):
+        _run_get_stats_data_with_failure(page2_503, monkeypatch)
 
     warnings_ = [r.getMessage() for r in caplog.records if r.levelno == logging.WARNING]
     # The initial response from _stats_initial_ok carries next=tok2.