Skip to content
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
57 changes: 40 additions & 17 deletions tagbot/action/repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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:
Expand All @@ -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:
Comment thread
arnavk23 marked this conversation as resolved.
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)
Expand All @@ -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."""
Expand Down
5 changes: 5 additions & 0 deletions test/action/test_backfilling.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@
from tagbot.action.repo import Repo, _metrics


@patch("tagbot.action.repo.Github")
def _repo(
mock_github,
*,
repo="",
registry="",
Expand All @@ -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,
Expand Down
35 changes: 33 additions & 2 deletions test/action/test_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand All @@ -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 = ""
Expand Down