Fix/groups sync (#684)

* fix: raise errors for catch statements in calling func

* fix: handle errors on getting users details

* feat: introduce retry request util function

* fix: properly handle errors

* fix: reorder args position

* fix: reorder args position

* fix: lint

app/integrations/google_workspace/google_directory.py

@@ -9,7 +9,7 @@
     DEFAULT_DELEGATED_ADMIN_EMAIL,
     DEFAULT_GOOGLE_WORKSPACE_CUSTOMER_ID,
 )
-from integrations.utils.api import convert_string_to_camel_case
+from integrations.utils.api import convert_string_to_camel_case, retry_request
 from utils import filters
 
 logger = getLogger(__name__)
@@ -209,37 +209,61 @@ def list_groups_with_members(
 
     groups_with_members = []
     for group in filtered_groups:
-        error_occured = False
         logger.info(f"Getting members for group: {group['email']}")
         try:
-            members = list_group_members(
-                group["email"], fields="members(email, role, type, status)"
+            members = retry_request(
+                list_group_members,
+                group["email"],
+                max_attempts=3,
+                delay=1,
+                fields="members(email, role, type, status)",
             )
         except Exception as e:
             logger.warning(f"Error getting members for group {group['email']}: {e}")
             continue
 
-        for member in members:
-            user_details = {}
-            try:
-                logger.info(f"Getting user details for member: {member['email']}")
-                user_details = get_user(member["email"], fields="name, primaryEmail")
-            except Exception as e:
-                logger.warning(
-                    f"Error getting user details for member {member['email']}: {e}"
-                )
-                error_occured = True
-                if not tolerate_errors:
-                    break
-            if user_details:
-                member.update(user_details)
-        if members and (not error_occured or tolerate_errors):
+        members = get_members_details(members, tolerate_errors)
+        if members:
             group.update({"members": members})
             groups_with_members.append(group)
 
     return groups_with_members
 
 
+def get_members_details(members: list[dict], tolerate_errors=False):
+    """Get user details for a list of members.
+
+    Args:
+        members (list): A list of member objects.
+        tolerate_errors (bool): Whether to tolerate errors when getting user details.
+
+    Returns:"""
+
+    error_occured = False
+    for member in members:
+        user_details = {}
+        try:
+            logger.info(f"Getting user details for member: {member['email']}")
+            user_details = retry_request(
+                get_user,
+                member["email"],
+                max_attempts=3,
+                delay=1,
+                fields="name, primaryEmail",
+            )
+        except Exception as e:
+            logger.warning(
+                f"Error getting user details for member {member['email']}: {e}"
+            )
+            error_occured = True
+            if not tolerate_errors:
+                break
+        if user_details:
+            member.update(user_details)
+
+    return members if not error_occured or tolerate_errors else []
+
+
 def convert_google_groups_members_to_dataframe(groups):
     """Converts a list of Google groups with members to a DataFrame.
 

app/integrations/google_workspace/google_service.py

@@ -106,18 +106,22 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
             logging.error(
                 f"An HTTP error occurred in function '{func.__module__}:{func.__name__}': {e}"
             )
+            raise e
         except ValueError as e:
             logging.error(
                 f"A ValueError occurred in function '{func.__module__}:{func.__name__}': {e}"
             )
+            raise e
         except RefreshError as e:
             logging.error(
                 f"A RefreshError occurred in function '{func.__module__}:{func.__name__}': {e}"
             )
+            raise e
         except Error as e:
             logging.error(
                 f"An error occurred in function '{func.__module__}:{func.__name__}': {e}"
             )
+            raise e
         except Exception as e:  # Catch-all for any other types of exceptions
             message = str(e)
             func_name = func.__name__
@@ -131,7 +135,7 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
                 logging.error(
                     f"An unexpected error occurred in function '{func.__module__}:{func.__name__}': {e}"
                 )
-        return None
+            raise e
 
     return wrapper
 

app/integrations/utils/api.py

@@ -3,6 +3,8 @@
 import re
 import string
 import random
+import time
+import logging
 
 
 def convert_string_to_camel_case(snake_str):
@@ -85,3 +87,33 @@ def generate_segment():
     unique_id = "-".join(segments)
 
     return unique_id
+
+
+def retry_request(
+    func,
+    *args,
+    max_attempts=3,
+    delay=1,
+    **kwargs,
+):
+    """Retry a function up to a maximum number of attempts with a delay between each attempt.
+
+    Args:
+        func (function): The function to call.
+        max_attempts (int): The maximum number of attempts to make.
+        delay (int): The delay between each attempt in seconds.
+        *args: Positional arguments to pass to the function.
+        **kwargs: Keyword arguments to pass to the function.
+
+    Returns:
+        Any: The result of the function call.
+    """
+    for i in range(max_attempts):
+        try:
+            return func(*args, **kwargs)
+        except Exception as e:
+            if i == max_attempts - 1:
+                logging.warning(f"Error after {max_attempts} attempts: {e}")
+                raise e
+            time.sleep(delay)
+            continue

app/tests/integrations/google_workspace/test_google_calendar.py

@@ -470,11 +470,12 @@ def test_insert_event_api_call_error(
     emails = ["[email protected]", "[email protected]"]
     title = "Test Event"
     document_id = "test_document_id"
-    google_calendar.insert_event(start, end, emails, title, document_id)
-    assert (
-        "An unexpected error occurred in function 'integrations.google_workspace.google_calendar:insert_event': API call error"
-        in caplog.text
-    )
+    with pytest.raises(Exception):
+        google_calendar.insert_event(start, end, emails, title, document_id)
+        assert (
+            "An unexpected error occurred in function 'integrations.google_workspace.google_calendar:insert_event': API call error"
+            in caplog.text
+        )
     assert not mock_convert_string_to_camel_case.called
     assert mock_os_environ_get.called
     assert not mock_handle_errors.called

app/tests/integrations/google_workspace/test_google_directory.py

@@ -395,13 +395,11 @@ def test_list_groups_with_members_filtered(
     assert mock_get_user.call_count == 2
 
 
+@patch("integrations.google_workspace.google_directory.retry_request")
 @patch("integrations.google_workspace.google_directory.list_groups")
-@patch("integrations.google_workspace.google_directory.list_group_members")
-@patch("integrations.google_workspace.google_directory.get_user")
 def test_list_groups_with_members_error_in_list_group_members(
-    mock_get_user,
-    mock_list_group_members,
     mock_list_groups,
+    mock_retry_request,
     google_groups,
     google_group_members,
     google_users,
@@ -412,8 +410,12 @@ def test_list_groups_with_members_error_in_list_group_members(
     users = google_users(2)
 
     mock_list_groups.return_value = groups
-    mock_list_group_members.side_effect = group_members
-    mock_get_user.side_effect = users
+    mock_retry_request.side_effect = [
+        group_members[0],
+        group_members[1],
+        users[0],
+        users[1],
+    ]
 
     # Only the second group should be processed
     expected_groups_with_users = [groups[1]]
@@ -424,13 +426,13 @@ def test_list_groups_with_members_error_in_list_group_members(
     assert google_directory.list_groups_with_members() == expected_groups_with_users
 
 
+@patch("integrations.google_workspace.google_directory.get_members_details")
+@patch("integrations.google_workspace.google_directory.retry_request")
 @patch("integrations.google_workspace.google_directory.list_groups")
-@patch("integrations.google_workspace.google_directory.list_group_members")
-@patch("integrations.google_workspace.google_directory.get_user")
 def test_list_groups_with_members_error_in_get_user(
-    mock_get_user,
-    mock_list_group_members,
     mock_list_groups,
+    mock_retry_request,
+    mock_get_members_details,
     google_groups,
     google_group_members,
     google_users,
@@ -445,8 +447,14 @@ def test_list_groups_with_members_error_in_get_user(
     ]
 
     mock_list_groups.return_value = groups
-    mock_list_group_members.side_effect = group_members
-    mock_get_user.side_effect = users
+    mock_retry_request.side_effect = [
+        group_members[0],
+        group_members[1],
+    ]
+    mock_get_members_details.side_effect = [
+        [],
+        group_members[1],
+    ]
 
     # Only the second group should be processed
     expected_groups_with_users = [groups[1]]
@@ -457,13 +465,11 @@ def test_list_groups_with_members_error_in_get_user(
     assert google_directory.list_groups_with_members() == expected_groups_with_users
 
 
+@patch("integrations.google_workspace.google_directory.retry_request")
 @patch("integrations.google_workspace.google_directory.list_groups")
-@patch("integrations.google_workspace.google_directory.list_group_members")
-@patch("integrations.google_workspace.google_directory.get_user")
 def test_list_groups_with_members_tolerate_errors(
-    mock_get_user,
-    mock_list_group_members,
     mock_list_groups,
+    mock_retry_request,
     google_groups_w_users,
 ):
 
@@ -501,8 +507,14 @@ def test_list_groups_with_members_tolerate_errors(
     ]
 
     mock_list_groups.return_value = groups
-    mock_list_group_members.side_effect = group_members
-    mock_get_user.side_effect = users
+    mock_retry_request.side_effect = [
+        group_members[0],
+        users[0],
+        users[1],
+        group_members[1],
+        users[2],
+        users[3],
+    ]
 
     # Expected result should include both groups, with the second group having one member
     expected_groups_with_users = [
@@ -617,3 +629,52 @@ def test_list_groups_with_members_filtered_dataframe(
         "member_given_name",
         "member_family_name",
     }
+
+
+@patch("integrations.google_workspace.google_directory.retry_request")
+def test_get_members_details_breaks_on_error(mock_retry_request):
+    members = [
+        {"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
+        {"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
+    ]
+
+    users = [
+        Exception("Error fetching user details"),
+        {"name": "user2", "primaryEmail": "email2"},
+    ]
+
+    mock_retry_request.side_effect = [users[0], users[1]]
+
+    result = google_directory.get_members_details(members)
+
+    assert result == []
+
+
+@patch("integrations.google_workspace.google_directory.retry_request")
+def test_get_members_details_continues_on_tolerate_errors(mock_retry_request):
+    members = [
+        {"email": "email1", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
+        {"email": "email2", "role": "MEMBER", "type": "USER", "status": "ACTIVE"},
+    ]
+
+    users = [
+        Exception("Error fetching user details"),
+        {"name": "user2", "primaryEmail": "email2"},
+    ]
+
+    mock_retry_request.side_effect = [users[0], users[1]]
+
+    result = google_directory.get_members_details(members, tolerate_errors=True)
+
+    expected_result = [
+        members[0],
+        {
+            "email": "email2",
+            "role": "MEMBER",
+            "type": "USER",
+            "status": "ACTIVE",
+            "name": "user2",
+            "primaryEmail": "email2",
+        },
+    ]
+    assert result == expected_result

app/tests/integrations/google_workspace/test_google_drive.py

@@ -1,7 +1,7 @@
 """Unit tests for google_drive module."""
 
 from unittest.mock import patch, call
-
+import pytest
 from integrations.google_workspace import google_drive
 
 
@@ -152,9 +152,10 @@ def test_create_file_with_invalid_type_raises_value_error(
     execute_google_api_call_mock.side_effect = ValueError(
         "Invalid file_type: invalid_file_type"
     )
-    result = google_drive.create_file("name", "folder", "invalid_file_type")
+    with pytest.raises(ValueError):
+        result = google_drive.create_file("name", "folder", "invalid_file_type")
+        assert result is None
 
-    assert result is None
     mocked_logging_error.assert_called_once_with(
         "A ValueError occurred in function 'integrations.google_workspace.google_drive:create_file': Invalid file_type: invalid_file_type"
     )