nonetype handling for repo collaborators

cartography/intel/github/repos.py

@@ -14,6 +14,8 @@
 
 from cartography.intel.github.util import fetch_all
 from cartography.intel.github.util import PaginatedGraphqlData
+from cartography.util import backoff_handler
+from cartography.util import retries_with_backoff
 from cartography.util import run_cleanup_job
 from cartography.util import timeit
 
@@ -162,14 +164,27 @@ def _get_repo_collaborators_for_multiple_repos(
             result[repo_url] = []
             continue
 
-        collab_users = []
-        collab_permission = []
-        collaborators = _get_repo_collaborators(token, api_url, org, repo_name, affiliation)
-        # nodes and edges are expected to always be present given that we only call for them if totalCount is > 0
-        for collab in collaborators.nodes:
-            collab_users.append(collab)
-        for perm in collaborators.edges:
-            collab_permission.append(perm['permission'])
+        collab_users: List[dict[str, Any]] = []
+        collab_permission: List[str] = []
+
+        def get_repo_collaborators_inner_func(
+            org: str, api_url: str, token: str, repo_name: str, affiliation: str,
+            collab_users: List[dict[str, Any]], collab_permission: List[str],
+        ) -> None:
+            logger.info(f"Loading {affiliation} collaborators for repo {repo_name}.")
+            collaborators = _get_repo_collaborators(token, api_url, org, repo_name, affiliation)
+            # nodes and edges are expected to always be present given that we only call for them if totalCount is > 0
+            # however sometimes GitHub returns None, as in issue 1334
+            for collab in collaborators.nodes or []:
+                collab_users.append(collab)
+            # The `or []` is because `.edges` can be None.
+            for perm in collaborators.edges:
+                collab_permission.append(perm['permission'])
+
+        retries_with_backoff(get_repo_collaborators_inner_func, TypeError, 5, backoff_handler)(
+            org=org, api_url=api_url, token=token, repo_name=repo_name, affiliation=affiliation,
+            collab_users=collab_users, collab_permission=collab_permission,
+        )
 
         result[repo_url] = [
             UserAffiliationAndRepoPermission(user, permission, affiliation)

tests/unit/cartography/intel/github/test_repos.py

@@ -0,0 +1,35 @@
+from unittest.mock import MagicMock
+from unittest.mock import patch
+
+import pytest
+
+from cartography.intel.github.repos import _get_repo_collaborators_for_multiple_repos
+
+
+@patch('cartography.intel.github.repos._get_repo_collaborators')
+@patch('cartography.intel.github.repos.backoff_handler', spec=True)
+def test_get_team_users_github_returns_none(mock_backoff_handler, mock_get_team_collaborators):
+    """
+    This test happens to use 'OUTSIDE' affiliation, but it's irrelevant for the test, it just needs either valid value.
+    """
+    # Arrange
+    repo_data = [{'name': 'repo1', 'url': 'https://github.com/repo1', 'outsideCollaborators': {'totalCount': 1}}]
+    mock_repo_collabs = MagicMock()
+    # Set up the condition where GitHub returns a None url and None edge as in #1334.
+    mock_repo_collabs.nodes = [None]
+    mock_repo_collabs.edges = [None]
+    mock_get_team_collaborators.return_value = mock_repo_collabs
+
+    # Assert we raise an exception
+    with pytest.raises(TypeError):
+        _get_repo_collaborators_for_multiple_repos(
+            repo_data,
+            'OUTSIDE',
+            'test-org',
+            'https://api.github.com',
+            'test-token',
+        )
+
+    # Assert that we retry and give up
+    assert mock_get_team_collaborators.call_count == 5
+    assert mock_backoff_handler.call_count == 4