From 0bc2d30d208a867f8e2075e031534ec7577a23aa Mon Sep 17 00:00:00 2001 From: thodson-usgs Date: Wed, 4 Nov 2026 11:52:33 -0600 Subject: [PATCH] fix(transport): stop retrying certificate failures and name the fix (#461) A server certificate that Python does not trust, usually because a network proxy or firewall re-signs HTTPS with an organization root that certifi does not contain, was classed as transient. The fan-out getters spent their retry budget on it and raised ServiceInterrupted, telling the caller to resume once the service recovered. ssl.SSLCertVerificationError is now a deterministic failure, so it is neither retried nor resumable, and network_error() appends a remedy: verify against the operating system's certificates with truststore, or set SSL_CERT_FILE to a bundle containing the organization's root certificate. No dependency is added. A NetworkError from an unchunked OGC query also names the request URL instead of "unknown service". --- NEWS.md | 2 + dataretrieval/interruptions.py | 21 ++++++- dataretrieval/transport/fanout.py | 17 +++++- dataretrieval/transport/http.py | 23 +++++++- docs/source/userguide/errors.rst | 27 +++++++++ tests/transport_test.py | 96 +++++++++++++++++++++++++++++++ 6 files changed, 181 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index 11ce324a..959891a8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,5 @@ +**11/04/2026:** **Bug fix:** a server certificate that Python does not trust, usually because a network proxy or firewall re-signs HTTPS traffic, is no longer retried (#461). The Water Data, NGWMN, and NWDC getters spent their retry budget on it and then raised `ServiceInterrupted`, advising the caller to resume once the service recovered. Every getter now raises `NetworkError` on the first attempt, with a message that names the fix: verify against the operating system's certificates with `truststore`, or set `SSL_CERT_FILE` to a bundle that contains your organization's root certificate. See [Handling errors](https://doi-usgs.github.io/dataretrieval-python/userguide/errors.html). A `NetworkError` from a query that was not chunked now names the URL it requested rather than `unknown service`. + **10/29/2026:** `waterdata.get_latest_continuous()` accepts `method_category`, a column USGS added to the `latest-continuous` collection in October 2026; `get_continuous()` has accepted it since 09/22/2026. It is the RLMS method category code (`STNRD`, `LMTUS`, `EXPER` or `UNKWN`) for the method in effect over an observation's interval, returned on every record and null for time series that have not been categorized. It could already be passed through `**queryables`; it is now a documented, keyword-only parameter. **10/16/2026:** **Bug fix:** the time-zone table behind the `*DateTime` columns that `waterdata.get_samples()` and `wqp.get_results()` append, and behind `utils.format_datetime()`, now follows the code lists those services use: the USGS NWIS time-zone list (Samples, and the USGS records that the Water Quality Portal passes through) and the WQX TimeZone domain (Water Quality Portal). USGS records write American Samoa as `ZP-11`, which was missing, so every timed American Samoa row from `get_samples()`, and every timed USGS American Samoa row from `get_results()`, had a `NaT` datetime; it is now UTC-11. 39 other codes from the two lists that used to become `NaT` now resolve, among them the remaining `ZP*` offsets, `IDLW`, `IDLE`, `NZT`, `SWT`, and the retired WQX codes `AHST` and `YST`. **Behavior change:** codes that the table read with a different meaning from the USGS list now take the USGS meaning: `CAST` (+9:30), `CCT` (+8), `EASST` (+11), `EAST` (+10), `NFT` (-3:30), `NZDT` (+13), `WAST` (+7), and `WAT` (-1). `FJST` is now UTC+13. Where the two lists disagree, WQX wins: `SST` stays American Samoa (UTC-11), the meaning the Water Quality Portal returns, and `BST` is UTC-11. diff --git a/dataretrieval/interruptions.py b/dataretrieval/interruptions.py index a77e5ded..16d2f234 100644 --- a/dataretrieval/interruptions.py +++ b/dataretrieval/interruptions.py @@ -32,6 +32,7 @@ from __future__ import annotations import socket +import ssl from collections.abc import Iterator from typing import TYPE_CHECKING, Any, ClassVar @@ -276,8 +277,9 @@ def _walk_causes( def _deterministic_failure(exc: BaseException) -> bool: """Whether a transport failure would fail identically on every retry. - True for an unsupported scheme, a malformed request, or a hostname the - resolver rejects permanently. A *temporary* resolver failure is not in that + True for an unsupported scheme, a malformed request, a server certificate + the local trust store rejects, or a hostname the resolver rejects + permanently. A *temporary* resolver failure is not in that class and stays retryable (see :data:`_PERMANENT_DNS_ERRORS`). Bounding retry to failures a later attempt might not repeat is ADR 0006. @@ -292,12 +294,27 @@ class and stays retryable (see :data:`_PERMANENT_DNS_ERRORS`). Bounding for current in _walk_causes(exc, follow_context=True): if isinstance(current, (httpx.UnsupportedProtocol, httpx.LocalProtocolError)): return True + if isinstance(current, ssl.SSLCertVerificationError): + return True if isinstance(current, socket.gaierror): # Return, not continue: the first resolver code found decides. return current.errno in _PERMANENT_DNS_ERRORS return False +def _certificate_rejected(exc: BaseException) -> bool: + """Whether a transport failure is a server certificate the trust store rejected. + + One of the :func:`_deterministic_failure` cases, asked separately so the error + message can name its remedy. httpcore chains the ``ssl`` error implicitly, so + ``__context__`` is walked. + """ + return any( + isinstance(current, ssl.SSLCertVerificationError) + for current in _walk_causes(exc, follow_context=True) + ) + + def _classify_transient( exc: BaseException, ) -> tuple[type[FanOutInterrupted], float | None] | None: diff --git a/dataretrieval/transport/fanout.py b/dataretrieval/transport/fanout.py index e135d92e..4152c53c 100644 --- a/dataretrieval/transport/fanout.py +++ b/dataretrieval/transport/fanout.py @@ -142,6 +142,21 @@ def active_client() -> httpx.AsyncClient | None: return _active_client.get() +def _failed_url(canonical_url: str | None, exc: httpx.TransportError) -> str: + """Name the URL a transport failure reached for. + + A query that needed no chunking plan has no ``canonical_url``; the failed + request still carries its own, which tells the caller far more than a + placeholder. + """ + if canonical_url: + return canonical_url + try: + return str(exc.request.url) + except RuntimeError: # httpx raises this when no request is attached + return "unknown service" + + # --------------------------------------------------------------------------- # Type aliases for the FanOut contract # --------------------------------------------------------------------------- @@ -329,7 +344,7 @@ def _normalize_failure(self, exc: BaseException) -> BaseException: for current in _walk_causes(exc): if isinstance(current, httpx.TransportError): wrapped = network_error( - self.canonical_url or "unknown service", current + _failed_url(self.canonical_url, current), current ) wrapped.__cause__ = current return wrapped diff --git a/dataretrieval/transport/http.py b/dataretrieval/transport/http.py index 82029f90..fe520ee2 100644 --- a/dataretrieval/transport/http.py +++ b/dataretrieval/transport/http.py @@ -17,6 +17,7 @@ strip_api_key_from_untrusted_host_async, ) from dataretrieval.exceptions import NetworkError +from dataretrieval.interruptions import _certificate_rejected # Re-exported for the adapters that import credential policy through the # transport module they already import. ``dataretrieval.credentials`` is the @@ -77,10 +78,28 @@ def default_headers(target_url: str | httpx.URL | None = None) -> dict[str, str] } +#: Remedy for a server certificate the local trust store rejects. +_CERTIFICATE_REMEDY = ( + "To recover: the server's certificate is not signed by an authority that " + "Python trusts, often because a network proxy or firewall re-signs HTTPS " + "traffic. Retrying will fail the same way. Either verify against your " + "operating system's certificates by running `pip install truststore` and " + "then calling `import truststore; truststore.inject_into_ssl()` before " + "the first request, or set the SSL_CERT_FILE environment variable to a " + "PEM bundle that contains your organization's root certificate." +) + + def network_error(url: str | httpx.URL, exc: httpx.TransportError) -> NetworkError: - """Build a typed error for a failed round trip with no HTTP response.""" + """Build a typed error for a failed round trip with no HTTP response. + + A certificate verification failure gets the steps that fix it appended. + """ detail = str(exc) or type(exc).__name__ - return NetworkError(f"Could not reach the service at {url}: {detail}") + message = f"Could not reach the service at {url}: {detail}" + if _certificate_rejected(exc): + message = f"{message}. {_CERTIFICATE_REMEDY}" + return NetworkError(message) def get(url: str | httpx.URL, **kwargs: Any) -> httpx.Response: diff --git a/docs/source/userguide/errors.rst b/docs/source/userguide/errors.rst index 73ca99f0..d9bd2302 100644 --- a/docs/source/userguide/errors.rst +++ b/docs/source/userguide/errors.rst @@ -27,6 +27,33 @@ inline retries during fan-out is a resumable ``ServiceInterrupted``. A *no-data* result is **not** an error: the modern getters return an empty ``DataFrame`` when nothing matches, so check ``df.empty`` rather than catching anything. +Certificate verification failures +================================= + +A ``NetworkError`` whose message contains ``CERTIFICATE_VERIFY_FAILED`` means +Python does not trust the certificate the server presented. The usual cause is a +network that intercepts HTTPS -- common on agency and corporate networks -- and +re-signs it with a root certificate that your operating system trusts but +Python's default bundle (``certifi``) does not contain. Retrying fails the same +way, so the error is raised immediately rather than retried. + +Either verify against the operating system's certificates with +`truststore `_ (``pip install truststore``), +called once at the start of your script or notebook: + +.. code-block:: python + + import truststore + + truststore.inject_into_ssl() + +or point ``SSL_CERT_FILE`` at a PEM bundle that contains your organization's +root certificate, before the first request: + +.. code-block:: bash + + export SSL_CERT_FILE=/path/to/org-ca-bundle.pem + Branch without knowing the concrete type ========================================= diff --git a/tests/transport_test.py b/tests/transport_test.py index 72b09014..c9903d7b 100644 --- a/tests/transport_test.py +++ b/tests/transport_test.py @@ -6,8 +6,10 @@ import datetime import itertools import socket +import ssl from unittest import mock +import httpcore import httpx import pandas as pd import pytest @@ -25,6 +27,7 @@ ServiceUnavailable, ) from dataretrieval.transport.fanout import FanOut +from dataretrieval.transport.http import network_error from dataretrieval.transport.pagination import paginate @@ -580,6 +583,99 @@ def test_deterministic_failures_are_not_offered_as_resumable() -> None: assert _classify_chunk_error(temporary) is not None +_FAILED_URL = "https://example.test/collections/daily/items" + + +def _certificate_failure() -> httpx.ConnectError: + """A certificate rejection shaped the way httpx raises one. + + httpx raises its ``ConnectError`` from httpcore's (so both ``__cause__`` and + ``__context__`` point at it), and httpcore raises its own while handling the + ``ssl`` error, so the ``ssl`` error is reachable only through ``__context__``. + """ + rejected = ssl.SSLCertVerificationError( + 1, "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed" + ) + transport_failed = httpcore.ConnectError(rejected) + transport_failed.__context__ = rejected + failure = httpx.ConnectError( + "[SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed", + request=httpx.Request("GET", _FAILED_URL), + ) + failure.__cause__ = failure.__context__ = transport_failed + return failure + + +def test_certificate_failure_is_neither_retried_nor_resumable() -> None: + """A rejected certificate is rejected again on every attempt. + + Classed as transient, it spent the retry budget and then surfaced as a + ``ServiceInterrupted`` telling the caller to resume once the service + recovers, when the fault is in the local trust store. + """ + from dataretrieval.interruptions import _classify_chunk_error + + assert retry._retryable(_certificate_failure()) == (False, None) + assert _classify_chunk_error(_certificate_failure()) is None + + +def test_certificate_failure_names_the_fix() -> None: + """The fan-out raises a ``NetworkError`` whose message says what to change. + + The query has no ``canonical_url`` (nothing was chunked), so the message + names the failed request's URL rather than a placeholder. + """ + attempts = 0 + + async def operation(_item: str) -> tuple[pd.DataFrame, httpx.Response]: + nonlocal attempts + attempts += 1 + raise _certificate_failure() + + policy = retry.RetryPolicy(max_retries=3, base_backoff=0.0) + with pytest.raises(NetworkError) as excinfo: + FanOut(["only"], operation, policy).resume() + + message = str(excinfo.value) + assert attempts == 1 + assert f"Could not reach the service at {_FAILED_URL}" in message + assert "CERTIFICATE_VERIFY_FAILED" in message + assert "To recover:" in message + assert "pip install truststore" in message + assert "truststore.inject_into_ssl()" in message + assert "SSL_CERT_FILE" in message + assert isinstance(excinfo.value.__cause__, httpx.ConnectError) + + +def test_other_connection_failures_get_no_certificate_advice() -> None: + """Only a certificate rejection is told to change the trust store.""" + message = str(network_error(_FAILED_URL, httpx.ConnectTimeout("timed out"))) + + assert message == f"Could not reach the service at {_FAILED_URL}: timed out" + + +@pytest.mark.parametrize( + ("canonical_url", "request_url", "named"), + [ + # The query's own URL wins over the chunk that happened to fail. + ("https://example.test/query", _FAILED_URL, "https://example.test/query"), + (None, _FAILED_URL, _FAILED_URL), + # An error raised with no request attached has no URL to report. + (None, None, "unknown service"), + ], +) +def test_fan_out_failure_names_the_best_url_available( + canonical_url: str | None, request_url: str | None, named: str +) -> None: + request = httpx.Request("GET", request_url) if request_url else None + + async def operation(_item: str) -> tuple[pd.DataFrame, httpx.Response]: + raise httpx.UnsupportedProtocol("no scheme", request=request) + + with pytest.raises(NetworkError, match=f"service at {named}: no scheme$"): + FanOut(["only"], operation, canonical_url=canonical_url).resume() + + def test_exception_chain_walk_terminates_on_a_self_referencing_chain() -> None: """Every classification of a failure chain shares one guarded traversal.