mirror of
https://github.com/semantica-agi/semantica.git
synced 2026-08-29 04:26:20 +00:00
fix(seed): report real cause of API failures in load_from_api (#972)
``requests.exceptions.RequestException`` subclasses ``OSError``, so the ``except (ImportError, OSError)`` handler in ``load_from_api`` swallowed genuine network failures (connection errors, timeouts, HTTP errors) and reported them as "requests library not available", hiding the real cause. Remove the obsolete handler so those failures fall through to the generic handler, which reports "Failed to load from API: ..." and chains the real exception as ``__cause__``. Update the docstring's ``Raises`` section to match the actual behavior. Fixes #949 Co-authored-by: Pravit Ampapathini <pravit.amp@gmail.com>
This commit is contained in:
co-authored by
Pravit Ampapathini
parent
2f63896fb4
commit
4217f23df2
@@ -482,8 +482,8 @@ class SeedDataManager:
|
||||
List of loaded data records as dictionaries
|
||||
|
||||
Raises:
|
||||
ProcessingError: If API request fails, response parsing fails, or
|
||||
requests library is not available
|
||||
ProcessingError: If the API request fails (connection error,
|
||||
timeout, non-2xx status) or the response cannot be parsed
|
||||
|
||||
Example:
|
||||
>>> records = manager.load_from_api(
|
||||
@@ -559,10 +559,6 @@ class SeedDataManager:
|
||||
self.logger.info(f"Loaded {len(records)} records from API: {full_url}")
|
||||
return records
|
||||
|
||||
except (ImportError, OSError):
|
||||
raise ProcessingError(
|
||||
"requests library not available. Install with: pip install requests"
|
||||
)
|
||||
except Exception as e:
|
||||
raise ProcessingError(f"Failed to load from API: {e}") from e
|
||||
|
||||
|
||||
@@ -5,6 +5,9 @@ import json
|
||||
import csv
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import requests
|
||||
|
||||
from semantica.seed.seed_manager import SeedDataManager, SeedDataSource, SeedData
|
||||
from semantica.utils.exceptions import ProcessingError
|
||||
|
||||
@@ -263,6 +266,61 @@ def test_load_from_api_does_not_mutate_empty_headers_dict(mock_guard, seed_manag
|
||||
guard_headers = call_kwargs.get("headers", {})
|
||||
assert guard_headers.get("Authorization") == "Bearer key"
|
||||
|
||||
|
||||
# requests.exceptions.RequestException subclasses OSError, so network failures raised
|
||||
# by request_with_ssrf_guard used to be reported as "requests library not available"
|
||||
# by the obsolete ImportError / OSError handler. They must surface the real cause.
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
requests.exceptions.ConnectionError("connection refused"),
|
||||
requests.exceptions.Timeout("timed out"),
|
||||
requests.exceptions.HTTPError("500 Server Error"),
|
||||
],
|
||||
)
|
||||
@patch("semantica.seed.seed_manager.request_with_ssrf_guard")
|
||||
def test_load_from_api_request_failure_reports_real_cause(mock_guard, error, seed_manager):
|
||||
mock_guard.side_effect = error
|
||||
|
||||
with pytest.raises(ProcessingError) as excinfo:
|
||||
seed_manager.load_from_api(api_url="http://api.example.com", endpoint="users")
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "Failed to load from API" in message
|
||||
assert str(error) in message
|
||||
assert "requests library not available" not in message
|
||||
assert excinfo.value.__cause__ is error
|
||||
|
||||
|
||||
@patch("semantica.seed.seed_manager.request_with_ssrf_guard")
|
||||
def test_load_from_api_http_status_error_reports_real_cause(mock_guard, seed_manager):
|
||||
http_error = requests.exceptions.HTTPError("404 Client Error: Not Found")
|
||||
mock_response = MagicMock()
|
||||
mock_response.raise_for_status.side_effect = http_error
|
||||
mock_guard.return_value = mock_response
|
||||
|
||||
with pytest.raises(ProcessingError) as excinfo:
|
||||
seed_manager.load_from_api(api_url="http://api.example.com", endpoint="users")
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "404 Client Error: Not Found" in message
|
||||
assert "requests library not available" not in message
|
||||
mock_response.json.assert_not_called()
|
||||
|
||||
|
||||
@patch("semantica.seed.seed_manager.request_with_ssrf_guard")
|
||||
def test_load_from_api_invalid_json_reports_real_cause(mock_guard, seed_manager):
|
||||
mock_response = MagicMock()
|
||||
mock_response.json.side_effect = ValueError("Expecting value: line 1 column 1")
|
||||
mock_guard.return_value = mock_response
|
||||
|
||||
with pytest.raises(ProcessingError) as excinfo:
|
||||
seed_manager.load_from_api(api_url="http://api.example.com")
|
||||
|
||||
message = str(excinfo.value)
|
||||
assert "Failed to load from API" in message
|
||||
assert "Expecting value" in message
|
||||
|
||||
def test_load_source(seed_manager, temp_data_dir):
|
||||
json_file = temp_data_dir / "source.json"
|
||||
with open(json_file, "w") as f:
|
||||
|
||||
Reference in New Issue
Block a user