Skip to content

Commit

Permalink
catch 400 on list item unique role assignments (#1610)
Browse files Browse the repository at this point in the history
* Address specific edge case of 400 on list item unique role permissions

* Don't retry 400 errors

* Autoformat
  • Loading branch information
seanstory authored and elastic committed Sep 11, 2023
1 parent 0b9960a commit 6d04bc5
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 3 deletions.
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

0 comments on commit 6d04bc5

Please sign in to comment.