Implement Copilot improvements: preserve tracebacks, robust HTML detection, and test refinement

thodson-usgs · thodson-usgs · commit cbf7075495f0 · 2026-04-03T14:36:14.000-05:00
diff --git a/dataretrieval/nwis.py b/dataretrieval/nwis.py
@@ -484,12 +484,16 @@ def get_dv(
     try:
         df = _read_json(response.json())
     except Exception as e:
-        if "<html>" in response.text.lower():
+        if (
+            "<html>" in response.text.lower()
+            or "<!doctype" in response.text.lower()
+            or "text/html" in response.headers.get("Content-Type", "").lower()
+        ):
             raise ValueError(
                 "Received HTML response instead of JSON. This often indicates "
                 "that the service is currently unavailable."
             ) from e
-        raise e
+        raise
 
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
@@ -678,12 +682,16 @@ def get_iv(
     try:
         df = _read_json(response.json())
     except Exception as e:
-        if "<html>" in response.text.lower():
+        if (
+            "<html>" in response.text.lower()
+            or "<!doctype" in response.text.lower()
+            or "text/html" in response.headers.get("Content-Type", "").lower()
+        ):
             raise ValueError(
                 "Received HTML response instead of JSON. This often indicates "
                 "that the service is currently unavailable."
             ) from e
-        raise e
+        raise
     return format_response(df, **kwargs), NWIS_Metadata(response, **kwargs)
 
 
@@ -860,7 +868,7 @@ def get_record(
         - 'peaks': discharge peaks
         - 'gwlevels': (defunct) use `waterdata.get_field_measurements`
         - 'pmcodes': (defunct) use `get_reference_table`
-        - 'water_use': (defunct) defunct
+        - 'water_use': (defunct) no replacement available
         - 'ratings': get rating table
         - 'stat': get statistics
     ssl_check: bool, optional
@@ -886,8 +894,6 @@ def get_record(
         >>> # Get site description for site 01585200
         >>> df = dataretrieval.nwis.get_record(sites="01585200", service="site")
 
-        >>> # Get site description for site 01585200
-        >>> df = dataretrieval.nwis.get_record(sites="01585200", service="site")
 
         >>> # Get discharge peaks for site 01585200
         >>> df = dataretrieval.nwis.get_record(sites="01585200", service="peaks")
@@ -1232,7 +1238,11 @@ def site_info(self) -> tuple[pd.DataFrame, BaseMetadata] | None:
             return None  # don't set metadata site_info attribute
 
     @property
-    def variable_info(self) -> tuple[pd.DataFrame, BaseMetadata] | None:
+    def variable_info(self) -> None:
+        """
+        Deprecated. Accessing variable_info via NWIS_Metadata is deprecated
+        as it relied on the defunct `get_pmcodes` function. Returns None.
+        """
         # define variable_info metadata based on parameterCd if available
         if "parameterCd" in self._parameters:
             warnings.warn(
diff --git a/tests/nwis_test.py b/tests/nwis_test.py
@@ -61,23 +61,35 @@ def test_iv_service_answer(requests_mock):
         DATETIME_COL,
     ], f"iv service returned incorrect index: {df.index.names}"
 
-
-def test_nwis_service_live():
-    """Live sanity check of NWIS service, tolerant of 502/503."""
+    """Live sanity check of NWIS service, tolerant of transient NWIS outages."""
     site = "01491000"
     try:
         # Minimal query: just most recent record
         get_iv(sites=site)
     except ValueError as e:
-        # Catch our custom 5xx error from utils.py
-        if any(err in str(e) for err in ["502", "503", "Service Unavailable"]):
-            pytest.skip(f"Service is currently unavailable (transient 502/503): {e}")
-        raise e
+        # Catch known transient service failures surfaced as ValueError
+        error_text = str(e)
+        if any(
+            err in error_text
+            for err in [
+                "500",
+                "502",
+                "503",
+                "Service Unavailable",
+                "Received HTML response instead of JSON",
+            ]
+        ):
+            pytest.skip(
+                f"Service is currently unavailable (transient NWIS outage): {e}"
+            )
+        raise
     except Exception as e:
         # Fallback for other potential transient network issues
         if "Expecting value" in str(e) or "JSON" in str(e):
-            pytest.skip(f"Service returned invalid response (likely 502/503): {e}")
-        raise e
+            pytest.skip(
+                f"Service returned invalid response (likely transient outage): {e}"
+            )
+        raise
 
 
 def test_preformat_peaks_response():
@@ -93,10 +105,6 @@ def test_preformat_peaks_response():
     assert df["datetime"].isna().sum() == 0
 
 
-if __name__ == "__main__":
-    test_iv_service_answer()
-
-
 # tests using real queries to USGS webservices
 # these specific queries represent some edge-cases and the tests to address
 # incomplete date-time information
