Fix silent / misleading failures in paginated waterdata pagination

[thodson-usgs]

Summary

Found while reviewing the repo for high-value bugfixes. Two latent bugs in _walk_pages and the parallel pagination loop in get_stats_data (dataretrieval/waterdata/utils.py) cause silent or misleading failures whenever a paginated waterdata request is interrupted mid-walk.

Bug 1 — resp is stale when client.request() itself raises

while curr_url:
    try:
        resp = client.request(...)              # ← may raise ConnectionError, Timeout, …
        dfs.append(_get_resp_data(resp, geopd))
        curr_url = _next_req_url(resp)
    except Exception:
        error_text = _error_body(resp)          # ← uses *previous* page's resp
        logger.error("Request incomplete. %s", error_text)
        ...

When the network call raises before resp is reassigned, resp still references the previous successful page. The log message thus describes the wrong response, and _error_body() falls into resp.json() on a 200 OK body that wasn't an error envelope — which can itself raise and bury the original failure.

Bug 2 — no status-code check on mid-pagination responses

The initial request gate at line 615 reads if resp.status_code != 200: raise RuntimeError(_error_body(resp)). The pagination loop does not. So a 5xx response with a JSON error body that lacks numberReturned flows into _get_resp_data(), which silently returns an empty DataFrame; _next_req_url() finds no next link; and the loop exits with no error logged anywhere. The caller gets truncated data and never knows.

This affects every paginated waterdata getter: get_daily, get_continuous, get_monitoring_locations, get_time_series_metadata, get_combined_metadata, get_field_measurements, get_field_measurements_metadata, get_peaks, get_channel, get_reference_table, plus the stats getters (get_stats_por, get_stats_date_range).

Fix

Two-line change to each loop:

if resp.status_code != 200:
    raise RuntimeError(_error_body(resp))

…immediately after client.request(...) (mirrors the initial-request gate). And:

except Exception as e:
    logger.error("Request incomplete: %s", e)

…instead of _error_body(resp), so the actual exception text surfaces — works for both the network-error case (uses the exception) and the 5xx case (uses the RuntimeError(_error_body(resp)) we just raised on a fresh resp).

The "best-effort" behavior is preserved: on failure the loop still logs and returns whatever pages were collected. The change is purely to make the failure observable and the log message accurate.

Test plan

• test_walk_pages_logs_actual_exception_when_request_raises — simulates a ConnectionError mid-pagination, asserts the logged error reflects the actual exception ("boom") rather than something derived from a stale prior-page response.
• test_walk_pages_surfaces_5xx_mid_pagination — page 2 returns HTTP 503 with a JSON body, asserts the 503 / ServiceUnavailable shows up in the error log. Previously this was silently swallowed.
• Existing test_walk_pages_multiple_mocked (happy path) still passes.
• Full mocked test suite: 278 passed, ruff + format clean.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes two failure modes in waterdata pagination where mid-walk interruptions could either (a) log misleading/stale error details when requests.Session.request() raises, or (b) silently truncate results when a later page returns a non-200 response.

Changes:

• Added a shared _raise_for_non_200() helper and applied it to both the initial request and subsequent pagination requests in _walk_pages and get_stats_data.
• Updated pagination exception handling to log the actual exception message rather than deriving an error from a potentially stale prior response.
• Added regression tests for mid-pagination request exceptions and mid-pagination non-200 responses; documented the fixes in NEWS.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
dataretrieval/waterdata/utils.py	Ensures every paginated page is status-checked and avoids stale-response error logging during pagination failures.
tests/waterdata_utils_test.py	Adds targeted regression tests covering mid-pagination request exceptions and 5xx responses.
NEWS.md	Documents the pagination bugfixes and their user-visible impact.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

dataretrieval/waterdata/utils.py:1152

• Same as in _walk_pages: the PR description mentions switching to logger.error(...), but this code uses logger.exception(...) and will log a full traceback for mid-pagination failures. Please confirm that this additional verbosity is intended; otherwise align the logging call with the described behavior.

            except Exception as e:  # noqa: BLE001
                logger.exception("Request incomplete: %s", e)
                logger.warning(
                    "Request failed for URL: %s (next_token=%s). "

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.