diff --git a/.github/workflows/release_draft.yml b/.github/workflows/release_draft.yml index 44a43482..2561787c 100644 --- a/.github/workflows/release_draft.yml +++ b/.github/workflows/release_draft.yml @@ -60,7 +60,7 @@ jobs: - name: Generate Release Notes id: generate_release_notes - uses: AbsaOSS/generate-release-notes@B90223510d1704301a36a36f0d86a72a0e72f0cf # v1.0.0 + uses: AbsaOSS/generate-release-notes@bugfix/322-Compare-Mode-Fails-with-404-When-Target-Tag-Not-Yet-Created env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: diff --git a/docs/features/compare_mode.md b/docs/features/compare_mode.md index bdf9efc4..62a008e0 100644 --- a/docs/features/compare_mode.md +++ b/docs/features/compare_mode.md @@ -98,6 +98,48 @@ Issues are always filtered by timestamp regardless of mode. --- +## Fallback Behavior — Compare-Endpoint 404 / Eventual Consistency + +In CI/CD pipelines it is common to call the release notes generator immediately after pushing a new tag. +Due to eventual consistency, GitHub's compare endpoint may return a 404 even for a tag that was just pushed +and is technically resolvable via the API. + +**Fallback strategy:** + +If the compare API fails with a 404, the action performs the following: +1. Resolves the requested target ref (tag/branch/SHA) directly via `repo.get_commit()` +2. Retries the compare operation using the from-tag name and the resolved commit's SHA: `repo.compare(from_tag, commit_sha)` +3. A warning is logged to indicate the fallback occurred +4. If the target ref cannot be resolved, the action exits with a non-zero code +5. If the retry compare call fails (network error, GithubException), it is treated the same as the first call: errors are logged and the action exits + +For non-404 errors on the initial compare (auth failures, rate limits, server errors) the action logs the error and exits immediately without attempting fallback. + +This scenario is handled gracefully when tags are created in a sequence: + +```yaml +- name: Build & Tag Release + run: | + ./scripts/build.sh + git tag v2.6.4 + git push origin v2.6.4 + +- name: Generate Release Notes # Runs immediately after tagging + uses: AbsaOSS/generate-release-notes@v1 + with: + tag-name: v2.6.4 + from-tag-name: v2.6.3 +``` + +**Log output when fallback is triggered:** + +```log +2026-06-25 12:22:27 - INFO - Compare mode: using repo.compare('v2.6.3', 'v2.6.4'). +2026-06-25 12:22:27 - WARNING - Compare API returned 404 for 'v2.6.3'...'v2.6.4': target tag does not exist. Falling back to latest commit on default branch. +``` + +--- + ## Data Flow ``` @@ -109,8 +151,16 @@ from-tag-name provided? GitHub Compare API: get_commits(since=data.since) commits unique to to-tag get_pulls(state=closed) │ │ - extract PR numbers FilterByRelease drops - from commit messages PRs/commits before since + API fails (404)? FilterByRelease drops + │ PRs/commits before since + ├─ YES: Fallback to + │ get_commit(target) + │ retry compare with + │ resolved SHA + │ (log warning) + │ + extract PR numbers + from commit messages │ fetch each PR by number │ diff --git a/release_notes_generator/data/miner.py b/release_notes_generator/data/miner.py index dc32e271..81450fe3 100644 --- a/release_notes_generator/data/miner.py +++ b/release_notes_generator/data/miner.py @@ -28,7 +28,9 @@ import semver from github import Github from github.GitRelease import GitRelease +from github import GithubException from github.Issue import Issue +from requests.exceptions import ConnectionError as RequestsConnectionError, Timeout, RequestException from github.PullRequest import PullRequest from github.Repository import Repository from github.Commit import Commit as GithubCommit @@ -56,6 +58,7 @@ class DataMiner: def __init__(self, github_instance: Github, rate_limiter: GithubRateLimiter): self.github_instance = github_instance + self._rate_limiter = rate_limiter self._safe_call = safe_call_decorator(rate_limiter) def mine_data(self) -> MinedData: @@ -99,6 +102,11 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: Logic: - Fetch commits between from_tag and to_tag using repo.compare(). + - If compare fails with 404 (target tag does not exist yet), + fall back to the latest commit on the default branch. + - If compare fails with other errors (auth, rate-limit, transient), log and exit. + - If fallback resolve succeeds, use the latest commit on default branch as baseline. + - If fallback resolve also fails, log and exit. - Extract PR numbers from commit messages and fetch those PRs. - Filter out commits that already have a PR reference to avoid duplication. """ @@ -107,14 +115,72 @@ def _handle_compare_mode(self, repo: Repository, data: MinedData) -> None: ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name(), ) - comparison = self._safe_call(repo.compare)(ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()) - if comparison is None: + comparison = None + try: + comparison = self._rate_limiter(repo.compare)(ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name()) + except (RequestsConnectionError, Timeout, RequestException) as e: logger.error( - "Compare API returned no result for '%s'...'%s'. Ending!", + "Network error during compare API call for '%s'...'%s': %s", ActionInputs.get_from_tag_name(), ActionInputs.get_tag_name(), + e, ) sys.exit(1) + except GithubException as e: + if e.status == 404: + logger.warning( + "Compare API returned 404 for '%s'...'%s': target tag does not exist. " + "Falling back to latest commit on default branch.", + ActionInputs.get_from_tag_name(), + ActionInputs.get_tag_name(), + ) + else: + logger.error( + "Compare API failed for '%s'...'%s' with status %d: %s", + ActionInputs.get_from_tag_name(), + ActionInputs.get_tag_name(), + e.status, + e.data.get("message", str(e)) if isinstance(e.data, dict) else str(e), + ) + sys.exit(1) + + if comparison is None: + # Fall back: target tag/ref does not exist yet; resolve it directly + target_commit = self._safe_call(repo.get_commit)(ActionInputs.get_tag_name()) + if target_commit is None: + logger.error( + "Could not retrieve commit for target ref '%s'. Ending!", + ActionInputs.get_tag_name(), + ) + sys.exit(1) + # Retry compare using the resolved commit SHA as the target + try: + comparison = self._rate_limiter(repo.compare)(ActionInputs.get_from_tag_name(), target_commit.sha) + except (RequestsConnectionError, Timeout, RequestException) as e: + logger.error( + "Network error during retry compare API call for '%s'...'%s': %s", + ActionInputs.get_from_tag_name(), + target_commit.sha, + e, + ) + sys.exit(1) + except GithubException as e: + logger.error( + "Retry compare API failed for '%s'...'%s' with status %d: %s", + ActionInputs.get_from_tag_name(), + target_commit.sha, + e.status, + e.data.get("message", str(e)) if isinstance(e.data, dict) else str(e), + ) + sys.exit(1) + + if comparison is None: + logger.error( + "Compare API returned no result for '%s'...'%s'. Ending!", + ActionInputs.get_from_tag_name(), + target_commit.sha, + ) + sys.exit(1) compare_commits: list[GithubCommit] = list(comparison.commits) total_commits = getattr(comparison, "total_commits", None) if isinstance(total_commits, int) and total_commits > len(compare_commits): diff --git a/tests/unit/release_notes_generator/data/test_miner.py b/tests/unit/release_notes_generator/data/test_miner.py index 2c6a3554..0fd95768 100644 --- a/tests/unit/release_notes_generator/data/test_miner.py +++ b/tests/unit/release_notes_generator/data/test_miner.py @@ -21,6 +21,7 @@ from typing import Optional from github import Github +from github import GithubException from github.Commit import Commit from github.GitRelease import GitRelease from github.Issue import Issue @@ -305,7 +306,7 @@ def test_mine_data_commits_without_since(mocker, mock_repo): miner = DataMiner(gh, mocker.Mock()) # Bypass safe_call wrapper - miner._safe_call = lambda f: f + miner._safe_call = decorator_mock # Inputs / release behavior mocker.patch( @@ -332,7 +333,7 @@ def test_scan_sub_issues_for_parents(mocker, mock_repo, mined_data_simple): # miner setup miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = lambda f: f + miner._safe_call = decorator_mock mocker.patch.object(miner, "_make_bulk_sub_issue_collector", return_value=ChildBulkSubIssueCollector()) mocker.patch.object(miner, "_fetch_all_repositories_in_cache", return_value=None) @@ -355,7 +356,7 @@ def test_fetch_all_repositories_in_cache(mocker, mock_repo, mined_data_simple): # miner setup miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = lambda f: f + miner._safe_call = decorator_mock patch_parents_sub_issues: dict[str, list[str]] = {} patch_parents_sub_issues["org_1/another_repo#122"] = ["org_2/another_repo#122", "org_3/another_repo#122", "o/r#1"] @@ -404,7 +405,7 @@ def fake_get_issue(num): # miner setup miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = lambda f: f + miner._safe_call = decorator_mock patch_parents_sub_issues: dict[str, list[str]] = {} patch_parents_sub_issues["org/repo#1"] = [ @@ -454,7 +455,7 @@ def fake_get_issue(num): # miner setup miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = lambda f: f + miner._safe_call = decorator_mock patch_parents_sub_issues: dict[str, list[str]] = {} @@ -477,7 +478,7 @@ def test_fetch_prs_for_fetched_cross_issues(mocker, mock_repo): # Miner with safe_call bypassed gh = mocker.Mock() miner = DataMiner(gh, mocker.Mock()) - miner._safe_call = lambda f: f # no decorator wrapping + miner._safe_call = decorator_mock # PR object returned by as_pull_request() pr_obj = mocker.Mock(spec=PullRequest) @@ -605,6 +606,7 @@ def _make_compare_miner(mocker, mock_repo, *, from_tag="v2.6.3", to_tag="v2.6.4" miner = DataMiner(github_mock, mocker.Mock()) miner._safe_call = decorator_mock + miner._rate_limiter = decorator_mock return miner @@ -746,6 +748,171 @@ def test_mine_data_compare_mode_warns_on_retrieval_cap_without_total(mocker, moc ) +def test_mine_data_compare_mode_fallback_to_target_sha_on_404(mocker, mock_repo): + """Test that when compare API raises 404, fallback resolves the target ref and retries compare.""" + target_commit = mocker.Mock() + target_commit.sha = "targetsha123" + target_commit.commit.message = "Latest commit on target ref (no PR ref)" + + mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v2.6.3") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v2.6.4") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_github_repository", return_value="org/repo") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_published_at", return_value=False) + + release_mock = mocker.Mock(spec=GitRelease) + release_mock.created_at = datetime(2026, 5, 7) + release_mock.published_at = None + release_mock.tag_name = "v2.6.3" + mock_repo.get_release.return_value = release_mock + + mock_repo.get_commit.return_value = target_commit + + fallback_comparison = mocker.Mock() + fallback_comparison.commits = [target_commit] + fallback_comparison.total_commits = 1 + + # First compare raises 404; second (with resolved SHA) succeeds + mock_repo.compare.side_effect = [ + GithubException(404, {"message": "Comparison failed"}), + fallback_comparison, + ] + mock_repo.get_issues.return_value = [] + + warning_mock = mocker.patch("release_notes_generator.data.miner.logger.warning") + + github_mock = mocker.Mock(spec=Github) + github_mock.get_repo.return_value = mock_repo + + miner = DataMiner(github_mock, mocker.Mock()) + miner._safe_call = decorator_mock + miner._rate_limiter = decorator_mock + data = miner.mine_data() + + # Verify 404 warning was logged + first_warning = warning_mock.call_args_list[0] + assert "Compare API returned 404" in first_warning[0][0] + assert "Falling back to latest commit on default branch" in first_warning[0][0] + + # Verify fallback resolved the target ref via get_commit + mock_repo.get_commit.assert_called_once_with("v2.6.4") + + # Verify second compare call used from_tag and resolved SHA + assert mock_repo.compare.call_count == 2 + mock_repo.compare.assert_called_with("v2.6.3", "targetsha123") + + # Verify the fallback commit was included in the result + assert "targetsha123" in data.compare_commit_shas, "Target commit SHA must be in compare_commit_shas" + assert len(data.commits) == 1, "Fallback commit should be included in data.commits" + mock_repo.compare.assert_called_with("v2.6.3", "targetsha123") + + # Verify the fallback commit was included in the result + assert "targetsha123" in data.compare_commit_shas, "Target commit SHA must be in compare_commit_shas" + assert len(data.commits) == 1, "Fallback commit should be included in data.commits" + + +def test_mine_data_compare_mode_exits_when_fallback_branch_not_found(mocker, mock_repo): + """Test that action exits when compare API raises 404 and get_commit returns None.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v2.6.3") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v2.6.4") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_github_repository", return_value="org/repo") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_published_at", return_value=False) + + release_mock = mocker.Mock(spec=GitRelease) + release_mock.created_at = datetime(2026, 5, 7) + release_mock.published_at = None + release_mock.tag_name = "v2.6.3" + mock_repo.get_release.return_value = release_mock + + # compare raises 404; get_commit returns None (ref not resolvable) + mock_repo.compare.side_effect = GithubException(404, {"message": "Not Found"}) + mock_repo.get_commit.return_value = None + mock_repo.get_issues.return_value = [] + + github_mock = mocker.Mock(spec=Github) + github_mock.get_repo.return_value = mock_repo + + miner = DataMiner(github_mock, mocker.Mock()) + miner._safe_call = decorator_mock + miner._rate_limiter = decorator_mock + + with pytest.raises(SystemExit): + miner.mine_data() + + +def test_mine_data_compare_mode_exits_when_fallback_compare_returns_none(mocker, mock_repo): + """Test that action exits when compare API raises 404 and the fallback compare call returns None.""" + target_commit = mocker.Mock() + target_commit.sha = "targetsha123" + + mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v2.6.3") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v2.6.4") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_github_repository", return_value="org/repo") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_published_at", return_value=False) + + release_mock = mocker.Mock(spec=GitRelease) + release_mock.created_at = datetime(2026, 5, 7) + release_mock.published_at = None + release_mock.tag_name = "v2.6.3" + mock_repo.get_release.return_value = release_mock + + mock_repo.get_commit.return_value = target_commit + + # First compare raises 404; second (fallback) returns None + mock_repo.compare.side_effect = [ + GithubException(404, {"message": "Not Found"}), + None, + ] + mock_repo.get_issues.return_value = [] + + github_mock = mocker.Mock(spec=Github) + github_mock.get_repo.return_value = mock_repo + + miner = DataMiner(github_mock, mocker.Mock()) + miner._safe_call = decorator_mock + miner._rate_limiter = decorator_mock + + with pytest.raises(SystemExit): + miner.mine_data() + + +def test_mine_data_compare_mode_exits_on_non_404_github_exception(mocker, mock_repo): + """Non-404 GithubException from compare API must exit immediately without falling back.""" + mocker.patch("release_notes_generator.action_inputs.ActionInputs.is_from_tag_name_defined", return_value=True) + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_from_tag_name", return_value="v2.6.3") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_tag_name", return_value="v2.6.4") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_github_repository", return_value="org/repo") + mocker.patch("release_notes_generator.action_inputs.ActionInputs.get_published_at", return_value=False) + + release_mock = mocker.Mock(spec=GitRelease) + release_mock.created_at = datetime(2026, 5, 7) + release_mock.published_at = None + release_mock.tag_name = "v2.6.3" + mock_repo.get_release.return_value = release_mock + + mock_repo.compare.side_effect = GithubException(500, {"message": "Internal Server Error"}) + mock_repo.get_issues.return_value = [] + + error_mock = mocker.patch("release_notes_generator.data.miner.logger.error") + + github_mock = mocker.Mock(spec=Github) + github_mock.get_repo.return_value = mock_repo + + miner = DataMiner(github_mock, mocker.Mock()) + miner._safe_call = decorator_mock + miner._rate_limiter = decorator_mock + + with pytest.raises(SystemExit): + miner.mine_data() + + # Fallback (get_commit) must NOT have been attempted + mock_repo.get_commit.assert_not_called() + # Error must have been logged with the non-404 status + assert any("500" in str(call) for call in error_mock.call_args_list) + + # --- mine_data timestamp mode (regression) ---