More information in get_endpoint exceptions and small docstring fixes

CHANGES.rst

@@ -6,6 +6,9 @@ Enhancements and Fixes
 
 - Make deletion of TAP jobs optional via a new ``delete`` kwarg. [#640]
 
+- Provide more informative exception message when requests to endpoints fail. [#641]
+
+
 Deprecations and Removals
 -------------------------
 

pyvo/dal/query.py

@@ -20,7 +20,8 @@
 standard data model.  Usually the field names are used to uniquely
 identify table columns.
 """
-__all__ = ["DALService", "DALQuery", "DALResults", "Record"]
+__all__ = ["DALService", "DALServiceError", "DALQuery", "DALQueryError",
+           "DALResults", "Record"]
 
 import os
 import shutil
@@ -64,7 +65,7 @@ def __init__(self, baseurl, *, session=None, capability_description=None,):
            the base URL that should be used for forming queries to the service.
         session : object
            optional session to use for network requests
-        description : str, optional
+        capability_description : str, optional
            the description of the service.
         """
         self._baseurl = baseurl

pyvo/dal/tap.py

@@ -118,14 +118,18 @@ def __init__(self, baseurl, *, capability_description=None, session=None):
         session : object
            optional session to use for network requests
         """
-        super().__init__(baseurl, session=session, capability_description=capability_description)
-
-        # Check if the session has an update_from_capabilities attribute.
-        # This means that the session is aware of IVOA capabilities,
-        # and can use this information in processing network requests.
-        # One such use case for this is auth.
-        if hasattr(self._session, 'update_from_capabilities'):
-            self._session.update_from_capabilities(self.capabilities)
+        try:
+            super().__init__(baseurl, session=session, capability_description=capability_description)
+
+            # Check if the session has an update_from_capabilities attribute.
+            # This means that the session is aware of IVOA capabilities,
+            # and can use this information in processing network requests.
+            # One such use case for this is auth.
+            if hasattr(self._session, 'update_from_capabilities'):
+                self._session.update_from_capabilities(self.capabilities)
+        except DALServiceError as e:
+            raise DALServiceError(f"Cannot find TAP service at '"
+                                  f"{baseurl}'.\n\n{str(e)}") from None
 
     def get_tap_capability(self):
         """
@@ -373,8 +377,6 @@ def create_query(
 
         Parameters
         ----------
-        baseurl : str
-            the base URL for the TAP service
         query : str
             the query string / parameters
         mode : str
@@ -940,6 +942,8 @@ def wait(self, *, phases=None, timeout=600.):
         ----------
         phases : list
             phases to wait for
+        timeout : float
+            maximum time to wait in seconds
 
         Raises
         ------

pyvo/dal/tests/test_tap.py

@@ -11,6 +11,7 @@
 import tempfile
 
 import pytest
+import requests
 import requests_mock
 
 from pyvo.dal.tap import escape, search, AsyncTAPJob, TAPService
@@ -792,16 +793,216 @@ def test_get_udf(self, tapservice):
         assert func.form == "ivo_hasword(haystack TEXT, needle TEXT) -> INTEGER"
 
 
-def test_get_endpoint_candidates():
-    # Directly instantiate the TAPService with a known base URL
-    svc = TAPService("http://astroweb.projects.phys.ucl.ac.uk:8000/tap")
+def test_invalid_service_url_format():
+    service = TAPService('http://[invalid-url')
 
-    # Check if the correct endpoint candidates are generated
-    expected_urls = [
-        "http://astroweb.projects.phys.ucl.ac.uk:8000/tap/capabilities",
-        "http://astroweb.projects.phys.ucl.ac.uk:8000/capabilities"
-    ]
-    assert svc._get_endpoint_candidates("capabilities") == expected_urls
+    with pytest.raises(DALServiceError) as excinfo:
+        _ = service.capabilities
+
+    error_message = str(excinfo.value)
+    assert "Invalid service URL" in error_message
+    assert "Please provide a valid URL in the format 'http(s)://domain/path'" in error_message
+
+
+def test_timeout_error():
+    service = TAPService('http://example.com/tap')
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            exc=requests.Timeout("Request timed out")
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            exc=requests.Timeout("Request timed out")
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Request timed out" in error_message
+
+
+def test_generic_request_exception():
+    service = TAPService('http://example.com/tap')
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            exc=requests.RequestException("Some request error")
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            exc=requests.RequestException("Some request error")
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Some request error" in error_message
+
+
+def test_unexpected_exception():
+    service = TAPService('http://example.com/tap')
+
+    class CustomException(Exception):
+        pass
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            exc=CustomException("Unexpected error occurred")
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            exc=CustomException("Unexpected error occurred")
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Unexpected error: Unexpected error occurred" in error_message
+
+
+def test_tap_service_initialization_error():
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            status_code=404
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            status_code=404
+        )
+
+        service = TAPService('http://example.com/tap')
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Unable to access the capabilities endpoint at:" in error_message
+        assert "404" in error_message
+
+
+def test_tap_service_invalid_url_error():
+    service = TAPService('http://[invalid-url')
+    with pytest.raises(DALServiceError) as excinfo:
+        _ = service.capabilities
+
+    error_message = str(excinfo.value)
+    assert "Invalid service URL" in error_message
+    assert "Please provide a valid URL in the format 'http(s)://domain/path'" in error_message
+
+
+def test_endpoint_connection_errors():
+    service = TAPService('http://example.com/tap')
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            status_code=404
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            status_code=404
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Unable to access the capabilities endpoint at:" in error_message
+        assert "404" in error_message
+        assert "The service URL is incorrect" in error_message
+
+
+def test_invalid_tap_url():
+    service = TAPService('not-a-url')
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            requests_mock.ANY,
+            exc=requests.exceptions.ConnectionError(
+                "Failed to establish connection")
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Unable to access the capabilities endpoint at:" in error_message
+        assert "Connection failed" in error_message
+
+
+def test_http_error_responses():
+    error_codes = {
+        403: "Forbidden",
+        500: "Internal Server Error",
+        502: "Bad Gateway",
+        503: "Service Unavailable"
+    }
+
+    for code, reason in error_codes.items():
+        with requests_mock.Mocker() as rm:
+            rm.register_uri(
+                'GET',
+                'http://example.com/tap/capabilities',
+                status_code=code,
+                reason=reason
+            )
+            rm.register_uri(
+                'GET',
+                'http://example.com/capabilities',
+                status_code=code,
+                reason=reason
+            )
+
+            service = TAPService('http://example.com/tap')
+            with pytest.raises(DALServiceError) as excinfo:
+                _ = service.capabilities
+
+            error_message = str(excinfo.value)
+            assert "Unable to access the capabilities endpoint at:" in error_message
+            assert f"{code} {reason}" in error_message
+
+
+def test_network_error():
+    service = TAPService('http://example.com/tap')
+
+    with requests_mock.Mocker() as rm:
+        rm.register_uri(
+            'GET',
+            'http://example.com/tap/capabilities',
+            exc=requests.exceptions.ConnectionError(
+                "Failed to establish connection")
+        )
+        rm.register_uri(
+            'GET',
+            'http://example.com/capabilities',
+            exc=requests.exceptions.ConnectionError(
+                "Failed to establish connection")
+        )
+
+        with pytest.raises(DALServiceError) as excinfo:
+            _ = service.capabilities
+
+        error_message = str(excinfo.value)
+        assert "Unable to access the capabilities endpoint at:" in error_message
+        assert "Connection failed" in error_message
 
 
 @pytest.mark.remote_data

pyvo/dal/vosi.py

@@ -32,17 +32,43 @@ def _get_endpoint_candidates(self, endpoint):
         return [f'{curated_baseurl}/{endpoint}', url_sibling(curated_baseurl, endpoint)]
 
     def _get_endpoint(self, endpoint):
-        for ep_url in self._get_endpoint_candidates(endpoint):
+        """Attempt to connect to service endpoint"""
+        attempted_urls = []
+
+        try:
+            candidates = self._get_endpoint_candidates(endpoint)
+        except Exception as e:
+            raise DALServiceError(
+                f"Invalid service URL '{self.baseurl}'. "
+                "Please provide a valid URL in the format 'http(s)://domain/path'"
+            ) from e
+
+        for ep_url in candidates:
             try:
                 response = self._session.get(ep_url, stream=True)
                 response.raise_for_status()
-                break
-            except requests.RequestException:
-                continue
-        else:
-            raise DALServiceError(f"No working {endpoint} endpoint provided")
-
-        return response.raw
+                return response.raw
+            except requests.HTTPError as e:
+                attempted_urls.append(f"- {ep_url}: {e.response.status_code} {e.response.reason}")
+            except requests.ConnectionError as e:
+                attempted_urls.append(f"- {ep_url}: Connection failed - service may be down")
+            except requests.Timeout as e:
+                attempted_urls.append(f"- {ep_url}: Request timed out")
+            except requests.RequestException as e:
+                attempted_urls.append(f"- {ep_url}: {str(e)}")
+            except Exception as e:
+                attempted_urls.append(f"- {ep_url}: Unexpected error: {str(e)}")
+
+        # If we get here no endpoints worked
+        error_msg = (
+            f"Unable to access the {endpoint} endpoint at:\n"
+            f"{chr(10).join(attempted_urls)}\n\n"
+            f"This could mean:\n"
+            f"1. The service URL is incorrect\n"
+            f"2. The service is temporarily unavailable\n"
+            f"3. The service doesn't support this protocol\n"
+        )
+        raise DALServiceError(error_msg)
 
 
 @deprecated(since="1.5")
@@ -88,8 +114,7 @@ class CapabilityMixin(EndpointMixin):
     @stream_decode_content
     def _capabilities(self):
         """
-        Returns capabilities as a
-        py:class:`~pyvo.io.vosi.availability.Availability` object
+        Retrieve the raw capabilities document from the service.
         """
         return self._get_endpoint('capabilities')