Raise on mid-pagination 429 instead of silently truncating

`_walk_pages` and `get_stats_data`'s pagination loops have, since
PR #273, logged failures correctly but preserved the "best effort"
behavior of returning whatever pages had been collected when a page
mid-walk failed. For 429 specifically that's a data-correctness
footgun: a rate-limit hit means rows are *definitively* missing
(not "the upstream had a transient glitch"), and the caller can
retry once the rate-limit window resets — but only if they know
the result was incomplete. Silently handing back a short DataFrame
hides that.

The loops now re-raise the `RuntimeError` raised by
`_raise_for_non_200` when the failing response carried HTTP 429.
For transient network errors and 5xx the existing best-effort
return-partial behavior is preserved (so an upstream blip on the
last page of a long walk doesn't lose the prior pages). Two new
tests assert the new contract:
- `test_walk_pages_raises_on_mid_pagination_429`
- `test_get_stats_data_raises_on_mid_pagination_429`

Existing 5xx tests still pass — proves the network/5xx path is
unchanged.

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 a mid-pagination HTTP 429 instead of logging a warning and returning the rows accumulated up to that point. Rate-limit hits mean rows are definitively missing and the caller can retry once the rate-limit window resets, so silently returning a short DataFrame was a data-correctness footgun. Best-effort return-partial behavior is preserved for transient network and 5xx failures.
+
 **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

@@ -647,8 +647,12 @@ def _walk_pages(
 
     Raises
     ------
-    Exception
-        If a request fails/returns a non-200 status code.
+    RuntimeError
+        If the initial request or any subsequent page returns a non-200
+        status code. Mid-pagination 429s also raise — best-effort
+        return-partial is preserved only for transient network/5xx
+        failures, since 429s mean rows are definitively missing and the
+        caller can retry once the rate-limit window resets.
     """
     logger.info("Requesting: %s", req.url)
 
@@ -694,6 +698,12 @@ def _walk_pages(
                 logger.warning(
                     "Request failed for URL: %s. Data download interrupted.", curr_url
                 )
+                # Surface mid-pagination 429s instead of returning truncated
+                # rows silently: rate-limit hits are deterministic and
+                # recoverable on retry, unlike transient network/5xx blips
+                # for which the best-effort behavior is preserved.
+                if getattr(resp, "status_code", None) == 429:
+                    raise
                 curr_url = None
 
         # Concatenate all pages at once for efficiency
@@ -1161,6 +1171,8 @@ def get_stats_data(
                     url,
                     next_token,
                 )
+                if getattr(resp, "status_code", None) == 429:
+                    raise
                 next_token = None
 
         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
@@ -163,6 +164,26 @@ def test_walk_pages_surfaces_5xx_mid_pagination(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 instead of returning partial rows.
+
+    Rate-limit hits mean rows are definitively missing; silently
+    returning the pages we already collected hands the caller truncated
+    data with no exception to catch.
+    """
+    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="429"):
+        _walk_pages_with_failure(page2_429)
+
+    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()
@@ -236,6 +257,21 @@ def test_get_stats_data_surfaces_5xx_mid_pagination(caplog, monkeypatch):
     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-instead-of-truncating 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="429"):
+        _run_get_stats_data_with_failure(page2_429, monkeypatch)
+
+    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