Extract _raise_for_non_200 helper and trim PR #273 comments

The status-code guard appeared four times in waterdata/utils.py
(initial-request and pagination paths in both _walk_pages and
get_stats_data) with bit-identical bodies. Extract into a single
named helper; the helper's docstring carries the "silent-empty-frame"
WHY that the inline comment was apologizing for.

Also:
- trim the second multi-line comment in _walk_pages to a single line
  noting the actual constraint (`resp` may be stale)
- hoist `import logging` to module level in waterdata_utils_test.py
- condense the two new test docstrings to one-line contract statements;
  the bug history lives in the commit message

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

Author: thodson-usgs

dataretrieval/waterdata/utils.py

@@ -368,6 +368,17 @@ def _error_body(resp: requests.Response):
     )
 
 
+def _raise_for_non_200(resp: requests.Response) -> None:
+    """Raise ``RuntimeError(_error_body(resp))`` if ``resp`` is not 200.
+
+    Used by both the initial-request gate and the pagination loops; without
+    the loop-side check, a 5xx body lacking ``numberReturned`` would be
+    silently treated as an empty page and pagination would stop quietly.
+    """
+    if resp.status_code != 200:
+        raise RuntimeError(_error_body(resp))
+
+
 def _construct_api_requests(
     service: str,
     properties: list[str] | None = None,
@@ -612,8 +623,7 @@ def _walk_pages(
     client = client or requests.Session()
     try:
         resp = client.send(req)
-        if resp.status_code != 200:
-            raise RuntimeError(_error_body(resp))
+        _raise_for_non_200(resp)
 
         # Store the initial response for metadata
         initial_response = resp
@@ -635,17 +645,11 @@ def _walk_pages(
                     headers=headers,
                     data=content if method == "POST" else None,
                 )
-                # Mirror the initial-request check; otherwise a 5xx body
-                # without "numberReturned" silently yields an empty frame
-                # and the loop quietly stops.
-                if resp.status_code != 200:
-                    raise RuntimeError(_error_body(resp))
+                _raise_for_non_200(resp)
                 dfs.append(_get_resp_data(resp, geopd=geopd))
                 curr_url = _next_req_url(resp)
             except Exception as e:  # noqa: BLE001
-                # Report the *actual* failure — not _error_body(resp) on a
-                # stale prior-page response (which would describe the wrong
-                # request and can itself raise on non-JSON bodies).
+                # `resp` may be stale (prior page) if client.request() raised.
                 logger.error("Request incomplete: %s", e)
                 logger.warning(
                     "Request failed for URL: %s. Data download interrupted.", curr_url
@@ -1079,8 +1083,7 @@ def get_stats_data(
 
     try:
         resp = client.send(req)
-        if resp.status_code != 200:
-            raise RuntimeError(_error_body(resp))
+        _raise_for_non_200(resp)
 
         # Store the initial response for metadata
         initial_response = resp
@@ -1106,8 +1109,7 @@ def get_stats_data(
                     params=args,
                     headers=headers,
                 )
-                if resp.status_code != 200:
-                    raise RuntimeError(_error_body(resp))
+                _raise_for_non_200(resp)
                 body = resp.json()
                 all_dfs.append(_handle_stats_nesting(body, geopd=GEOPANDAS))
                 next_token = body["next"]

tests/waterdata_utils_test.py

@@ -1,3 +1,4 @@
+import logging
 from unittest import mock
 
 import pandas as pd
@@ -123,12 +124,7 @@ def _walk_pages_with_failure(failure_resp_or_exc):
 
 
 def test_walk_pages_logs_actual_exception_when_request_raises(caplog):
-    """When client.request() itself raises (e.g. a connection error), the
-    logged failure must reflect that exception — not _error_body() on the
-    stale prior-page response, which described the wrong request and could
-    itself crash on non-JSON bodies."""
-    import logging
-
+    """Exception from client.request() must be logged with its actual message."""
     caplog.set_level(logging.ERROR, logger="dataretrieval.waterdata.utils")
 
     df, _ = _walk_pages_with_failure(requests.ConnectionError("boom"))
@@ -143,12 +139,7 @@ def test_walk_pages_logs_actual_exception_when_request_raises(caplog):
 
 
 def test_walk_pages_surfaces_5xx_mid_pagination(caplog):
-    """A non-200 response on a paginated page must be detected; previously
-    the loop happily called _get_resp_data() on the 5xx body, which —
-    lacking 'numberReturned' — silently produced an empty DataFrame and
-    the loop quietly stopped with no error logged."""
-    import logging
-
+    """A non-200 mid-pagination response must be logged, not silently swallowed."""
     caplog.set_level(logging.ERROR, logger="dataretrieval.waterdata.utils")
 
     page2_500 = mock.MagicMock()