Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.10] catch 400 on list item unique role assignments (#1610) #1613

Merged
merged 1 commit into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion connectors/sources/sharepoint_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,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."""

Expand Down Expand Up @@ -286,7 +295,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:
Expand Down Expand Up @@ -429,6 +438,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

Expand Down Expand Up @@ -701,6 +713,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
Expand Down
40 changes: 38 additions & 2 deletions tests/sources/test_sharepoint_online.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
ACCESS_CONTROL,
DEFAULT_RETRY_SECONDS,
WILDCARD,
BadRequestError,
DriveItemsPage,
GraphAPIToken,
InternalServerError,
Expand Down Expand Up @@ -611,8 +612,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:
Expand All @@ -621,6 +620,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,
Expand Down Expand Up @@ -1241,6 +1263,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"
Expand Down