fix: set ResponseError.status for failed replies

DAVError gained a status attribute in 3.6.0, but a failing status
inside a multistatus (a 507 <propstat> or <response> status in a
PROPFIND reply, say) raised a ResponseError with status None, so the
code could only be read from the message text.  That check now
passes the code, or None for a status line it cannot parse.  For
consistency the parse_* methods' non-2xx raises go through
error_from_response(), keeping body and preconditions too, and the
foreign-href sync 507 passes its code.

Prompt: A 507 from propfind comes back as a ResponseError with status set to None

Assisted-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reviewed-by: Tobias Brox <tobias@redpill-linpro.com>

Author: tobixen

caldav/collection.py

@@ -2349,7 +2349,9 @@ def _sync_page_truncated(self, response: Any) -> bool:
         ## while the calendar URL may keep a literal @ (Nextcloud).
         marker = unquote(self.url.join(self._href_to_url(href)).path).rstrip("/")
         if marker != unquote(self.url.path).rstrip("/"):
-            raise error.ResponseError(f"sync-collection: 507 Insufficient Storage for {href}")
+            raise error.ResponseError(
+                f"sync-collection: 507 Insufficient Storage for {href}", status=507
+            )
         return True
 
     def _absorb_sync_page(

caldav/response.py

@@ -100,7 +100,16 @@ def _validate_status(status: str | None) -> None:
     if status is None:
         return
     if not any(code in status for code in (" 200 ", " 201 ", " 207 ", " 404 ")):
-        raise error.ResponseError(status)
+        raise error.ResponseError(status, status=_parse_status_code(status))
+
+
+def _parse_status_code(status: str) -> int | None:
+    """The code in a status line like "HTTP/1.1 507 Insufficient Storage",
+    or None if there is none (unlike _status_to_code, no 200 fallback)."""
+    parts = status.split()
+    if len(parts) >= 2 and parts[1].isdigit():
+        return int(parts[1])
+    return None
 
 
 def _is_sync_truncation(response: _Element, siblings: Iterable[_Element]) -> bool:
@@ -497,7 +506,9 @@ def parse_propfind(self) -> "list[PropfindResult]":
         if self.status == 404:
             return []
         if self.status not in (200, 207):
-            raise error.ResponseError(f"PROPFIND failed with status {self.status}")
+            raise error.error_from_response(
+                error.ResponseError, self, f"PROPFIND failed with status {self.status}"
+            )
         if self.tree is None:
             return []
         results: list[PropfindResult] = []
@@ -520,7 +531,9 @@ def parse_propfind(self) -> "list[PropfindResult]":
     def parse_calendar_query(self) -> "list[CalendarQueryResult]":
         """Parse the response body as a calendar-query or calendar-multiget REPORT reply."""
         if self.status not in (200, 207):
-            raise error.ResponseError(f"REPORT failed with status {self.status}")
+            raise error.error_from_response(
+                error.ResponseError, self, f"REPORT failed with status {self.status}"
+            )
         if self.tree is None:
             return []
         results: list[CalendarQueryResult] = []
@@ -542,7 +555,9 @@ def parse_calendar_query(self) -> "list[CalendarQueryResult]":
     def parse_sync_collection(self) -> "SyncCollectionResult":
         """Parse the response body as a sync-collection REPORT reply."""
         if self.status not in (200, 207):
-            raise error.ResponseError(f"sync-collection failed with status {self.status}")
+            raise error.error_from_response(
+                error.ResponseError, self, f"sync-collection failed with status {self.status}"
+            )
         if self.tree is None:
             return SyncCollectionResult()
         changed: list[CalendarQueryResult] = []

changes/+response-error-status.fixed.md

@@ -0,0 +1 @@
+A `ResponseError` raised for a failed PROPFIND, REPORT or sync-collection reply, or for a failing `<propstat>` status inside a multistatus (e.g. `507 Insufficient Storage`), now has its `status` attribute set; it was `None`, so the code had to be parsed out of the message.  The failed replies also keep `body` and `preconditions`, like other `DAVError`s.

tests/test_caldav_unit.py

@@ -5881,13 +5881,15 @@ class TestPropstatStatusValidation:
 """
 
     def test_server_error_propstat_raises(self):
-        with pytest.raises(error.ResponseError):
+        with pytest.raises(error.ResponseError) as excinfo:
             MockedDAVResponse(self.XML).expand_simple_props(props=[dav.DisplayName()])
+        assert excinfo.value.status == 500
 
     def test_insufficient_storage_propstat_raises(self):
         xml = self.XML.replace("500 Internal Server Error", "507 Insufficient Storage")
-        with pytest.raises(error.ResponseError):
+        with pytest.raises(error.ResponseError) as excinfo:
             MockedDAVResponse(xml).expand_simple_props(props=[dav.DisplayName()])
+        assert excinfo.value.status == 507
 
     def test_404_propstat_is_still_a_missing_property(self):
         """The 404 quirk must survive: a 404 propstat means "not set here",
@@ -5896,6 +5898,17 @@ def test_404_propstat_is_still_a_missing_property(self):
         result = MockedDAVResponse(xml).expand_simple_props(props=[dav.DisplayName()])
         assert result == {"/dav/calendars/user/broken/": {"{DAV:}displayname": None}}
 
+    @pytest.mark.parametrize(
+        "status", ["500 Internal Server Error", "HTTP/1.1 Internal Server Error"]
+    )
+    def test_unparseable_status_line_leaves_status_unset(self, status):
+        """A status line the client cannot read is no evidence of a code:
+        ``status`` stays None rather than claiming 200."""
+        xml = self.XML.replace("HTTP/1.1 500 Internal Server Error", status)
+        with pytest.raises(error.ResponseError) as excinfo:
+            MockedDAVResponse(xml).expand_simple_props(props=[dav.DisplayName()])
+        assert excinfo.value.status is None
+
 
 class TestAsyncDAVClientCredentialPairing:
     """Gate finding F6, async twin: same field-by-field merge, same result."""
@@ -7658,8 +7671,9 @@ def test_507_for_another_href_is_not_a_truncation(self):
             b"<D:href>/dav/cal/</D:href>", b"<D:href>/dav/othercal/</D:href>"
         )
         _, calendar = self._calendar(page)
-        with pytest.raises(error.DAVError):
+        with pytest.raises(error.DAVError) as excinfo:
             calendar.get_objects_by_sync_token("tok-1", disable_fallback=True)
+        assert excinfo.value.status == 507
 
     def test_truncated_initial_sync_on_stalwart_stops_short(self):
         """Stalwart's checkpoint of a truncated initial sync skips members.

tests/test_protocol.py

@@ -151,6 +151,17 @@ def test_parse_propfind_404_returns_empty(self):
         results = DAVResponse.from_bytes(b"", status_code=404).parse_propfind()
         assert results == []
 
+    @pytest.mark.parametrize(
+        "method", ["parse_propfind", "parse_calendar_query", "parse_sync_collection"]
+    )
+    def test_parse_failure_keeps_status(self, method):
+        """A failing reply raises a ResponseError carrying the HTTP status."""
+        from caldav.lib import error
+
+        with pytest.raises(error.ResponseError) as excinfo:
+            getattr(DAVResponse.from_bytes(b"", status_code=507), method)()
+        assert excinfo.value.status == 507
+
     def test_parse_calendar_query_response(self):
         """Parse calendar-query response with calendar data."""
         xml = b"""<?xml version="1.0"?>
@@ -279,8 +290,9 @@ def test_507_outside_sync_reply_still_raises(self):
                 <D:status>HTTP/1.1 507 Insufficient Storage</D:status>
             </D:response>
         </D:multistatus>"""
-        with pytest.raises(error.ResponseError):
+        with pytest.raises(error.ResponseError) as excinfo:
             DAVResponse.from_bytes(xml).expand_simple_props([dav.GetEtag()])
+        assert excinfo.value.status == 507
 
     def test_parse_sync_collection_generic_responsedescription(self):
         """A 404 <response> may carry an arbitrary <responsedescription>.