diff --git a/.github/workflows/release-notes.yaml b/.github/workflows/release-notes.yaml index 3834f84..a23b649 100644 --- a/.github/workflows/release-notes.yaml +++ b/.github/workflows/release-notes.yaml @@ -57,6 +57,7 @@ jobs: enable-cache: false - id: towncrier name: ⚙️ Read Towncrier configuration + if: inputs.changelog == '' uses: $/read-towncrier-config - name: 📦 Install locked release tools uses: $/install-locked-tools diff --git a/README.md b/README.md index 1b486cf..9c029ec 100644 --- a/README.md +++ b/README.md @@ -871,7 +871,8 @@ workflow. `release-notes.yaml` takes these inputs and returns one output: - `tag` **(required)**: release tag, such as `1.2.3` or `pyproject-fmt/1.2.3`. -- `changelog`: tracked changelog path. Empty reads `filename` from the committed Towncrier configuration. +- `changelog`: tracked changelog path. Empty reads `filename` from the committed Towncrier configuration, so a + repository without one, such as a multi-package workspace, must set it. - `artifact-url` and `artifact-label`: passed to `render-release-notes`. - `package`: package section of a shared Markdown changelog. The previous tag is then the one before the tag in the same package namespace. @@ -1138,6 +1139,12 @@ leaves the draft for a later retry. The Action removes incomplete `starter` asse the release by ID for up to ten seconds after draft creation or asset upload; a mismatch still fails when that window ends. +A GitHub API call that fails with a server error or a network error is retried up to three times, with backoff; a 4xx +fails at once. Draft creation is the exception: a request GitHub had already applied would be sent again and could leave +a second draft, so that call is not retried. If the publish call fails, the Action reads the release again and succeeds +when GitHub already published it with the expected assets and notes, which covers a response lost after GitHub applied +the change. + ## Reconcile a post-release pull request `publish-post-release-pr` consumes a candidate commit prepared without an App token. The preparation job records the diff --git a/changelog.d/+notes-changelog-input.bugfix.md b/changelog.d/+notes-changelog-input.bugfix.md new file mode 100644 index 0000000..c6911bc --- /dev/null +++ b/changelog.d/+notes-changelog-input.bugfix.md @@ -0,0 +1,2 @@ +`release-notes.yaml` reads the Towncrier configuration only when the `changelog` input is empty, so a repository without +`[tool.towncrier]`, such as a multi-package workspace, can render its release notes when it passes `changelog`. diff --git a/changelog.d/+publish-retry.bugfix.md b/changelog.d/+publish-retry.bugfix.md new file mode 100644 index 0000000..584c600 --- /dev/null +++ b/changelog.d/+publish-retry.bugfix.md @@ -0,0 +1,3 @@ +`publish-github-release` retries a GitHub API call that fails without a client error, up to three attempts with backoff. +When the publish call fails, the Action reads the release again and succeeds if GitHub already published it with the +expected assets and notes, so a lost response no longer fails a release that is live. Draft creation is not retried. diff --git a/publish-github-release/action.yml b/publish-github-release/action.yml index 6caedd8..aeb66d0 100644 --- a/publish-github-release/action.yml +++ b/publish-github-release/action.yml @@ -57,7 +57,9 @@ runs: TAG: ${{ inputs.tag }} run: | set -euo pipefail - gh api --paginate "repos/$GH_REPO/releases?per_page=100" | + # shellcheck disable=SC1091 # GitHub supplies this Action path at runtime. + source "$GITHUB_ACTION_PATH/retry.sh" + gh_api --paginate "repos/$GH_REPO/releases?per_page=100" | jq -s --arg tag "$TAG" '[.[][] | select(.tag_name == $tag)]' >"$RUNNER_TEMP/release-existing.json" if jq -e 'length > 0' "$RUNNER_TEMP/release-existing.json" >/dev/null; then jq -er '.[0].id' "$RUNNER_TEMP/release-existing.json" >"$RUNNER_TEMP/release-id" @@ -84,6 +86,8 @@ runs: NOTES_FILE: ${{ inputs.notes-file }} run: | set -euo pipefail + # shellcheck disable=SC1091 # GitHub supplies this Action path at runtime. + source "$GITHUB_ACTION_PATH/retry.sh" if jq -e '.[0].draft == false' "$RUNNER_TEMP/release-existing.json" >/dev/null; then echo "New Contributors skipped: GitHub Release $TAG is already published" cp "$NOTES_FILE" "$RUNNER_TEMP/release-body.md" @@ -96,7 +100,8 @@ runs: request+=(-f "previous_tag_name=$previous") fi echo "Generated notes range: ${previous:-first release}...$TAG" - gh api -X POST "repos/$GH_REPO/releases/generate-notes" "${request[@]}" --jq .body >"$RUNNER_TEMP/generated-notes.md" + gh_api -X POST "repos/$GH_REPO/releases/generate-notes" "${request[@]}" --jq .body \ + >"$RUNNER_TEMP/generated-notes.md" python3 "$GITHUB_ACTION_PATH/../tools/new_contributors.py" \ "$NOTES_FILE" "$RUNNER_TEMP/generated-notes.md" "$RUNNER_TEMP/release-body.md" - name: 🚀 Create or update the draft @@ -109,11 +114,14 @@ runs: PRERELEASE: ${{ inputs.prerelease }} run: | set -euo pipefail + # shellcheck disable=SC1091 # GitHub supplies this Action path at runtime. + source "$GITHUB_ACTION_PATH/retry.sh" if jq -e 'length == 0' "$RUNNER_TEMP/release-existing.json" >/dev/null; then jq -n --arg tag "$TAG" --arg commit "$COMMIT" --arg title "$TITLE" \ --rawfile body "$RUNNER_TEMP/release-body.md" --argjson prerelease "$PRERELEASE" \ '{tag_name: $tag, target_commitish: $commit, name: $title, body: $body, draft: true, prerelease: $prerelease}' >"$RUNNER_TEMP/release-request.json" + # Not retried: if GitHub stored the draft before answering 5xx, a second POST could create another. gh api -X POST "repos/$GH_REPO/releases" --input "$RUNNER_TEMP/release-request.json" \ >"$RUNNER_TEMP/release-created.json" jq -er '.id | select(type == "number")' "$RUNNER_TEMP/release-created.json" >"$RUNNER_TEMP/release-id" @@ -123,7 +131,7 @@ runs: "$RUNNER_TEMP/release-existing.json" >/dev/null; then jq -n --arg title "$TITLE" --rawfile body "$RUNNER_TEMP/release-body.md" --argjson prerelease "$PRERELEASE" \ '{name: $title, body: $body, prerelease: $prerelease}' >"$RUNNER_TEMP/release-update.json" - gh api -X PATCH "repos/$GH_REPO/releases/$(<"$RUNNER_TEMP/release-id")" \ + gh_api -X PATCH "repos/$GH_REPO/releases/$(<"$RUNNER_TEMP/release-id")" \ --input "$RUNNER_TEMP/release-update.json" \ --jq '"GitHub Release \(.tag_name): updated draft \(.id) to the expected title, notes and prerelease flag"' elif jq -e '.[0].draft' "$RUNNER_TEMP/release-existing.json" >/dev/null; then @@ -155,11 +163,11 @@ runs: if jq -e '.[0].draft' "$RUNNER_TEMP/release-existing.json" >/dev/null; then while IFS= read -r asset_id; do echo "Deleting incomplete draft asset $asset_id left by an earlier attempt" - gh api -X DELETE "repos/$GH_REPO/releases/assets/$asset_id" + gh_api -X DELETE "repos/$GH_REPO/releases/assets/$asset_id" done < <(jq -r '.[0].assets[] | select(.state != "uploaded") | .id' "$RUNNER_TEMP/release-existing.json") fi read_draft() { - gh api "repos/$GH_REPO/releases/$release_id" | jq -s . >"$RUNNER_TEMP/release-current.json" && + gh_api "repos/$GH_REPO/releases/$release_id" | jq -s . >"$RUNNER_TEMP/release-current.json" && jq -e --arg phase draft --slurpfile assets "$RUNNER_TEMP/release-assets.json" \ -f "$GITHUB_ACTION_PATH/match.jq" "$RUNNER_TEMP/release-current.json" >/dev/null } @@ -185,20 +193,20 @@ runs: else for attempt in {1..6}; do if ((attempt > 1)); then - release=$(gh api "repos/$GH_REPO/releases/$release_id") + release=$(gh_api "repos/$GH_REPO/releases/$release_id") asset_ids=$(jq -r --arg name "$name" \ '.assets[] | select(.name == $name and .state != "uploaded") | .id' <<<"$release") while IFS= read -r asset_id; do [[ -n "$asset_id" ]] || continue echo "Deleting incomplete upload $asset_id of $name before retrying" - gh api -X DELETE "repos/$GH_REPO/releases/assets/$asset_id" + gh_api -X DELETE "repos/$GH_REPO/releases/assets/$asset_id" done <<<"$asset_ids" fi echo "Asset $name: uploading (attempt $attempt of 6)" if gh release upload "$TAG" "$asset" --repo "$GH_REPO"; then break fi - if gh api "repos/$GH_REPO/releases/$release_id" | jq -e --arg name "$name" \ + if gh_api "repos/$GH_REPO/releases/$release_id" | jq -e --arg name "$name" \ --slurpfile assets "$RUNNER_TEMP/release-assets.json" ' any(.assets[]; .name == $name and .state == "uploaded" and (.digest as $digest | any($assets[0][]; .name == $name and .digest == $digest))) @@ -229,7 +237,7 @@ runs: source "$GITHUB_ACTION_PATH/retry.sh" release_id=$(<"$RUNNER_TEMP/release-id") read_complete() { - gh api "repos/$GH_REPO/releases/$release_id" | jq -s . >"$RUNNER_TEMP/release-complete.json" && + gh_api "repos/$GH_REPO/releases/$release_id" | jq -s . >"$RUNNER_TEMP/release-complete.json" && jq -e --arg phase complete --slurpfile assets "$RUNNER_TEMP/release-assets.json" \ -f "$GITHUB_ACTION_PATH/match.jq" "$RUNNER_TEMP/release-complete.json" >/dev/null } @@ -244,15 +252,36 @@ runs: shell: bash env: GH_REPO: ${{ github.repository }} + TAG: ${{ inputs.tag }} MODE: ${{ inputs.mode }} run: | set -euo pipefail + # shellcheck disable=SC1091 # GitHub supplies this Action path at runtime. + source "$GITHUB_ACTION_PATH/retry.sh" if [[ "$MODE" == stage ]]; then echo 'Publish skipped: mode stage leaves the release as a draft' elif jq -e '.[0].draft' "$RUNNER_TEMP/release-complete.json" >/dev/null; then release_id=$(<"$RUNNER_TEMP/release-id") - gh api -X PATCH "repos/$GH_REPO/releases/$release_id" -F draft=false \ - --jq '"GitHub Release \(.tag_name): published \(.html_url)"' + publish_draft() { + gh_api -X PATCH "repos/$GH_REPO/releases/$release_id" -F draft=false \ + --jq '"GitHub Release \(.tag_name): published \(.html_url)"' + } + read_published() { + gh_api "repos/$GH_REPO/releases/$release_id" | jq -s . >"$RUNNER_TEMP/release-published.json" && + jq -e --arg phase complete --slurpfile assets "$RUNNER_TEMP/release-assets.json" \ + -f "$GITHUB_ACTION_PATH/match.jq" "$RUNNER_TEMP/release-published.json" >/dev/null && + jq -e --rawfile body "$RUNNER_TEMP/release-body.md" '.[0] | .draft == false and .body == $body' \ + "$RUNNER_TEMP/release-published.json" >/dev/null + } + # A publish call can fail after GitHub has already published the release, so the state decides. + if ! publish_draft; then + echo "::warning::publish call for GitHub Release $TAG failed; reading the release to check it" >&2 + if ! retry read_published; then + echo "::error::GitHub Release $TAG is not published with the expected assets and notes" >&2 + exit 1 + fi + echo "GitHub Release $TAG: already published with the expected assets and notes" + fi else echo 'Publish skipped: the release is already published' fi diff --git a/publish-github-release/retry.sh b/publish-github-release/retry.sh index 528ea28..e8ea956 100644 --- a/publish-github-release/retry.sh +++ b/publish-github-release/retry.sh @@ -9,3 +9,16 @@ retry() { sleep 2 done } + +# Retries a gh api call unless GitHub answered with a 4xx, which repeats itself. gh prints a non-JSON 5xx body as a +# parse error without the status, so the 4xx marker is the reliable test. +gh_api() { + local attempt errors="$RUNNER_TEMP/gh-api-error.txt" + for attempt in 1 2 3; do + if gh api "$@" 2>"$errors"; then return 0; fi + cat "$errors" >&2 + if ((attempt == 3)) || grep -q 'HTTP 4[0-9][0-9]' "$errors"; then return 1; fi + echo "GitHub API call failed without a client error, attempt $attempt of 3, retrying in $((attempt * 5))s" + sleep $((attempt * 5)) + done +} diff --git a/tests/publish_github_release/test_publish.py b/tests/publish_github_release/test_publish.py index a0e6912..25ea3de 100644 --- a/tests/publish_github_release/test_publish.py +++ b/tests/publish_github_release/test_publish.py @@ -55,6 +55,19 @@ def _release_requests(server: HTTPServer) -> list[Request]: return [request for request, _ in server.log if request.path != _GENERATE_NOTES] +def _requests(server: HTTPServer, method: str, path: str) -> list[Request]: + return [request for request, _ in server.log if request.method == method and request.path == path] + + +def _expect_draft_for_publish(server: HTTPServer, notes: Path) -> None: + prefix = "/api/v3/repos/tox-dev/fixture" + server.expect_ordered_request(f"{prefix}/releases", query_string="per_page=100", method="GET").respond_with_json( + [_release(notes, draft=True)] + ) + _expect_generated_notes(server) + server.expect_ordered_request(f"{prefix}/releases/42", method="GET").respond_with_json(_release(notes, draft=True)) + + def _release_calls(tmp_path: Path) -> list[tuple[str, ...]]: path = tmp_path / "release-cli-arguments" return ( @@ -802,3 +815,179 @@ def test_new_release_credits_first_time_contributors_above_the_links( generate, create = (request.get_json() for request, _ in server.log if request.method == "POST") assert generate == {"tag_name": "0.1.0", "target_commitish": commit, "previous_tag_name": "0.0.9"} assert create["body"] == credited + + +@pytest.mark.parametrize( + ("status", "body", "mimetype"), + [ + pytest.param(502, "Bad Gateway", "text/html", id="html-body"), + pytest.param(500, '{"message": "Server Error"}', "application/json", id="json-body"), + ], +) +def test_server_error_on_generated_notes_is_retried( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], + status: int, + body: str, + mimetype: str, +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + prefix = "/api/v3/repos/tox-dev/fixture" + server.expect_ordered_request(f"{prefix}/releases", query_string="per_page=100", method="GET").respond_with_json([]) + server.expect_ordered_request(_GENERATE_NOTES, method="POST").respond_with_data( + body, status=status, mimetype=mimetype + ) + _expect_generated_notes(server) + server.expect_ordered_request(f"{prefix}/releases", method="POST").respond_with_json( + _release(notes, draft=True), status=201 + ) + server.expect_ordered_request(f"{prefix}/releases/42", method="GET").respond_with_json(_release(notes, draft=True)) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli, mode="stage") + assert result.returncode == 0, result.stderr + assert len(_requests(server, "POST", _GENERATE_NOTES)) == 2 + + +def test_client_error_on_generated_notes_is_not_retried( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + prefix = "/api/v3/repos/tox-dev/fixture" + server.expect_ordered_request(f"{prefix}/releases", query_string="per_page=100", method="GET").respond_with_json([]) + server.expect_ordered_request(_GENERATE_NOTES, method="POST").respond_with_json( + {"message": "Not Found"}, status=404 + ) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli, mode="stage") + assert result.returncode != 0 + assert "Not Found (HTTP 404)" in result.stderr + assert len(_requests(server, "POST", _GENERATE_NOTES)) == 1 + + +def test_draft_creation_is_not_retried( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + prefix = "/api/v3/repos/tox-dev/fixture" + server.expect_ordered_request(f"{prefix}/releases", query_string="per_page=100", method="GET").respond_with_json([]) + _expect_generated_notes(server) + server.expect_ordered_request(f"{prefix}/releases", method="POST").respond_with_data(status=502) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli, mode="stage") + assert result.returncode != 0 + assert len(_requests(server, "POST", f"{prefix}/releases")) == 1 + + +def test_server_error_on_publish_is_retried( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + _expect_draft_for_publish(server, notes) + prefix = "/api/v3/repos/tox-dev/fixture" + server.expect_ordered_request(f"{prefix}/releases/42", method="PATCH").respond_with_data(status=502) + server.expect_ordered_request(f"{prefix}/releases/42", method="PATCH").respond_with_json( + _release(notes, draft=False) + ) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli) + assert result.returncode == 0, result.stderr + assert [request.method for request in _release_requests(server)] == ["GET", "GET", "PATCH", "PATCH"] + + +def test_failed_publish_is_accepted_when_release_is_published( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + _expect_draft_for_publish(server, notes) + prefix = "/api/v3/repos/tox-dev/fixture" + for _ in range(3): + server.expect_ordered_request(f"{prefix}/releases/42", method="PATCH").respond_with_data(status=502) + server.expect_ordered_request(f"{prefix}/releases/42", method="GET").respond_with_json(_release(notes, draft=False)) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli) + assert result.returncode == 0, result.stderr + assert "already published" in result.stdout + assert [request.method for request in _release_requests(server)] == ["GET", "GET", *["PATCH"] * 3, "GET"] + + +@pytest.mark.parametrize( + "published", + [ + pytest.param({"body": "Other notes.\n"}, id="notes"), + pytest.param( + {"assets": [{"name": "stray.zip", "digest": "sha256:" + "0" * 64, "state": "uploaded"}]}, id="asset" + ), + ], +) +def test_failed_publish_is_rejected_when_published_release_differs( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], + published: dict[str, str | list[dict[str, str]]], +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + _expect_draft_for_publish(server, notes) + prefix = "/api/v3/repos/tox-dev/fixture" + for _ in range(3): + server.expect_ordered_request(f"{prefix}/releases/42", method="PATCH").respond_with_data(status=502) + for _ in range(6): + server.expect_ordered_request(f"{prefix}/releases/42", method="GET").respond_with_json( + _release(notes, draft=False) | published + ) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli) + assert result.returncode != 0 + assert "is not published with the expected assets and notes" in result.stderr + assert len(_requests(server, "PATCH", f"{prefix}/releases/42")) == 3 + + +@pytest.mark.parametrize( + ("status", "attempts"), + [pytest.param(502, 3, id="server-error"), pytest.param(422, 1, id="client-error")], +) +def test_failed_publish_on_draft_still_draft_fails( + repository: Path, + tmp_path: Path, + action_scripts: Path, + github_api: tuple[HTTPServer, dict[str, str]], + release_cli: dict[str, str], + status: int, + attempts: int, +) -> None: + server, environment = github_api + notes, commit = _notes_and_commit(repository, tmp_path) + _expect_draft_for_publish(server, notes) + prefix = "/api/v3/repos/tox-dev/fixture" + for _ in range(attempts): + server.expect_ordered_request(f"{prefix}/releases/42", method="PATCH").respond_with_json( + {"message": "Error"}, status=status + ) + for _ in range(6): + server.expect_ordered_request(f"{prefix}/releases/42", method="GET").respond_with_json( + _release(notes, draft=True) + ) + result = _request(repository, tmp_path, action_scripts, notes, commit, environment | release_cli) + assert result.returncode != 0 + assert "is not published with the expected assets and notes" in result.stderr + assert len(_requests(server, "PATCH", f"{prefix}/releases/42")) == attempts diff --git a/tests/release_workflow/test_release_notes_workflow.py b/tests/release_workflow/test_release_notes_workflow.py index 513300c..e100620 100644 --- a/tests/release_workflow/test_release_notes_workflow.py +++ b/tests/release_workflow/test_release_notes_workflow.py @@ -61,6 +61,10 @@ def test_release_notes_default_changelog_comes_from_towncrier_config(workflow: J } +def test_towncrier_config_is_read_only_without_changelog_input(workflow: JsonObject) -> None: + assert step(workflow, "notes", "⚙️ Read Towncrier configuration")["if"] == "inputs.changelog == ''" + + def test_release_notes_uploads_artifact_per_run_attempt(workflow: JsonObject) -> None: upload = step(workflow, "notes", "📤 Upload release notes")