Try ignoring BadRequestError for SPO site list item attachments

connectors/sources/sharepoint_online.py

@@ -112,6 +112,17 @@
 SPO_MAX_EXPAND_SIZE = 20
 
 
+class UnexpectedClientError(Exception):
+    """Internal exception class for non-ok and non-error responses from API.
+    In some cases we receive non-okay responses from API that do not raise errors automatically.
+
+    These cases do not seem to be handleable, so we wrap them in this special error class. Mostly they seem
+    to happen when calling legacy REST sharepoint endpoints.
+    """
+
+    pass
+
+
 class NotFound(Exception):
     """Internal exception class to handle 404s from the API that has a meaning, that collection
     for specific object is empty.
@@ -455,7 +466,13 @@ async def _get(self, absolute_url, retry_count=0):
                 absolute_url,
                 headers=headers,
             ) as resp:
-                yield resp
+                if resp.ok:
+                    yield resp
+                else:
+                    msg = f"Received non-ok status: {resp.status}. Response body: {await resp.read()}"
+                    raise UnexpectedClientError(
+                        msg
+                    )
         except aiohttp.client_exceptions.ClientOSError:
             self._logger.warning(
                 "Graph API dropped the connection. It might indicate, that connector makes too many requests - decrease concurrency settings, otherwise Graph API can block this app."
@@ -871,7 +888,7 @@ async def site_list_item_has_unique_role_assignments(
             return response.get("value", False)
         except NotFound:
             return False
-        except BadRequestError:
+        except (BadRequestError, UnexpectedClientError):
             self._logger.warning(
                 f"Received error response when retrieving `{list_item_id}` from list: `{site_list_name}` in site: `{site_web_url}`"
             )
@@ -913,8 +930,9 @@ async def site_list_item_attachments(self, site_web_url, list_title, list_item_i
 
             for attachment in list_item["AttachmentFiles"]:
                 yield attachment
-        except NotFound:
+        except (NotFound, BadRequestError, UnexpectedClientError):
             # We can safely ignore cause Sharepoint can return 404 in case List Item is of specific types that do not support/have attachments
+            # 400 is sometimes returned for weird cases when Sharepoint Server responds with incorrect Content-Length header
             # Yes, makes no sense to me either.
             return
 

tests/sources/test_sharepoint_online.py

@@ -567,6 +567,26 @@ async def test_post_with_consecutive_batch_failures(
         with pytest.raises(ThrottledError):
             await microsoft_api_session.post(url, payload)
 
+    @pytest.mark.asyncio
+    async def test_fetch_with_client_payload_error(
+        self, microsoft_api_session, patch_sleep
+    ):
+        url = "http://localhost:1234/url"
+        response_mock = AsyncMock()
+        response_mock.status = 400
+        response_mock.ok = False
+        response_mock.read = AsyncMock(return_value="Failed to read body")
+        response_mock.json = AsyncMock(
+            # return_value = {}
+            side_effect=[aiohttp.client_exceptions.ClientPayloadError("something")]
+        )
+
+        async_response = AsyncMock()
+        async_response.__aenter__ = AsyncMock(return_value=response_mock)
+
+        with patch("aiohttp.client.ClientSession.get", return_value=async_response):
+            await microsoft_api_session.fetch(url)
+
     @pytest.mark.asyncio
     async def test_fetch_with_retry(
         self, microsoft_api_session, mock_responses, patch_sleep