Surface the API's error detail when a dataset import fails

src/dataworkbench/gateway.py

@@ -11,9 +11,50 @@
 logger = setup_logger(__name__)
 
 
+def _parse_problem_details(response: requests.Response) -> dict[str, Any] | None:
+    """Parse a ProblemDetails body, returning None when the body is not a JSON object."""
+    try:
+        body = json.loads(response.text)
+    except (ValueError, TypeError):
+        return None
+    return body if isinstance(body, dict) else None
+
+
 def _get_trace_id_from_response(response: requests.Response) -> str | None:
-    response_dict = json.loads(response.text)
-    return response_dict.get("traceId")
+    problem = _parse_problem_details(response)
+    return problem.get("traceId") if problem else None
+
+
+def _format_validation_errors(errors: dict[str, Any]) -> str:
+    return "; ".join(
+        f"{field}: {', '.join(str(m) for m in messages)}"
+        if isinstance(messages, list)
+        else f"{field}: {messages}"
+        for field, messages in errors.items()
+    )
+
+
+def _describe_failure(response: requests.Response | None) -> str:
+    """Summarise why the API rejected the request, for the caller to act on."""
+    if response is None:
+        return "no response received"
+
+    problem = _parse_problem_details(response)
+    if problem is None:
+        text = (response.text or "").strip()
+        return f"HTTP {response.status_code}: {text[:200]}" if text else f"HTTP {response.status_code}"
+
+    parts: list[str] = []
+
+    reason = problem.get("detail") or problem.get("title")
+    if reason:
+        parts.append(str(reason))
+
+    errors = problem.get("errors")
+    if isinstance(errors, dict) and errors:
+        parts.append(_format_validation_errors(errors))
+
+    return "\n".join(parts) if parts else f"HTTP {response.status_code}"
 
 
 class Gateway:
@@ -153,8 +194,10 @@ def import_dataset(
                 else None
             )
             error_msg = (
-                f"Failed to create data catalog entry. correlation-id: {trace_id}"
+                f"Failed to create data catalog entry: {_describe_failure(e.response)}"
             )
+            if trace_id:
+                error_msg = f"{error_msg} (correlation-id: {trace_id})"
 
             logger.error(error_msg)
             raise type(e)(error_msg) from e

tests/test_gateway.py

@@ -40,7 +40,7 @@ def test_import_dataset_failure(mock_gateway, mock_post):
     response_body = {"type":"BusinessError","traceId":"8b01e7eb14484611add6138618daf112"}
 
     mock_response = MagicMock()
-    mock_response.return_value.status_code = 400
+    mock_response.status_code = 400
     mock_response.text = json.dumps(response_body)
 
     http_error = requests.exceptions.HTTPError()
@@ -52,5 +52,88 @@ def test_import_dataset_failure(mock_gateway, mock_post):
     with pytest.raises(RequestException) as e:
         mock_gateway.import_dataset("dataset_name", "dataset_description", "schema_id", {"tag": "value"}, "folder_id")
 
-    assert e.value.args[0] == f"Failed to create data catalog entry. correlation-id: {response_body['traceId']}"
+    assert response_body["traceId"] in e.value.args[0]
     mock_post.assert_called_once()
+
+
+def _failing_post(mock_post, *, text, status_code=400):
+    """Point requests.post at a response that raises HTTPError carrying `text`."""
+    response = MagicMock()
+    response.status_code = status_code
+    response.text = text
+
+    http_error = requests.exceptions.HTTPError()
+    http_error.response = response
+    response.raise_for_status.side_effect = http_error
+    mock_post.return_value = response
+    return response
+
+
+def _import(gateway):
+    return gateway.import_dataset(
+        "dataset_name", "dataset_description", "schema_id", {"tag": "value"}, "folder_id"
+    )
+
+
+def test_import_dataset_failure_surfaces_problem_detail(mock_gateway, mock_post):
+    """The `detail` the API explains the failure with must reach the caller."""
+    _failing_post(mock_post, text=json.dumps({
+        "title": "BadRequest",
+        "status": 400,
+        "detail": "Ensure DatasetName is unique when creating Predefined Dataset.",
+        "traceId": "abc123",
+    }))
+
+    with pytest.raises(RequestException) as e:
+        _import(mock_gateway)
+
+    assert "Ensure DatasetName is unique when creating Predefined Dataset." in e.value.args[0]
+    assert "abc123" in e.value.args[0]
+
+
+def test_import_dataset_failure_surfaces_validation_errors(mock_gateway, mock_post):
+    """Per-field validation errors are the actionable part of a 400."""
+    _failing_post(mock_post, text=json.dumps({
+        "title": "BadRequest",
+        "status": 400,
+        "errors": {"datasetName": ["must not be empty", "must be unique"]},
+        "traceId": "def456",
+    }))
+
+    with pytest.raises(RequestException) as e:
+        _import(mock_gateway)
+
+    assert "datasetName" in e.value.args[0]
+    assert "must not be empty" in e.value.args[0]
+    # The summary and the field errors belong on separate lines so the message stays readable.
+    assert "BadRequest\ndatasetName: must not be empty, must be unique" in e.value.args[0]
+
+
+def test_import_dataset_failure_falls_back_to_title(mock_gateway, mock_post):
+    """A body with no `detail` should still say something better than the trace id."""
+    _failing_post(mock_post, text=json.dumps({"title": "Conflict", "status": 409, "traceId": "ghi789"}))
+
+    with pytest.raises(RequestException) as e:
+        _import(mock_gateway)
+
+    assert "Conflict" in e.value.args[0]
+
+
+def test_import_dataset_failure_with_non_json_body(mock_gateway, mock_post):
+    """A gateway/proxy can answer with HTML; parsing it must not mask the real error."""
+    _failing_post(mock_post, text="<html><body>502 Bad Gateway</body></html>", status_code=502)
+
+    with pytest.raises(RequestException) as e:
+        _import(mock_gateway)
+
+    assert "502" in e.value.args[0]
+
+
+def test_import_dataset_failure_without_response(mock_gateway, mock_post):
+    """A connection error has no response at all."""
+    mock_post.side_effect = requests.exceptions.ConnectionError("connection refused")
+
+    with pytest.raises(RequestException) as e:
+        _import(mock_gateway)
+
+    assert "Failed to create data catalog entry" in e.value.args[0]