feat: enhance error handling in batch operations and improve related tests

Author: Samson Gebre

src/PowerPlatform/Dataverse/data/_batch.py

@@ -11,7 +11,7 @@
 from dataclasses import dataclass, field
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
 
-from ..core.errors import MetadataError, ValidationError
+from ..core.errors import HttpError, MetadataError, ValidationError
 from ..core._error_codes import METADATA_TABLE_NOT_FOUND, METADATA_COLUMN_NOT_FOUND
 from ..models.batch import BatchItemResponse, BatchResult
 from ..models.relationship import (
@@ -545,7 +545,12 @@ def _parse_batch_response(self, response: Any) -> BatchResult:
         content_type = response.headers.get("Content-Type", "")
         boundary = _extract_boundary(content_type)
         if not boundary:
-            return BatchResult()
+            # Non-multipart response: the batch request itself was rejected by Dataverse
+            # (common for top-level 4xx, e.g. malformed body, missing OData headers).
+            # Returning an empty BatchResult() here would silently hide the error and
+            # make has_errors=False, which is actively misleading. Raise instead.
+            _raise_top_level_batch_error(response)
+            return BatchResult()  # unreachable; satisfies type checkers
         parts = _split_multipart(response.text or "", boundary)
         responses: List[BatchItemResponse] = []
         for part_headers, part_body in parts:
@@ -569,6 +574,31 @@ def _parse_batch_response(self, response: Any) -> BatchResult:
 # ---------------------------------------------------------------------------
 
 
+def _raise_top_level_batch_error(response: Any) -> None:
+    """Parse a non-multipart batch response and raise HttpError with the service message.
+
+    Dataverse returns ``application/json`` with an ``{"error": {...}}`` payload when
+    it rejects the batch request at the HTTP level (e.g. malformed multipart body,
+    missing OData headers). This helper surfaces that detail instead of silently
+    returning an empty ``BatchResult``.
+    """
+    status_code: int = getattr(response, "status_code", 0)
+    service_error_code: Optional[str] = None
+    try:
+        payload = response.json()
+        error = payload.get("error", {})
+        service_error_code = error.get("code") or None
+        message: str = error.get("message") or response.text or "Unexpected non-multipart response from $batch"
+    except Exception:
+        message = (getattr(response, "text", None) or "") or "Unexpected non-multipart response from $batch"
+    raise HttpError(
+        message=f"Batch request rejected by Dataverse: {message}",
+        status_code=status_code,
+        subcode="4xx" if 400 <= status_code < 500 else ("5xx" if status_code >= 500 else None),
+        service_error_code=service_error_code,
+    )
+
+
 def _extract_boundary(content_type: str) -> Optional[str]:
     m = re.search(r'boundary="?([^";,\s]+)"?', content_type, re.IGNORECASE)
     return m.group(1) if m else None

src/PowerPlatform/Dataverse/operations/batch.py

@@ -251,9 +251,9 @@ def upsert(
         """
         Add an upsert operation to the batch.
 
-        Mirrors :meth:`~PowerPlatform.Dataverse.operations.records.RecordOperations.upsert`
-        exactly: a single item becomes a PATCH request using the alternate key; multiple
-        items become one ``UpsertMultiple`` POST.
+        Mirrors :meth:`~PowerPlatform.Dataverse.operations.records.RecordOperations.upsert`:
+        a single item becomes a PATCH request using the alternate key; multiple items
+        become one ``UpsertMultiple`` POST.
 
         Each item must be a :class:`~PowerPlatform.Dataverse.models.upsert.UpsertItem`
         or a plain ``dict`` with ``"alternate_key"`` and ``"record"`` keys (both dicts).
@@ -262,6 +262,10 @@ def upsert(
         :param items: Non-empty list of :class:`~PowerPlatform.Dataverse.models.upsert.UpsertItem`
             instances or equivalent dicts.
 
+        :raises TypeError: If ``items`` is not a non-empty list, or if any element is
+            neither a :class:`~PowerPlatform.Dataverse.models.upsert.UpsertItem` nor a
+            dict with ``"alternate_key"`` and ``"record"`` keys.
+
         Example::
 
             from PowerPlatform.Dataverse.models.upsert import UpsertItem
@@ -278,18 +282,15 @@ def upsert(
             ])
         """
         if not isinstance(items, list) or not items:
-            raise ValidationError("items must be a non-empty list", subcode="VALIDATION_UPSERT_EMPTY")
+            raise TypeError("items must be a non-empty list of UpsertItem or dicts")
         normalized: List[UpsertItem] = []
         for i in items:
             if isinstance(i, UpsertItem):
                 normalized.append(i)
             elif isinstance(i, dict) and isinstance(i.get("alternate_key"), dict) and isinstance(i.get("record"), dict):
                 normalized.append(UpsertItem(alternate_key=i["alternate_key"], record=i["record"]))
             else:
-                raise ValidationError(
-                    "Each upsert item must be a UpsertItem or a dict with 'alternate_key' and 'record' keys.",
-                    subcode="VALIDATION_UPSERT_ITEM",
-                )
+                raise TypeError("Each item must be an UpsertItem or a dict with 'alternate_key' and 'record' keys")
         self._batch._items.append(_RecordUpsert(table=table, items=normalized))
 
 

src/PowerPlatform/Dataverse/operations/records.py

@@ -521,7 +521,7 @@ def upsert(self, table: str, items: List[Union[UpsertItem, Dict[str, Any]]]) ->
             elif isinstance(i, dict) and isinstance(i.get("alternate_key"), dict) and isinstance(i.get("record"), dict):
                 normalized.append(UpsertItem(alternate_key=i["alternate_key"], record=i["record"]))
             else:
-                raise TypeError("Each item must be a UpsertItem or a dict with 'alternate_key' and 'record' keys")
+                raise TypeError("Each item must be an UpsertItem or a dict with 'alternate_key' and 'record' keys")
         with self._client._scoped_odata() as od:
             entity_set = od._entity_set_from_schema_name(table)
             if len(normalized) == 1:

tests/unit/data/test_batch_serialization.py

@@ -19,11 +19,13 @@
     _TableList,
     _QuerySql,
     _extract_boundary,
+    _raise_top_level_batch_error,
     _split_multipart,
     _parse_mime_part,
     _parse_http_response_part,
     _CRLF,
 )
+from PowerPlatform.Dataverse.core.errors import HttpError
 from PowerPlatform.Dataverse.models.upsert import UpsertItem
 from PowerPlatform.Dataverse.data._raw_request import _RawRequest
 from PowerPlatform.Dataverse.models.batch import BatchItemResponse, BatchResult
@@ -430,18 +432,16 @@ def test_upsert_plain_dict_normalised_to_upsert_item(self):
 
     def test_upsert_empty_list_raises(self):
         from PowerPlatform.Dataverse.operations.batch import BatchRecordOperations
-        from PowerPlatform.Dataverse.core.errors import ValidationError
 
         rec_ops, _ = self._make_batch()
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(TypeError):
             rec_ops.upsert("account", [])
 
     def test_upsert_invalid_item_raises(self):
         from PowerPlatform.Dataverse.operations.batch import BatchRecordOperations
-        from PowerPlatform.Dataverse.core.errors import ValidationError
 
         rec_ops, _ = self._make_batch()
-        with self.assertRaises(ValidationError):
+        with self.assertRaises(TypeError):
             rec_ops.upsert("account", ["not_a_valid_item"])
 
     def test_upsert_multiple_items_all_normalised(self):
@@ -461,5 +461,65 @@ def test_upsert_multiple_items_all_normalised(self):
         self.assertEqual(intent.items[1].alternate_key, {"accountnumber": "B"})
 
 
+class TestRaiseTopLevelBatchError(unittest.TestCase):
+    """_raise_top_level_batch_error surfaces Dataverse error details as HttpError."""
+
+    def _make_response(self, status_code, json_body=None, text=None):
+        resp = MagicMock()
+        resp.status_code = status_code
+        resp.text = text or ""
+        if json_body is not None:
+            resp.json.return_value = json_body
+        else:
+            resp.json.side_effect = ValueError("no JSON")
+        return resp
+
+    def test_raises_http_error(self):
+        """Always raises HttpError, never returns."""
+        resp = self._make_response(400, json_body={"error": {"code": "0x0", "message": "Bad batch"}})
+        with self.assertRaises(HttpError):
+            _raise_top_level_batch_error(resp)
+
+    def test_status_code_preserved(self):
+        """HttpError.status_code matches the response status code."""
+        resp = self._make_response(400, json_body={"error": {"code": "0x0", "message": "Bad batch"}})
+        with self.assertRaises(HttpError) as ctx:
+            _raise_top_level_batch_error(resp)
+        self.assertEqual(ctx.exception.status_code, 400)
+
+    def test_service_message_in_exception(self):
+        """The Dataverse error message is included in the raised exception."""
+        resp = self._make_response(400, json_body={"error": {"code": "BadRequest", "message": "Malformed OData batch"}})
+        with self.assertRaises(HttpError) as ctx:
+            _raise_top_level_batch_error(resp)
+        self.assertIn("Malformed OData batch", str(ctx.exception))
+
+    def test_service_error_code_preserved(self):
+        """The Dataverse error code is forwarded into HttpError.details."""
+        resp = self._make_response(400, json_body={"error": {"code": "0x80040216", "message": "..."}})
+        with self.assertRaises(HttpError) as ctx:
+            _raise_top_level_batch_error(resp)
+        self.assertEqual(ctx.exception.details.get("service_error_code"), "0x80040216")
+
+    def test_falls_back_to_response_text_when_no_json(self):
+        """Falls back to response.text when the body is not valid JSON."""
+        resp = self._make_response(400, text="plain text error body")
+        with self.assertRaises(HttpError) as ctx:
+            _raise_top_level_batch_error(resp)
+        self.assertIn("plain text error body", str(ctx.exception))
+
+    def test_parse_batch_response_raises_on_missing_boundary(self):
+        """_BatchClient._parse_batch_response raises HttpError for non-multipart responses."""
+        od = _make_od()
+        client = _BatchClient(od)
+        resp = MagicMock()
+        resp.headers = {"Content-Type": "application/json"}
+        resp.status_code = 400
+        resp.text = ""
+        resp.json.return_value = {"error": {"code": "0x0", "message": "Invalid batch"}}
+        with self.assertRaises(HttpError):
+            client._parse_batch_response(resp)
+
+
 if __name__ == "__main__":
     unittest.main()

tests/unit/data/test_sql_parse.py

@@ -1,7 +1,6 @@
 # Copyright (c) Microsoft Corporation.
 # Licensed under the MIT license.
 
-import unittest
 from unittest.mock import patch
 from urllib.parse import parse_qs, urlparse