diff --git a/tagbot/action/repo.py b/tagbot/action/repo.py index 1af04774..edbfdc81 100644 --- a/tagbot/action/repo.py +++ b/tagbot/action/repo.py @@ -1627,7 +1627,7 @@ def _release_already_exists(exc: GithubException) -> bool: if e.status == 422 and _release_already_exists(e): logger.info(f"Release for tag {version_tag} already exists, skipping") return - elif e.status == 403 and "resource not accessible" in str(e).lower(): + elif self._is_resource_not_accessible_error(e): logger.error( "Release creation blocked: token lacks required permissions. " "Use a PAT with contents:write (and workflows if tagging " @@ -1641,6 +1641,17 @@ def _release_already_exists(exc: GithubException) -> bool: raise logger.info(f"GitHub release {version_tag} created successfully") + @staticmethod + def _is_resource_not_accessible_error(exc: GithubException) -> bool: + """Identify GitHub's known 403 integration access error variant.""" + if exc.status != 403: + return False + data = getattr(exc, "data", {}) or {} + message = str(data.get("message", "")) if isinstance(data, dict) else str(data) + if "resource not accessible" in message.lower(): + return True + return "resource not accessible" in str(exc).lower() + def _check_rate_limit(self) -> None: """Check and log GitHub API rate limit status.""" try: @@ -1655,43 +1666,55 @@ def _check_rate_limit(self) -> None: def handle_error(self, e: Exception, *, raise_abort: bool = True) -> None: """Handle an unexpected error.""" - allowed = False internal = True + report_error = True + fatal = True trace = self._sanitize(traceback.format_exc()) if isinstance(e, Abort): # Abort is a characterized user-side failure (e.g., permission denied, # missing deploy key). Don't report to TagBotErrorReports — the user # gets a manual intervention issue on their repo instead. internal = False - allowed = True + report_error = False + fatal = False elif isinstance(e, RequestException): logger.warning("TagBot encountered a likely transient HTTP exception") logger.info(trace) - allowed = True + report_error = False + fatal = False elif isinstance(e, GithubException): logger.info(e.headers) if 500 <= e.status < 600: logger.warning("GitHub returned a 5xx error code") logger.info(trace) - allowed = True + report_error = False + fatal = False elif e.status == 401: logger.error( "GitHub returned 401 Bad credentials. Verify that your token " "is valid and has access to the repository and registry." ) internal = False - allowed = False elif e.status == 403: self._check_rate_limit() - logger.error( - "GitHub returned a 403 error. This may indicate: " - "1. Rate limiting - check the rate limit status above, " - "2. Insufficient permissions - verify your token & repo access, " - "3. Resource not accessible - see setup documentation" - ) - internal = False - allowed = False - if not allowed: + if self._is_resource_not_accessible_error(e): + logger.warning( + "GitHub returned 403 Resource not accessible by integration; " + "skipping internal error reporting for this known permissions " + "scenario." + ) + internal = False + report_error = False + else: + logger.error( + "GitHub returned a 403 error. This may indicate: " + "1. Rate limiting - check the rate limit status above, " + "2. Insufficient permissions - verify your token & repo " + "access, " + "3. Resource not accessible - see setup documentation" + ) + internal = False + if report_error: if internal: logger.error("TagBot experienced an unexpected internal failure") logger.info(trace) @@ -1700,8 +1723,8 @@ def handle_error(self, e: Exception, *, raise_abort: bool = True) -> None: except Exception: logger.error("Issue reporting failed") logger.info(traceback.format_exc()) - if raise_abort: - raise Abort("Cannot continue due to internal failure") + if fatal and raise_abort: + raise Abort("Cannot continue due to internal failure") def commit_sha_of_version(self, version: str) -> Optional[str]: """Get the commit SHA from a registered version.""" diff --git a/test/action/test_backfilling.py b/test/action/test_backfilling.py index 11a9715b..1d79dd41 100644 --- a/test/action/test_backfilling.py +++ b/test/action/test_backfilling.py @@ -9,7 +9,9 @@ from tagbot.action.repo import Repo, _metrics +@patch("tagbot.action.repo.Github") def _repo( + mock_github, *, repo="", registry="", @@ -30,6 +32,9 @@ def _repo( subdir=None, tag_prefix=None, ): + mock_gh_instance = Mock() + mock_github.return_value = mock_gh_instance + mock_gh_instance.get_repo.return_value = Mock() return Repo( repo=repo, registry=registry, diff --git a/test/action/test_repo.py b/test/action/test_repo.py index e94c595f..2323fc41 100644 --- a/test/action/test_repo.py +++ b/test/action/test_repo.py @@ -153,7 +153,9 @@ def test_project_subdir(): ) assert r._project("name") == "FooBar" assert r._project("uuid") == "abc-def" - r._repo.get_contents.assert_called_once_with("path/to/FooBar.jl/Project.toml") + r._repo.get_contents.assert_called_once_with( + os.path.join("path/to/FooBar.jl", "Project.toml") + ) r._repo.get_contents.side_effect = UnknownObjectException(404, "???", {}) r._Repo__project = None with pytest.raises(InvalidProject): @@ -974,7 +976,7 @@ def test_create_dispatch_event(): @patch("tagbot.action.repo.mkstemp", side_effect=[(0, "abc"), (0, "xyz")] * 3) @patch("os.chmod") @patch("subprocess.run") -@patch("pexpect.spawn") +@patch("pexpect.spawn", create=True) def test_configure_ssh(spawn, run, chmod, mkstemp): r = _repo(github="gh.com", repo="foo") r._repo = Mock(ssh_url="sshurl") @@ -1374,6 +1376,8 @@ def test_handle_error(mock_logger, format_exc): @patch("tagbot.action.repo.logger") def test_handle_error_403_checks_rate_limit(mock_logger, format_exc): r = _repo() + r._token = "" + r._registry_token = "" r._report_error = Mock() r._check_rate_limit = Mock() try: @@ -1384,6 +1388,33 @@ def test_handle_error_403_checks_rate_limit(mock_logger, format_exc): assert any("403" in str(call) for call in mock_logger.error.call_args_list) +@patch("traceback.format_exc", return_value="ahh") +@patch("tagbot.action.repo.logger") +def test_handle_error_403_resource_not_accessible_not_reported(mock_logger, format_exc): + r = _repo() + r._token = "" + r._registry_token = "" + r._report_error = Mock() + r._check_rate_limit = Mock() + + # Known permissions issue should not be treated as an internal failure. + with pytest.raises(Abort): + r.handle_error( + GithubException( + 403, + {"message": "Resource not accessible by integration"}, + {}, + ) + ) + + r._check_rate_limit.assert_called_once() + r._report_error.assert_not_called() + assert any( + "Resource not accessible by integration" in str(call) + for call in mock_logger.warning.call_args_list + ) + + def test_commit_sha_of_version(): r = _repo() r._Repo__registry_path = ""