Skip to content

Report all changelog errors at once and surface fetch failures in add #3472

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cotti wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Report all changelog errors at once and surface fetch failures in add #3472

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions docs/cli-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2554,6 +2554,14 @@
"summary": "Optional: When used with --prs or --report, remove square brackets and text within them from the beginning of PR titles, and also remove a colon if it follows the closing bracket (e.g., \u0022[Inference API] Title\u0022 becomes \u0022Title\u0022, \u0022[ES|QL]: Title\u0022 becomes \u0022Title\u0022, \u0022[Discover][ESQL] Title\u0022 becomes \u0022Title\u0022)",
"defaultValue": "false"
},
{
"role": "flag",
"name": "strict-fetch",
"type": "boolean",
"required": false,
"summary": "Optional: Treat a failure to fetch any PR or issue from GitHub (with --prs, --issues, or --report) as an error that exits non-zero, instead of a warning. Use in CI so a missing or unauthorized GITHUB_TOKEN fails the run rather than silently producing unfiltered changelogs with missing titles. Files are still written so they can be inspected.",
"defaultValue": "false"
},
{
"role": "flag",
"name": "subtype",
Expand Down
25 changes: 25 additions & 0 deletions docs/cli/changelog/cmd-add.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ For details and examples, go to [](/contribute/create-changelogs.md).
The valid product identifiers are listed in [products.yml](https://github.com/elastic/docs-builder/blob/main/config/products.yml).
For more information about valid product and lifecycle values, go to [Product format](#product-format-and-resolution).

: `--strict-fetch`
Treat a failure to fetch any PR or issue from GitHub (when using `--prs`, `--issues`, or `--report`) as an error that exits non-zero, instead of a warning.
Refer to [Fetch failures](#fetch-failures).

: `--use-pr-number`
Use PR numbers for filenames instead of the configured `filename` strategy.
Requires `--prs`, `--issues`, or `--report`. Mutually exclusive with `--use-issue-number`.
Expand Down Expand Up @@ -98,6 +102,27 @@ In each of these cases where validation fails, a changelog file is not created.
If the configuration file contains `rules.create` definitions and a PR or issue has a blocking label, that PR is skipped and no changelog file is created for it.
For more information, refer to [](/contribute/create-changelogs.md#rules).

## Fetch failures

`rules.create` label filtering and automatic `title`/`type` derivation both depend on fetching each PR or issue from GitHub.
When a fetch fails (for example, a missing or unauthorized `GITHUB_TOKEN`, a private or cross-repository reference, or API rate limiting), the affected entry **bypasses `rules.create` filtering** and is written with its `title` and `type` commented out.
Such entries later cause `changelog bundle` to fail with `missing required field: title`.

By default, each fetch failure is a warning and a single summary is emitted at the end of bulk creation (for example, `3 of 225 pull request(s) could not be fetched from GitHub`).
The command still exits `0` so a best-effort changelog is produced for offline or partial-access workflows.

Pass `--strict-fetch` to escalate fetch failures to an error so the command exits non-zero.
Use this in CI so a token or rate-limit problem fails the run loudly instead of silently producing unfiltered changelogs with missing titles.
The generated files are still written so you can inspect them.

```sh
docs-builder changelog add --report ./promotion-report.html --strict-fetch
```

:::{tip}
If you hit this, verify that `GITHUB_TOKEN` is set and can access every repository referenced by your PRs or promotion report, then delete the generated changelog files and re-run.
:::

## CI auto-detection

When running inside GitHub Actions, `changelog add` automatically reads the following environment variables to fill in arguments not provided on the command line:
Expand Down
69 changes: 41 additions & 28 deletions src/services/Elastic.Changelog/Bundling/BundleBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,39 +152,17 @@ private static List<BundledProduct> BuildProducts(
IReadOnlyList<MatchedChangelogFile> entries)
{
var resolvedEntries = new List<BundledEntry>();
var hasInvalidEntries = false;

foreach (var entry in entries)
{
var data = entry.Data;

// Validate required fields
if (string.IsNullOrWhiteSpace(data.Title))
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: title");
return null;
}

// Validate type is not Invalid (missing or unrecognized)
if (data.Type == ChangelogEntryType.Invalid)
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: type");
return null;
}

if (data.Products == null || data.Products.Count == 0)
if (!IsResolvedEntryValid(collector, entry))
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: products");
return null;
hasInvalidEntries = true;
continue;
}

// Validate products have required fields
if (data.Products.Any(product => string.IsNullOrWhiteSpace(product.ProductId)))
{
collector.EmitError(entry.FilePath, "Changelog file has product entry missing required field: product");
return null;
}

var bundledEntry = data.ToBundledEntry() with
var bundledEntry = entry.Data.ToBundledEntry() with
{
File = new BundledFile
{
Expand All @@ -195,7 +173,42 @@ private static List<BundledProduct> BuildProducts(
resolvedEntries.Add(bundledEntry);
}

return resolvedEntries;
// Report every invalid entry in a single pass instead of aborting on the first,
// so a release with several broken changelogs surfaces them all at once.
return hasInvalidEntries ? null : resolvedEntries;
}

private static bool IsResolvedEntryValid(IDiagnosticsCollector collector, MatchedChangelogFile entry)
{
var data = entry.Data;

if (string.IsNullOrWhiteSpace(data.Title))
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: title");
return false;
}

// Validate type is not Invalid (missing or unrecognized)
if (data.Type == ChangelogEntryType.Invalid)
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: type");
return false;
}

if (data.Products == null || data.Products.Count == 0)
{
collector.EmitError(entry.FilePath, "Changelog file is missing required field: products");
return false;
}

// Validate products have required fields
if (data.Products.Any(product => string.IsNullOrWhiteSpace(product.ProductId)))
{
collector.EmitError(entry.FilePath, "Changelog file has product entry missing required field: product");
return false;
}

return true;
}

private static List<BundledEntry> BuildFileOnlyEntries(IReadOnlyList<MatchedChangelogFile> entries) =>
Expand Down
77 changes: 68 additions & 9 deletions src/services/Elastic.Changelog/Creation/ChangelogCreationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ public record CreateChangelogArguments
/// When true, omit schema reference comments from generated YAML files.
/// </summary>
public bool Concise { get; init; }

/// <summary>
/// When true, a failure to fetch any PR or issue from GitHub during bulk creation is treated as
/// an error (non-zero exit) instead of a warning. Use in CI to fail fast when a missing or
/// unauthorized GITHUB_TOKEN would otherwise silently produce unfiltered, title-less changelogs.
/// </summary>
public bool StrictFetch { get; init; }
}

/// <summary>
Expand Down Expand Up @@ -206,11 +213,12 @@ private async Task<bool> CreateChangelogsForMultiplePrsAsync(

var successCount = 0;
var skippedCount = 0;
var fetchFailedCount = 0;

foreach (var prTrimmed in input.Prs.Select(pr => pr.Trim()).Where(prTrimmed => !string.IsNullOrWhiteSpace(prTrimmed)))
{
// Check PR for blockers
var (shouldSkip, _) = await _prProcessor.CheckPrForBlockersAsync(
var (shouldSkip, prInfo) = await _prProcessor.CheckPrForBlockersAsync(
collector, prTrimmed, input.Owner, input.Repo, input.Products, config, ctx);

if (shouldSkip)
Expand All @@ -219,27 +227,36 @@ private async Task<bool> CreateChangelogsForMultiplePrsAsync(
continue;
}

// A null prInfo here means the GitHub fetch failed: this PR bypasses rules.create
// label filtering and is written without a derived title/type. Track it so the failure
// is surfaced as a single summary rather than buried among per-PR warnings.
if (prInfo == null)
fetchFailedCount++;

// Create a copy of input for this PR
var prInput = CreateInputForSinglePr(input, prTrimmed);

// Process this PR (treat as single PR)
var result = await CreateSingleChangelogAsync(collector, prInput, config, ctx);
// Process this PR (treat as single PR); the loop owns fetch-failure reporting
var result = await CreateSingleChangelogAsync(collector, prInput, config, ctx, reportFetchFailure: false);
if (result)
successCount++;
}

ReportBulkFetchFailures(collector, fetchFailedCount, input.Prs.Length, input.StrictFetch, "pull request");

if (successCount == 0 && skippedCount == 0)
return false;

_logger.LogInformation("Processed {SuccessCount} PR(s) successfully, skipped {SkippedCount} PR(s)", successCount, skippedCount);
_logger.LogInformation("Processed {SuccessCount} PR(s) successfully, skipped {SkippedCount} PR(s), {FetchFailedCount} PR(s) could not be fetched", successCount, skippedCount, fetchFailedCount);
return successCount > 0;
}

private async Task<bool> CreateSingleChangelogAsync(
IDiagnosticsCollector collector,
CreateChangelogArguments input,
ChangelogConfiguration config,
Cancel ctx)
Cancel ctx,
bool reportFetchFailure = true)
{
// Get the PR URL if Prs is provided (for single PR processing)
var prUrl = input.Prs is { Length: > 0 } ? input.Prs[0] : null;
Expand All @@ -262,6 +279,9 @@ private async Task<bool> CreateSingleChangelogAsync(

prFetchFailed = prResult.FetchFailed;

if (reportFetchFailure && prFetchFailed && input.StrictFetch)
EmitStrictFetchError(collector, "pull request", prUrl);

if (prResult.DerivedFields != null)
input = ApplyDerivedFields(input, prResult.DerivedFields);
else if (!prFetchFailed)
Expand Down Expand Up @@ -310,10 +330,11 @@ private async Task<bool> CreateChangelogsForMultipleIssuesAsync(

var successCount = 0;
var skippedCount = 0;
var fetchFailedCount = 0;

foreach (var issueUrl in input.Issues.Select(i => i.Trim()).Where(i => !string.IsNullOrWhiteSpace(i)))
{
var (shouldSkip, _) = await _issueProcessor.CheckIssueForBlockersAsync(
var (shouldSkip, issueInfo) = await _issueProcessor.CheckIssueForBlockersAsync(
collector, issueUrl, input.Owner, input.Repo, input.Products, config, ctx);

if (shouldSkip)
Expand All @@ -322,24 +343,32 @@ private async Task<bool> CreateChangelogsForMultipleIssuesAsync(
continue;
}

// A null issueInfo means the GitHub fetch failed: the entry bypasses rules.create
// filtering and is written without a derived title/type. Track it for a summary report.
if (issueInfo == null)
fetchFailedCount++;

var issueInput = input with { Issues = [issueUrl] };
var result = await CreateSingleChangelogFromIssueAsync(collector, issueInput, config, ctx);
var result = await CreateSingleChangelogFromIssueAsync(collector, issueInput, config, ctx, reportFetchFailure: false);
if (result)
successCount++;
}

ReportBulkFetchFailures(collector, fetchFailedCount, input.Issues.Length, input.StrictFetch, "issue");

if (successCount == 0 && skippedCount == 0)
return false;

_logger.LogInformation("Processed {SuccessCount} issue(s) successfully, skipped {SkippedCount} issue(s)", successCount, skippedCount);
_logger.LogInformation("Processed {SuccessCount} issue(s) successfully, skipped {SkippedCount} issue(s), {FetchFailedCount} issue(s) could not be fetched", successCount, skippedCount, fetchFailedCount);
return successCount > 0;
}

private async Task<bool> CreateSingleChangelogFromIssueAsync(
IDiagnosticsCollector collector,
CreateChangelogArguments input,
ChangelogConfiguration config,
Cancel ctx)
Cancel ctx,
bool reportFetchFailure = true)
{
var issueUrl = input.Issues is { Length: > 0 } ? input.Issues[0] : null;

Expand All @@ -351,6 +380,9 @@ private async Task<bool> CreateSingleChangelogFromIssueAsync(
if (issueResult.ShouldSkip)
return true;

if (reportFetchFailure && issueResult.FetchFailed && input.StrictFetch)
EmitStrictFetchError(collector, "issue", issueUrl);

if (issueResult.DerivedFields != null)
input = ApplyDerivedFields(input, issueResult.DerivedFields);
else if (!issueResult.FetchFailed)
Expand Down Expand Up @@ -379,6 +411,33 @@ private async Task<bool> CreateSingleChangelogFromIssueAsync(
ctx);
}

/// <summary>
/// Emits a single aggregate diagnostic when one or more items could not be fetched from GitHub during
/// bulk creation. These items bypass <c>rules.create</c> label filtering and are written without a
/// derived title/type, so the failure is escalated to an error under strict mode (non-zero exit).
/// </summary>
private static void ReportBulkFetchFailures(IDiagnosticsCollector collector, int fetchFailedCount, int total, bool strict, string itemKind)
{
if (fetchFailedCount <= 0)
return;

var message =
$"{fetchFailedCount} of {total} {itemKind}(s) could not be fetched from GitHub. " +
$"Their changelogs were created without rules.create label filtering and may be missing title or type, " +
$"which will cause 'changelog bundle' to fail. Verify GITHUB_TOKEN is set and can access the referenced " +
$"repositories, then delete the generated changelog files and re-run.";

if (strict)
collector.EmitError(string.Empty, message);
else
collector.EmitWarning(string.Empty, message);
}

private static void EmitStrictFetchError(IDiagnosticsCollector collector, string itemKind, string? url) =>
collector.EmitError(string.Empty,
$"Could not fetch {itemKind} '{url}' from GitHub and --strict-fetch is set. " +
"Verify GITHUB_TOKEN is set and can access the repository, then re-run.");

private static CreateChangelogArguments CreateInputForSinglePr(CreateChangelogArguments input, string prUrl) =>
input with { Prs = [prUrl] };

Expand Down
23 changes: 10 additions & 13 deletions src/services/Elastic.Changelog/Rendering/BundleValidationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ private async Task<bool> ValidateBundleEntriesAsync(
Cancel ctx)
{
var fileNamesInThisBundle = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
var allValid = true;

foreach (var entry in bundledData.Entries)
{
Expand All @@ -174,21 +175,17 @@ private async Task<bool> ValidateBundleEntriesAsync(
bundleList.Add(bundleInput.BundleFile);
}

// If entry has resolved data, validate it
if (!string.IsNullOrWhiteSpace(entry.Title) && entry.Type != null)
{
if (!ValidateResolvedEntry(collector, bundleInput.BundleFile, entry, seenPrs))
return false;
}
else
{
// Entry only has file reference - validate file exists
if (!await ValidateFileReferenceEntryAsync(collector, bundleInput.BundleFile, entry, bundleDirectory, seenPrs, ctx))
return false;
}
// If entry has resolved data, validate it; otherwise validate the file reference.
// Continue past invalid entries so every problem in the bundle is reported in one pass.
var entryValid = !string.IsNullOrWhiteSpace(entry.Title) && entry.Type != null
? ValidateResolvedEntry(collector, bundleInput.BundleFile, entry, seenPrs)
: await ValidateFileReferenceEntryAsync(collector, bundleInput.BundleFile, entry, bundleDirectory, seenPrs, ctx);

if (!entryValid)
allValid = false;
}

return true;
return allValid;
}

private static bool ValidateResolvedEntry(
Expand Down
5 changes: 4 additions & 1 deletion src/tooling/docs-builder/Commands/ChangelogCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ public Task<int> Init(
/// <param name="report">Optional: URL or file path to a promotion report HTML document. Extracts GitHub pull request URLs and creates one changelog per PR (same parsing as `changelog bundle --report`). Mutually exclusive with --prs, --issues, and --release-version.</param>
/// <param name="repo">Optional: GitHub repository name (used when --prs or --issues contains just numbers, or when using --release-version). Falls back to bundle.repo in changelog.yml when not specified.</param>
/// <param name="stripTitlePrefix">Optional: When used with --prs or --report, remove square brackets and text within them from the beginning of PR titles, and also remove a colon if it follows the closing bracket (e.g., "[Inference API] Title" becomes "Title", "[ES|QL]: Title" becomes "Title", "[Discover][ESQL] Title" becomes "Title")</param>
/// <param name="strictFetch">Optional: Treat a failure to fetch any PR or issue from GitHub (with --prs, --issues, or --report) as an error that exits non-zero, instead of a warning. Use in CI so a missing or unauthorized GITHUB_TOKEN fails the run rather than silently producing unfiltered changelogs with missing titles. Files are still written so they can be inspected.</param>
/// <param name="subtype">Optional: Subtype for breaking changes (api, behavioral, configuration, etc.)</param>
/// <param name="title">Optional: A short, user-facing title (max 80 characters). Required if neither --prs, --issues, nor --report is specified. If --prs and --title are specified, the latter value is used instead of what exists in the PR.</param>
/// <param name="type">Optional: Type of change (feature, enhancement, bug-fix, breaking-change, etc.). Required if neither --prs, --issues, nor --report is specified. If mappings are configured, type can be derived from the PR or issue.</param>
Expand Down Expand Up @@ -261,6 +262,7 @@ public async Task<int> Add(
string? releaseVersion = null,
string? repo = null,
bool stripTitlePrefix = false,
bool strictFetch = false,
string? subtype = null,
string? title = null,
string? type = null,
Expand Down Expand Up @@ -527,7 +529,8 @@ async static (s, collector, state, ctx) => await s.CreateChangelogsFromRelease(c
StripTitlePrefix = stripTitlePrefixResolved,
ExtractReleaseNotes = extractReleaseNotes,
ExtractIssues = extractIssues,
Concise = concise
Concise = concise,
StrictFetch = strictFetch
};

serviceInvoker.AddCommand(service, input,
Expand Down
Loading
Loading