diff --git a/connectors/sources/sharepoint_online.py b/connectors/sources/sharepoint_online.py index acc09d241..7c0f4da21 100644 --- a/connectors/sources/sharepoint_online.py +++ b/connectors/sources/sharepoint_online.py @@ -95,6 +95,15 @@ class NotFound(Exception): pass +class BadRequestError(Exception): + """Internal exception class to handle 400's from the API. + + Similar to the NotFound exception, this allows us to catch edge-case responses that should + be translated as empty resutls, and let us return [].""" + + pass + + class InternalServerError(Exception): """Exception class to indicate that something went wrong on the server side.""" @@ -289,7 +298,7 @@ async def wrapped(*args, **kwargs): async for item in func(*args, **kwargs): yield item break - except NotFound: + except (NotFound, BadRequestError): raise except Exception: if retry >= retries: @@ -432,6 +441,9 @@ async def _handle_client_response_error(self, absolute_url, e): raise NotFound from e # We wanna catch it in the code that uses this and ignore in some cases elif e.status == 500: raise InternalServerError from e + elif e.status == 400: + self._logger.warning(f"Received 400 response from {absolute_url}") + raise BadRequestError from e else: raise @@ -704,6 +716,11 @@ async def site_list_item_has_unique_role_assignments( return response.get("value", False) except NotFound: return False + except BadRequestError: + self._logger.warning( + f"Received error response when retrieving `{list_item_id}` from list: `{site_list_name}` in site: `{site_web_url}`" + ) + return False async def site_list_item_role_assignments( self, site_web_url, site_list_name, list_item_id diff --git a/tests/sources/test_sharepoint_online.py b/tests/sources/test_sharepoint_online.py index e46f6618f..cd3b7a7dd 100644 --- a/tests/sources/test_sharepoint_online.py +++ b/tests/sources/test_sharepoint_online.py @@ -24,6 +24,7 @@ ACCESS_CONTROL, DEFAULT_RETRY_SECONDS, WILDCARD, + BadRequestError, DriveItemsPage, GraphAPIToken, InternalServerError, @@ -627,8 +628,6 @@ async def test_call_api_with_404_without_retry_after_header( not_found_error.status = 404 not_found_error.message = "Something went wrong" - mock_responses.get(url, exception=not_found_error) - mock_responses.get(url, exception=not_found_error) mock_responses.get(url, exception=not_found_error) with pytest.raises(NotFound) as e: @@ -637,6 +636,29 @@ async def test_call_api_with_404_without_retry_after_header( assert e is not None + @pytest.mark.asyncio + async def test_call_api_with_400_without_retry_after_header( + self, + microsoft_api_session, + mock_responses, + patch_sleep, + patch_cancellable_sleeps, + ): + url = "http://localhost:1234/download-some-sample-file" + + # First throttle, then do not throttle + bad_request_error = ClientResponseError(None, None) + bad_request_error.status = 400 + bad_request_error.message = "You did something wrong" + + mock_responses.get(url, exception=bad_request_error) + + with pytest.raises(BadRequestError) as e: + async with microsoft_api_session._get(url) as _: + pass + + assert e is not None + @pytest.mark.asyncio async def test_call_api_with_unhandled_status( self, @@ -1257,6 +1279,20 @@ async def test_site_list_item_has_unique_role_assignments_not_found( site_list_item_role_assignments_url, list_title, list_item_id ) + @pytest.mark.asyncio + async def test_site_list_item_has_unique_role_assignments_bad_request( + self, client, patch_fetch + ): + site_list_item_role_assignments_url = f"https://{self.tenant_name}.sharepoint.com/random/totally/made/up/roleassignments" + list_title = "list_title" + list_item_id = 1 + + patch_fetch.side_effect = BadRequestError() + + assert not await client.site_list_item_has_unique_role_assignments( + site_list_item_role_assignments_url, list_title, list_item_id + ) + @pytest.mark.asyncio async def test_site_list_item_role_assignments(self, client, patch_scroll): site_list_item_role_assignments_url = f"https://{self.tenant_name}.sharepoint.com/random/totally/made/up/roleassignments"