From a19f741afc4c15f7dde724469c64c219bfdb74bf Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 15:16:04 -0600 Subject: [PATCH 1/2] fix(github): classify access, network and server failures; cancel a listing promptly The listers translated a rejected token, rate limits and "not found". Everything else escaped as a raw exception: a refusal of access (403, an organization that enforces SAML single sign-on, for example), an error on GitHub's side (5xx), a network failure, and a request that timed out. The CLI reported those as exit code 70, a bug in gclo, and the desktop app took a timed-out organization lookup for a superseded one and showed nothing. - GitHubFailures is the one mapping both listers use. 403 (and 451) is Forbidden, with GitHub's own reason in the message; 5xx, a network failure and a timeout are the new GitHubAccessKind.Unavailable. A failure it does not know stays what it is. - The CLI maps Unavailable to exit code 4 (retry later). For these cases the exit code changes from 70 to 3 or 4. - Each gateway request is awaited with the caller's token, so Ctrl+C during a request that hangs no longer waits for the HTTP timeout. A cancellation the caller did not ask for is a timeout. - OrgSyncEngine reports through RunProgress, which drops whatever arrives for a repository after its final state in the run. A git operation abandoned on cancel keeps reporting transfer progress; a row that said Canceled could go back to "Cloning 40%", and the CLI could print a progress line after the summary. README.md listed a rejected token under exit code 2; its table now has 3, 4 and 70 as docs/CLI.md does. Closes #153 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- README.md | 5 +- docs/CLI.md | 4 +- gclo.Cli.Tests/OrgsCommandTests.cs | 8 + gclo.Cli.Tests/ReposCommandTests.cs | 4 + gclo.Cli.Tests/SyncCommandTests.cs | 10 + gclo.Cli/ExitCodes.cs | 7 +- gclo.Engine.Tests/GitHubListerTests.cs | 279 ++++++++++++++++++++++++ gclo.Engine.Tests/OrgSyncEngineTests.cs | 48 ++++ gclo.Engine.Tests/RunProgressTests.cs | 104 +++++++++ gclo.Engine/GitHubAccessException.cs | 2 +- gclo.Engine/GitHubAccessKind.cs | 6 + gclo.Engine/GitHubFailures.cs | 114 ++++++++++ gclo.Engine/GitHubOrganizationLister.cs | 36 +-- gclo.Engine/GitHubRepositoryLister.cs | 38 +--- gclo.Engine/OrgSyncEngine.cs | 3 + gclo.Engine/RunProgress.cs | 54 +++++ 16 files changed, 661 insertions(+), 61 deletions(-) create mode 100644 gclo.Engine.Tests/RunProgressTests.cs create mode 100644 gclo.Engine/GitHubFailures.cs create mode 100644 gclo.Engine/RunProgress.cs diff --git a/README.md b/README.md index 214bb4b..154feb3 100644 --- a/README.md +++ b/README.md @@ -180,7 +180,10 @@ gclo sync --org contoso --target ~/src/contoso --json | | --- | --- | | 0 | Everything succeeded. | | 1 | The run completed, but some repositories failed or the run was canceled (Ctrl+C). | -| 2 | Fatal: bad arguments, missing/empty/rejected token, or organization not found. | +| 2 | Fatal: bad arguments, missing or empty token, or organization not found. | +| 3 | GitHub rejected the token or refused it access: fix the token, do not retry as is. | +| 4 | A rate limit, a network failure, a timeout, or an error on GitHub's side: retry later. | +| 70 | An unexpected error inside gclo; details are in the activity log. | Full reference — all options, output streams, Ctrl+C behavior, and more examples: [docs/CLI.md](docs/CLI.md). diff --git a/docs/CLI.md b/docs/CLI.md index 1648af5..8424bd4 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -403,8 +403,8 @@ its offset from UTC (`2026-11-10 14:03:22.123 -06:00 [INFO] ...`). Text that cam | 0 | Everything succeeded (or `--dry-run` / a listing completed). | — | | 1 | The run completed, but some repositories failed or the run was canceled (Ctrl+C). `--json`'s `wasCanceled` tells the two apart. | Inspect the failures; retry them. | | 2 | Fatal: bad arguments, missing/empty token, organization not found, unknown account, missing account token, unusable target folder, or an account command on a non-Windows OS. | Fix the invocation. | -| 3 | GitHub rejected the token (401) or the token may not see the organization (403). | Fix the token or its access; do not retry as is. | -| 4 | GitHub's primary or secondary rate limit, or a temporary throttle. | Retry later (the message says when). | +| 3 | GitHub rejected the token (401), or refused it access (403: the token may not see the organization, or the organization requires SAML single sign-on and the token is not authorized for it). The message carries GitHub's reason. | Fix the token or its access; do not retry as is. | +| 4 | Not your doing: GitHub's primary or secondary rate limit, a temporary throttle, a network failure, a request that timed out, or an error on GitHub's side (HTTP 5xx). | Retry later (for a rate limit, the message says when). | | 70 | An unexpected error inside gclo. The exception type is printed and the full details are in the activity log. | Please report it. | Codes 3 and 4 were part of code 2 before version 1.1; a script that tested diff --git a/gclo.Cli.Tests/OrgsCommandTests.cs b/gclo.Cli.Tests/OrgsCommandTests.cs index 536db20..e9047ba 100644 --- a/gclo.Cli.Tests/OrgsCommandTests.cs +++ b/gclo.Cli.Tests/OrgsCommandTests.cs @@ -144,6 +144,14 @@ public sealed class OrgsCommandTests var throttled = new FakeOrgLister { Throw = new GitHubAccessException(GitHubAccessKind.RateLimited, "slow down") }; ex = await Assert.ThrowsAsync(() => Run(throttled, "--token-env", EnvVar)); Assert.Equal(ExitCodes.Transient, ex.ExitCode); + + var unavailable = new FakeOrgLister { Throw = new GitHubAccessException(GitHubAccessKind.Unavailable, "no answer") }; + ex = await Assert.ThrowsAsync(() => Run(unavailable, "--token-env", EnvVar)); + Assert.Equal(ExitCodes.Transient, ex.ExitCode); + + var refused = new FakeOrgLister { Throw = new GitHubAccessException(GitHubAccessKind.Forbidden, "refused") }; + ex = await Assert.ThrowsAsync(() => Run(refused, "--token-env", EnvVar)); + Assert.Equal(ExitCodes.Auth, ex.ExitCode); } diff --git a/gclo.Cli.Tests/ReposCommandTests.cs b/gclo.Cli.Tests/ReposCommandTests.cs index f7efd12..083705a 100644 --- a/gclo.Cli.Tests/ReposCommandTests.cs +++ b/gclo.Cli.Tests/ReposCommandTests.cs @@ -158,6 +158,10 @@ public sealed class ReposCommandTests Assert.Equal(ExitCodes.Auth, ex.ExitCode); Assert.Contains("no access", ex.Message); + + var unavailable = new FakeRepoLister { Throw = new GitHubAccessException(GitHubAccessKind.Unavailable, "no answer") }; + ex = await Assert.ThrowsAsync(() => Run(unavailable, "--org", "acme", "--token-env", EnvVar)); + Assert.Equal(ExitCodes.Transient, ex.ExitCode); } diff --git a/gclo.Cli.Tests/SyncCommandTests.cs b/gclo.Cli.Tests/SyncCommandTests.cs index a1102f4..695230f 100644 --- a/gclo.Cli.Tests/SyncCommandTests.cs +++ b/gclo.Cli.Tests/SyncCommandTests.cs @@ -335,6 +335,16 @@ public sealed class SyncCommandTests : IDisposable ex = await Assert.ThrowsAsync( () => Run(missing, new FakeGit(), "--org", "ghost", "--target", Target, "--token-env", EnvVar)); Assert.Equal(ExitCodes.Fatal, ex.ExitCode); + + // Not the caller's doing (network, timeout, an error on GitHub's side): retry later. + var unavailable = new FakeRepoLister + { + Throw = new GitHubAccessException(GitHubAccessKind.Unavailable, "GitHub did not answer in time; retry later."), + }; + ex = await Assert.ThrowsAsync( + () => Run(unavailable, new FakeGit(), "--org", "acme", "--target", Target, "--token-env", EnvVar)); + Assert.Equal(ExitCodes.Transient, ex.ExitCode); + Assert.Equal("GitHub did not answer in time; retry later.", ex.Message); } diff --git a/gclo.Cli/ExitCodes.cs b/gclo.Cli/ExitCodes.cs index 138343f..257e01a 100644 --- a/gclo.Cli/ExitCodes.cs +++ b/gclo.Cli/ExitCodes.cs @@ -29,7 +29,10 @@ internal static class ExitCodes /// GitHub rejected the token or refused access (401/403): fix the token; never retry as is. public const int Auth = 3; - /// A rate limit or temporary throttle: retry later. + /// + /// Not the caller's doing: a rate limit or temporary throttle, a network failure, + /// a timeout, or an error on GitHub's side. Retry later. + /// public const int Transient = 4; /// An unexpected exception inside gclo; details are in the activity log. @@ -41,7 +44,7 @@ internal static class ExitCodes public static int ForAccess(GitHubAccessKind kind) => kind switch { GitHubAccessKind.Unauthorized or GitHubAccessKind.Forbidden => Auth, - GitHubAccessKind.RateLimited => Transient, + GitHubAccessKind.RateLimited or GitHubAccessKind.Unavailable => Transient, _ => Fatal, }; } diff --git a/gclo.Engine.Tests/GitHubListerTests.cs b/gclo.Engine.Tests/GitHubListerTests.cs index cd9d5fe..32e4031 100644 --- a/gclo.Engine.Tests/GitHubListerTests.cs +++ b/gclo.Engine.Tests/GitHubListerTests.cs @@ -504,4 +504,283 @@ public sealed class GitHubListerTests Assert.NotNull(new GitHubRepositoryLister()); Assert.NotNull(new GitHubOrganizationLister()); } + + + + // ---------------------------------------------------------------- failures that are not the request's fault (#153) + + private static ApiException ServerError() => new("Bad Gateway", HttpStatusCode.BadGateway); + + + + private static HttpRequestException NetworkFailure() => new("No such host is known. (api.github.com:443)"); + + + + /// What an HTTP client throws when a request runs into its timeout: a cancellation nobody asked for. + private static TaskCanceledException TimedOut() => new("The request was canceled due to the configured HttpClient.Timeout."); + + + + [Fact] + public async Task Repos_AccessRefused_IsForbidden_WithGitHubsReason() + { + _gateway.OrgPages = (_, _) => throw Forbidden(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Forbidden, ex.Kind); + Assert.StartsWith("GitHub refused access (403): ", ex.Message); + Assert.Contains("SAML single sign-on", ex.Message); + Assert.IsType(ex.InnerException); + } + + + + [Fact] + public async Task Repos_AccessRefused_WhileResolvingAUserAccount_IsForbiddenToo() + { + // The refusal can come from any of the requests, not only the first one. + _gateway.OrgPages = (_, _) => throw NotFound(); + _gateway.AccountKind = _ => throw Forbidden(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Forbidden, ex.Kind); + } + + + + [Fact] + public async Task Repos_ErrorOnGitHubsSide_IsUnavailable() + { + _gateway.OrgPages = (_, _) => throw ServerError(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Unavailable, ex.Kind); + Assert.Contains("HTTP 502", ex.Message); + Assert.Contains("retry later", ex.Message); + } + + + + [Fact] + public async Task Repos_NetworkFailure_IsUnavailable_WithTheReason() + { + _gateway.OrgPages = (_, _) => throw NetworkFailure(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Unavailable, ex.Kind); + Assert.Equal( + "GitHub could not be reached: No such host is known. (api.github.com:443). Check the network connection and retry.", + ex.Message); + } + + + + [Fact] + public async Task Repos_RequestTimedOut_IsUnavailable_NotACancellation() + { + _gateway.OrgPages = (_, _) => throw TimedOut(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Unavailable, ex.Kind); + Assert.Contains("did not answer in time", ex.Message); + } + + + + [Fact] + public async Task Repos_AFailureWithNoKnownMeaning_StaysWhatItIs() + { + _gateway.OrgPages = (_, _) => throw new ApiException("Validation Failed", HttpStatusCode.UnprocessableEntity); + + ApiException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(HttpStatusCode.UnprocessableEntity, ex.StatusCode); + } + + + + [Fact] + public async Task Repos_CancellationDuringARequestThatHangs_DoesNotWaitForIt() + { + using var cts = new CancellationTokenSource(); + var requested = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var never = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + _gateway.OrgPages = (_, _) => + { + requested.SetResult(); + return never.Task; + }; + + Task> listing = RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890", cts.Token); + await requested.Task.WaitAsync(TimeSpan.FromSeconds(30)); + cts.Cancel(); + + OperationCanceledException ex = await Assert.ThrowsAnyAsync( + () => listing.WaitAsync(TimeSpan.FromSeconds(30))); + Assert.Equal(cts.Token, ex.CancellationToken); // the caller's cancellation, not a timeout + } + + + + [Fact] + public async Task Repos_CancellationWhileTheAccountKindIsLookedUp_DoesNotWaitForIt() + { + using var cts = new CancellationTokenSource(); + var requested = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var never = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + _gateway.OrgPages = (_, _) => throw NotFound(); + _gateway.AccountKind = _ => + { + requested.SetResult(); + return never.Task; + }; + + Task> listing = RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890", cts.Token); + await requested.Task.WaitAsync(TimeSpan.FromSeconds(30)); + cts.Cancel(); + + await Assert.ThrowsAnyAsync(() => listing.WaitAsync(TimeSpan.FromSeconds(30))); + } + + + + [Fact] + public async Task Orgs_AccessRefusedForTheAccountItself_IsForbidden() + { + // Only a refusal of the organization list is the missing-scope case; this one is not. + _gateway.CurrentUser = () => throw Forbidden(); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => OrgLister().ListOrganizationsAsync("tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Forbidden, ex.Kind); + } + + + + [Fact] + public async Task Orgs_ErrorOnGitHubsSide_NetworkFailure_AndTimeout_AreUnavailable() + { + _gateway.OrgLogins = () => throw ServerError(); + GitHubAccessException server = await Assert.ThrowsAsync( + () => OrgLister().ListOrganizationsAsync("tok-1234567890")); + + _gateway.OrgLogins = () => throw NetworkFailure(); + GitHubAccessException network = await Assert.ThrowsAsync( + () => OrgLister().ListOrganizationsAsync("tok-1234567890")); + + _gateway.OrgLogins = () => throw TimedOut(); + GitHubAccessException timeout = await Assert.ThrowsAsync( + () => OrgLister().ListOrganizationsAsync("tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Unavailable, server.Kind); + Assert.Equal(GitHubAccessKind.Unavailable, network.Kind); + Assert.Equal(GitHubAccessKind.Unavailable, timeout.Kind); + } + + + + [Fact] + public async Task Orgs_CancellationDuringARequestThatHangs_DoesNotWaitForIt() + { + using var cts = new CancellationTokenSource(); + var requested = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var never = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + _gateway.OrgLogins = () => + { + requested.SetResult(); + return never.Task; + }; + + Task> listing = OrgLister().ListOrganizationsAsync("tok-1234567890", cts.Token); + await requested.Task.WaitAsync(TimeSpan.FromSeconds(30)); + cts.Cancel(); + + await Assert.ThrowsAnyAsync(() => listing.WaitAsync(TimeSpan.FromSeconds(30))); + } + + + + // ---------------------------------------------------------------- the shared mapping itself (#153) + + [Fact] + public void Failures_EveryKnownFailure_HasItsKind() + { + (Exception Failure, GitHubAccessKind Kind)[] known = + [ + (Unauthorized(), GitHubAccessKind.Unauthorized), + (RateLimited(), GitHubAccessKind.RateLimited), + (SecondaryRateLimited(), GitHubAccessKind.RateLimited), + (Abuse(), GitHubAccessKind.RateLimited), + (LoginAttemptsExceeded(), GitHubAccessKind.RateLimited), + (Forbidden(), GitHubAccessKind.Forbidden), + (new LegalRestrictionException("Repository access blocked", (HttpStatusCode)451), GitHubAccessKind.Forbidden), + (ServerError(), GitHubAccessKind.Unavailable), + (new ApiException("Service Unavailable", HttpStatusCode.ServiceUnavailable), GitHubAccessKind.Unavailable), + (NetworkFailure(), GitHubAccessKind.Unavailable), + (TimedOut(), GitHubAccessKind.Unavailable), + ]; + + Assert.All(known, k => + { + Assert.True(GitHubFailures.Knows(k.Failure, CancellationToken.None), k.Failure.GetType().Name); + GitHubAccessException translated = GitHubFailures.Translate(k.Failure, CancellationToken.None); + Assert.Equal(k.Kind, translated.Kind); + Assert.Same(k.Failure, translated.InnerException); + }); + } + + + + [Fact] + public void Failures_ALegalRestriction_SaysItsStatusAndReason() + { + GitHubAccessException translated = GitHubFailures.Translate( + new LegalRestrictionException("Repository access blocked.", (HttpStatusCode)451), CancellationToken.None); + + Assert.StartsWith("GitHub refused access (451): Repository access blocked. If the organization", translated.Message); + } + + + + [Fact] + public void Failures_AnEmptyReason_IsSaidAsSuch() + { + GitHubAccessException translated = GitHubFailures.Translate(new HttpRequestException(" . "), CancellationToken.None); + + Assert.StartsWith("GitHub could not be reached: no reason was given. ", translated.Message); + } + + + + [Fact] + public void Failures_WhatHasNoKnownMeaning_IsNotTranslated() + { + using var canceled = new CancellationTokenSource(); + canceled.Cancel(); + Exception[] unknown = + [ + NotFound(), // each lister decides what "not found" means where it happens + new ApiException("Validation Failed", HttpStatusCode.UnprocessableEntity), + new InvalidOperationException("a bug"), + new GitHubAccessException(GitHubAccessKind.Forbidden, "already translated"), + ]; + + Assert.All(unknown, failure => Assert.False(GitHubFailures.Knows(failure, CancellationToken.None), failure.GetType().Name)); + // A cancellation the caller asked for is a cancellation, not a timeout. + Assert.False(GitHubFailures.Knows(new OperationCanceledException(canceled.Token), canceled.Token)); + Assert.Throws(() => GitHubFailures.Translate(new InvalidOperationException("a bug"), CancellationToken.None)); + } } diff --git a/gclo.Engine.Tests/OrgSyncEngineTests.cs b/gclo.Engine.Tests/OrgSyncEngineTests.cs index 0f37cce..0c93164 100644 --- a/gclo.Engine.Tests/OrgSyncEngineTests.cs +++ b/gclo.Engine.Tests/OrgSyncEngineTests.cs @@ -593,5 +593,53 @@ public sealed class OrgSyncEngineTests : IDisposable Assert.True(cts.IsCancellationRequested); Assert.Equal(new SyncSummary(Total: 1, Cloned: 1, Updated: 0, Failed: 0, Canceled: 0, WasCanceled: false), summary); +||||||| parent of e2289fd (fix(github): classify access, network and server failures; cancel a listing promptly) + + + + // ---------------------------------------------------------------- nothing after a final state (#153) + + [Fact] + public async Task Sync_ProgressFromAnAbandonedClone_IsNotReportedAfterTheRepositoryWasCanceled() + { + _lister.Repositories = Repos("stuck"); + var neverDone = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + Action? transfer = null; + _git.CloneHandler = (_, _, _, onProgress, _) => + { + transfer = onProgress; + return neverDone.Task; // ignores cancellation, as a wedged transport does + }; + using var cts = new CancellationTokenSource(); + Task syncTask = CreateEngine().SyncAsync(CreateRequest(), _progress, cts.Token); + await WaitUntilAsync(() => transfer is not null, "the clone to start"); + transfer!(0.25); + + cts.Cancel(); + await syncTask.WaitAsync(TimeSpan.FromSeconds(10)); + transfer(0.5); // the abandoned operation is still running + neverDone.SetResult(); + + Assert.Equal( + [SyncStatus.Queued, SyncStatus.Cloning, SyncStatus.Cloning, SyncStatus.Canceled], + _progress.StatusesFor("stuck")); + Assert.Equal(0.25, _progress.Reports.Last(r => r.Status == SyncStatus.Cloning).Percent); + } + + + + [Fact] + public async Task Sync_ASecondRun_ReportsTheSameRepositoryAgain() + { + // The rule is per run: what a repository reported last time holds nothing back now. + _lister.Repositories = Repos("solo"); + OrgSyncEngine engine = CreateEngine(); + + await engine.SyncAsync(CreateRequest(), _progress); + await engine.SyncAsync(CreateRequest(), _progress); + + Assert.Equal( + [SyncStatus.Queued, SyncStatus.Cloning, SyncStatus.Done, SyncStatus.Queued, SyncStatus.Cloning, SyncStatus.Done], + _progress.StatusesFor("solo")); } } diff --git a/gclo.Engine.Tests/RunProgressTests.cs b/gclo.Engine.Tests/RunProgressTests.cs new file mode 100644 index 0000000..5379deb --- /dev/null +++ b/gclo.Engine.Tests/RunProgressTests.cs @@ -0,0 +1,104 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +namespace gclo.Engine.Tests; + + +/// +/// passes every report on, except what arrives for a +/// repository after its final state in the run (#153). +/// +public sealed class RunProgressTests +{ + private readonly RecordingProgress _seen = new(); + + + + private RunProgress NewRun() => new(_seen); + + + + [Fact] + public void Report_BeforeTheFinalState_IsPassedOn_InOrder() + { + RunProgress run = NewRun(); + + run.Report(new RepoProgress("alpha", SyncStatus.Queued)); + run.Report(new RepoProgress("alpha", SyncStatus.Cloning)); + run.Report(new RepoProgress("alpha", SyncStatus.Cloning, Percent: 0.5)); + run.Report(new RepoProgress("alpha", SyncStatus.Done)); + + Assert.Equal([SyncStatus.Queued, SyncStatus.Cloning, SyncStatus.Cloning, SyncStatus.Done], _seen.StatusesFor("alpha")); + Assert.Equal(0.5, _seen.Reports[2].Percent); + } + + + + [Theory] + [InlineData(SyncStatus.Done)] + [InlineData(SyncStatus.Failed)] + [InlineData(SyncStatus.Canceled)] + public void Report_AfterAFinalState_IsDropped_WhateverItSays(SyncStatus final) + { + RunProgress run = NewRun(); + run.Report(new RepoProgress("alpha", final)); + + run.Report(new RepoProgress("alpha", SyncStatus.Cloning, Percent: 0.9)); + run.Report(new RepoProgress("alpha", SyncStatus.Pulling)); + run.Report(new RepoProgress("alpha", SyncStatus.Queued)); + run.Report(new RepoProgress("alpha", SyncStatus.Done)); + run.Report(new RepoProgress("alpha", SyncStatus.Failed, "late")); + run.Report(new RepoProgress("ALPHA", SyncStatus.Cloning)); // one repository, whatever its case + + Assert.Equal([final], _seen.StatusesFor("alpha")); + Assert.Empty(_seen.StatusesFor("ALPHA")); + } + + + + [Fact] + public void Report_ForAnotherRepository_IsNotHeldBack() + { + RunProgress run = NewRun(); + run.Report(new RepoProgress("alpha", SyncStatus.Canceled)); + + run.Report(new RepoProgress("bravo", SyncStatus.Cloning)); + run.Report(new RepoProgress("bravo", SyncStatus.Done)); + + Assert.Equal([SyncStatus.Cloning, SyncStatus.Done], _seen.StatusesFor("bravo")); + } + + + + [Fact] + public void Report_FromManyThreads_NeverPutsProgressAfterTheFinalState() + { + for(int round = 0; round < 50; round++) + { + var seen = new RecordingProgress(); + var run = new RunProgress(seen); + + Parallel.Invoke( + () => + { + for(int i = 0; i < 200; i++) + { + run.Report(new RepoProgress("alpha", SyncStatus.Cloning, Percent: i / 200.0)); + } + }, + () => run.Report(new RepoProgress("alpha", SyncStatus.Canceled))); + + IReadOnlyList statuses = seen.StatusesFor("alpha"); + Assert.Equal(SyncStatus.Canceled, statuses[^1]); + Assert.Single(statuses, s => s == SyncStatus.Canceled); + } + } + + + + [Fact] + public void Constructor_RefusesAMissingProgress() + => Assert.Throws(() => new RunProgress(null!)); +} diff --git a/gclo.Engine/GitHubAccessException.cs b/gclo.Engine/GitHubAccessException.cs index 2c97506..8119339 100644 --- a/gclo.Engine/GitHubAccessException.cs +++ b/gclo.Engine/GitHubAccessException.cs @@ -7,7 +7,7 @@ namespace gclo.Engine; /// -/// A GitHub API refusal translated into an actionable message. Derives from +/// A failed GitHub API request (a refusal, or GitHub not answering) translated into an actionable message. Derives from /// so every existing catch keeps working; /// lets a caller tell "fix your token" from "wait and retry" /// without parsing the message — the CLI maps it to distinct exit codes. diff --git a/gclo.Engine/GitHubAccessKind.cs b/gclo.Engine/GitHubAccessKind.cs index 9100fe2..c3db52a 100644 --- a/gclo.Engine/GitHubAccessKind.cs +++ b/gclo.Engine/GitHubAccessKind.cs @@ -20,4 +20,10 @@ public enum GitHubAccessKind /// No organization or user account with that login is visible to the token. NotFound, + + /// + /// GitHub could not be reached, did not answer in time, or failed on its side + /// (a network error, a timeout, HTTP 5xx): nothing is wrong with the request, retry later. + /// + Unavailable, } diff --git a/gclo.Engine/GitHubFailures.cs b/gclo.Engine/GitHubFailures.cs new file mode 100644 index 0000000..76d83f6 --- /dev/null +++ b/gclo.Engine/GitHubFailures.cs @@ -0,0 +1,114 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +using System.Net; +using Octokit; + + +namespace gclo.Engine; + + +/// +/// The one place that says what a failed GitHub request means to a caller: which +/// failures are the token's (fix it), which are access (grant it), and which are +/// not the caller's at all (the network, a timeout, GitHub itself: retry later). +/// Both listers go through it, so they cannot drift apart, and a failure it does +/// not know stays what it is: an unexpected error. +/// +internal static class GitHubFailures +{ + /// + /// True when is a failure with a known meaning, which + /// then puts into words. + /// + /// What a gateway call threw. + /// + /// The caller's token: a canceled request is a timeout only when the caller did + /// not cancel it. + /// + public static bool Knows(Exception exception, CancellationToken cancellationToken) + => Find(exception, cancellationToken) is not null; + + + + /// + /// The that says what + /// means to a caller. Only for a failure that accepted. + /// + /// The failure has no known meaning. + public static GitHubAccessException Translate(Exception exception, CancellationToken cancellationToken) + => Find(exception, cancellationToken) + ?? throw new ArgumentException("This failure has no known meaning; ask Knows first.", nameof(exception)); + + + + private static GitHubAccessException? Find(Exception exception, CancellationToken cancellationToken) + { + /////////////////////////////////////////////////////////////////////////////// + // Order matters: the throttling exceptions derive from ForbiddenException, // + // and every one of these but the last two derives from ApiException. // + /////////////////////////////////////////////////////////////////////////////// + return exception switch + { + AuthorizationException => new GitHubAccessException( + GitHubAccessKind.Unauthorized, + "GitHub rejected the token (401). Check that the PAT is complete, has not expired or been revoked, " + + "and has 'repo' (classic) or repository read access (fine-grained).", + exception), + + RateLimitExceededException limit => new GitHubAccessException( + GitHubAccessKind.RateLimited, $"GitHub API rate limit exceeded; it resets at {limit.Reset:u}.", exception), + + // A burst of requests, not a scope problem; Octokit models it beside (not + // under) AbuseException, so both get the same translation. + SecondaryRateLimitExceededException => new GitHubAccessException( + GitHubAccessKind.RateLimited, + "GitHub's secondary rate limit was hit (too many requests in a short time); retry in a minute.", + exception), + + AbuseException abuse => new GitHubAccessException( + GitHubAccessKind.RateLimited, + "GitHub's secondary rate limit was hit (too many requests in a short time); " + + $"retry in {abuse.RetryAfterSeconds ?? 60} seconds.", + exception), + + LoginAttemptsExceededException => new GitHubAccessException( + GitHubAccessKind.RateLimited, + "GitHub temporarily blocked this token after too many failed attempts; wait a few minutes and retry.", + exception), + + ForbiddenException or LegalRestrictionException => new GitHubAccessException( + GitHubAccessKind.Forbidden, + $"GitHub refused access ({(int)((ApiException)exception).StatusCode}): {Sentence(exception.Message)} " + + "If the organization uses SAML single sign-on, authorize the token for it; " + + "otherwise check what the token is allowed to access.", + exception), + + ApiException { StatusCode: >= HttpStatusCode.InternalServerError } server => new GitHubAccessException( + GitHubAccessKind.Unavailable, + $"GitHub answered with an error on its side (HTTP {(int)server.StatusCode}); retry later.", + exception), + + HttpRequestException => new GitHubAccessException( + GitHubAccessKind.Unavailable, + $"GitHub could not be reached: {Sentence(exception.Message)} Check the network connection and retry.", + exception), + + OperationCanceledException when !cancellationToken.IsCancellationRequested => new GitHubAccessException( + GitHubAccessKind.Unavailable, "GitHub did not answer in time; retry later.", exception), + + _ => null, + }; + } + + + + /// A reason from elsewhere as one sentence: trimmed, ending in a period. + private static string Sentence(string text) + { + string trimmed = text.Trim().TrimEnd('.'); + return trimmed.Length == 0 ? "no reason was given." : trimmed + "."; + } +} diff --git a/gclo.Engine/GitHubOrganizationLister.cs b/gclo.Engine/GitHubOrganizationLister.cs index 50a7dc0..1921cfb 100644 --- a/gclo.Engine/GitHubOrganizationLister.cs +++ b/gclo.Engine/GitHubOrganizationLister.cs @@ -43,12 +43,14 @@ public sealed class GitHubOrganizationLister : IOrganizationLister try { // The token's own account is a valid sync target too (personal repos - // live under /users, not /orgs), so it heads the list. - userLogin = await gateway.GetCurrentUserLoginAsync().ConfigureAwait(false); + // live under /users, not /orgs), so it heads the list. The gateway cannot + // be canceled, so each request is awaited with the token: one that hangs + // is abandoned and ends on its own, at the HTTP timeout. + userLogin = await gateway.GetCurrentUserLoginAsync().WaitAsync(cancellationToken).ConfigureAwait(false); try { - organizations = await gateway.GetOrganizationLoginsAsync().ConfigureAwait(false); + organizations = await gateway.GetOrganizationLoginsAsync().WaitAsync(cancellationToken).ConfigureAwait(false); } catch(ForbiddenException ex) when(ex is not RateLimitExceededException @@ -65,32 +67,10 @@ public sealed class GitHubOrganizationLister : IOrganizationLister return [userLogin]; } } - catch(AuthorizationException ex) + catch(Exception ex) when(GitHubFailures.Knows(ex, cancellationToken)) { - throw new GitHubAccessException(GitHubAccessKind.Unauthorized, "GitHub rejected the token (401). Check the PAT.", ex); - } - catch(RateLimitExceededException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, $"GitHub API rate limit exceeded; it resets at {ex.Reset:u}.", ex); - } - catch(SecondaryRateLimitExceededException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, - "GitHub's secondary rate limit was hit (too many requests in a short time); retry in a minute.", ex); - } - catch(AbuseException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, - $"GitHub's secondary rate limit was hit (too many requests in a short time); retry in {ex.RetryAfterSeconds ?? 60} seconds.", ex); - } - catch(LoginAttemptsExceededException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, - "GitHub temporarily blocked this token after too many failed attempts; wait a few minutes and retry.", ex); + // What the failure means to a caller is decided in one place, for both listers. + throw GitHubFailures.Translate(ex, cancellationToken); } cancellationToken.ThrowIfCancellationRequested(); diff --git a/gclo.Engine/GitHubRepositoryLister.cs b/gclo.Engine/GitHubRepositoryLister.cs index ad0a333..5206b25 100644 --- a/gclo.Engine/GitHubRepositoryLister.cs +++ b/gclo.Engine/GitHubRepositoryLister.cs @@ -53,14 +53,16 @@ public sealed class GitHubRepositoryLister : IRepositoryLister IGitHubGateway gateway = _gatewayFactory(token); // Pages manually (instead of one GetAll* call) so cancellation is honored - // between round trips on owners with hundreds of repositories. + // between round trips on owners with hundreds of repositories. Each round + // trip is awaited with the token as well: the gateway cannot be canceled, so + // a request that hangs is abandoned (it ends on its own, at the HTTP timeout). async Task> PageAsync(Func>> fetchPage) { var all = new List(); for(int page = 1; ; page++) { cancellationToken.ThrowIfCancellationRequested(); - IReadOnlyList batch = await fetchPage(page).ConfigureAwait(false); + IReadOnlyList batch = await fetchPage(page).WaitAsync(cancellationToken).ConfigureAwait(false); all.AddRange(batch); if(batch.Count < IGitHubGateway.PageSize) { @@ -87,7 +89,8 @@ public sealed class GitHubRepositoryLister : IRepositoryLister // organization would list only its public repositories and report a // green sync that silently omitted the rest, so the account kind is // resolved first and an invisible organization is an error (#31). - GitHubAccountKind kind = await gateway.GetAccountKindAsync(organization).ConfigureAwait(false); + GitHubAccountKind kind = await gateway.GetAccountKindAsync(organization) + .WaitAsync(cancellationToken).ConfigureAwait(false); if(kind == GitHubAccountKind.Organization) { throw new GitHubAccessException( @@ -106,7 +109,8 @@ public sealed class GitHubRepositoryLister : IRepositoryLister // A user account: the token's own account gets its owned repos // (including private); any other user account yields the repos the // token can see there (public). - string currentUser = await gateway.GetCurrentUserLoginAsync().ConfigureAwait(false); + string currentUser = await gateway.GetCurrentUserLoginAsync() + .WaitAsync(cancellationToken).ConfigureAwait(false); if(string.Equals(currentUser, organization, StringComparison.OrdinalIgnoreCase)) { repositories = await PageAsync(gateway.GetOwnRepositoriesPageAsync) @@ -129,30 +133,10 @@ public sealed class GitHubRepositoryLister : IRepositoryLister } } } - catch(AuthorizationException ex) + catch(Exception ex) when(GitHubFailures.Knows(ex, cancellationToken)) { - throw new GitHubAccessException( - GitHubAccessKind.Unauthorized, - "GitHub rejected the token (401). Check the PAT and make sure it has 'repo' (classic) or repository read access (fine-grained).", ex); - } - catch(RateLimitExceededException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, $"GitHub API rate limit exceeded; it resets at {ex.Reset:u}.", ex); - } - catch(SecondaryRateLimitExceededException ex) - { - // A burst of requests, not a scope problem; Octokit models it beside (not - // under) AbuseException, so both get the same translation. - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, - "GitHub's secondary rate limit was hit (too many requests in a short time); retry in a minute.", ex); - } - catch(AbuseException ex) - { - throw new GitHubAccessException( - GitHubAccessKind.RateLimited, - $"GitHub's secondary rate limit was hit (too many requests in a short time); retry in {ex.RetryAfterSeconds ?? 60} seconds.", ex); + // What the failure means to a caller is decided in one place, for both listers. + throw GitHubFailures.Translate(ex, cancellationToken); } cancellationToken.ThrowIfCancellationRequested(); diff --git a/gclo.Engine/OrgSyncEngine.cs b/gclo.Engine/OrgSyncEngine.cs index 38c1d1d..9bb1cec 100644 --- a/gclo.Engine/OrgSyncEngine.cs +++ b/gclo.Engine/OrgSyncEngine.cs @@ -71,6 +71,9 @@ public sealed class OrgSyncEngine ArgumentException.ThrowIfNullOrWhiteSpace(request.Token, nameof(request)); ArgumentException.ThrowIfNullOrWhiteSpace(request.TargetRoot, nameof(request)); + // Nothing is reported for a repository after its final state in this run. + progress = progress is null ? null : new RunProgress(progress); + var repos = repositories // Names key the pending dictionary and the target folders; a duplicate // (possible from pagination shifts) must not crash the run or race two diff --git a/gclo.Engine/RunProgress.cs b/gclo.Engine/RunProgress.cs new file mode 100644 index 0000000..fe2f89c --- /dev/null +++ b/gclo.Engine/RunProgress.cs @@ -0,0 +1,54 @@ +/* + * Copyright (c) 2026 James Maes (KofTwentyTwo) + * SPDX-License-Identifier: MIT + */ + +namespace gclo.Engine; + + +/// +/// What one run reports through. Every report is passed on, except a report for a +/// repository that has already reported its final state in this run: a git +/// operation that was abandoned on cancellation keeps running for a while and +/// keeps reporting transfer progress, and without this a row that says Canceled +/// would go back to "Cloning 40%". A repository therefore ends a run in exactly +/// one final state, and nothing follows it. +/// +internal sealed class RunProgress : IProgress +{ + private readonly Lock _gate = new(); + + private readonly IProgress _inner; + + // The engine treats names that differ only by case as one repository. + private readonly HashSet _finished = new(StringComparer.OrdinalIgnoreCase); + + + + /// The caller's progress, which receives what is passed on. + public RunProgress(IProgress inner) + { + ArgumentNullException.ThrowIfNull(inner); + _inner = inner; + } + + + + /// + public void Report(RepoProgress value) + { + /////////////////////////////////////////////////////////////////////////////// + // Passed on inside the lock: a progress report racing the final one on // + // another thread is then either before it, or dropped; never after it. // + /////////////////////////////////////////////////////////////////////////////// + lock(_gate) + { + bool isFinal = value.Status is SyncStatus.Done or SyncStatus.Failed or SyncStatus.Canceled; + bool alreadyFinished = isFinal ? !_finished.Add(value.RepoName) : _finished.Contains(value.RepoName); + if(!alreadyFinished) + { + _inner.Report(value); + } + } + } +} -- 2.54.0 From d6750699b6e5e66b358bb62faebefb77eaba96da Mon Sep 17 00:00:00 2001 From: James Maes Date: Tue, 10 Nov 2026 15:38:52 -0600 Subject: [PATCH 2/2] fix(github): address the independent review of the failure mapping The caller's cancellation is decided once; 429 is a rate limit and a proxy's 407 is unavailable; messages take GitHub's reason only from a JSON answer and describe a network failure by its kind, never by the platform's text; an abandoned request's late failure is observed; a final state counts only once the caller's progress took it; help texts and docs match. Refs #153 Co-Authored-By: Claude Fable 5.2 Signed-off-by: James Maes --- docs/CLI.md | 12 +- gclo.Cli/OrgsCommand.cs | 4 +- gclo.Cli/Program.cs | 3 +- gclo.Cli/ReposCommand.cs | 2 +- gclo.Cli/SyncCommand.cs | 3 +- gclo.Engine.Tests/GitHubListerTests.cs | 200 +++++++++++++++++++++--- gclo.Engine.Tests/OrgSyncEngineTests.cs | 2 +- gclo.Engine.Tests/RunProgressTests.cs | 29 ++++ gclo.Engine/GitHubFailures.cs | 151 +++++++++++++++--- gclo.Engine/GitHubOrganizationLister.cs | 8 +- gclo.Engine/GitHubRepositoryLister.cs | 13 +- gclo.Engine/RunProgress.cs | 16 +- 12 files changed, 380 insertions(+), 63 deletions(-) diff --git a/docs/CLI.md b/docs/CLI.md index 8424bd4..27897fd 100644 --- a/docs/CLI.md +++ b/docs/CLI.md @@ -403,12 +403,16 @@ its offset from UTC (`2026-11-10 14:03:22.123 -06:00 [INFO] ...`). Text that cam | 0 | Everything succeeded (or `--dry-run` / a listing completed). | — | | 1 | The run completed, but some repositories failed or the run was canceled (Ctrl+C). `--json`'s `wasCanceled` tells the two apart. | Inspect the failures; retry them. | | 2 | Fatal: bad arguments, missing/empty token, organization not found, unknown account, missing account token, unusable target folder, or an account command on a non-Windows OS. | Fix the invocation. | -| 3 | GitHub rejected the token (401), or refused it access (403: the token may not see the organization, or the organization requires SAML single sign-on and the token is not authorized for it). The message carries GitHub's reason. | Fix the token or its access; do not retry as is. | -| 4 | Not your doing: GitHub's primary or secondary rate limit, a temporary throttle, a network failure, a request that timed out, or an error on GitHub's side (HTTP 5xx). | Retry later (for a rate limit, the message says when). | +| 3 | GitHub rejected the token (401), or refused it access (403: the token may not see the organization, or the organization requires SAML single sign-on and the token is not authorized for it). The message carries GitHub's reason when it gave one. | Fix the token or its access; do not retry as is. | +| 4 | Not your doing: GitHub's primary or secondary rate limit (403 or 429), a temporary throttle, a network failure (DNS, connection, TLS, proxy), a request that timed out, or an error on GitHub's side (HTTP 5xx). | Retry later (for a rate limit, the message says when). | | 70 | An unexpected error inside gclo. The exception type is printed and the full details are in the activity log. | Please report it. | Codes 3 and 4 were part of code 2 before version 1.1; a script that tested `-eq 2` for "usage error" should treat `3` and `4` as distinct outcomes. +Before version 1.1.1, a refusal of access other than an invisible organization +(403), a rate limit answered with 429, a network failure, a timeout, and an +error on GitHub's side ended with code 70; they now end with `3` (the refusal) +or `4` (the rest). A repository recovered by `--sanitize-paths` counts as a success: if every other repository also succeeds, the exit code is `0`. With `--json` or `--json-lines`, @@ -452,7 +456,7 @@ gclo sync --org contoso --target D:\mirror\contoso --parallel 16 --quiet switch ($LASTEXITCODE) { 0 { } 1 { Write-Warning "some repositories failed" } - 4 { Write-Warning "rate limited; will retry next run" } + 4 { Write-Warning "rate limit, network or GitHub-side failure; will retry next run" } default { Write-Error "sync ended with code $LASTEXITCODE" } } @@ -507,7 +511,7 @@ gclo sync --org contoso --target /srv/mirror/contoso --parallel 16 --quiet case $? in 0) ;; 1) echo "some repositories failed" ;; - 4) echo "rate limited; retrying next run" ;; + 4) echo "rate limit, network or GitHub-side failure; retrying next run" ;; *) echo "sync ended with code $?" >&2 ;; esac diff --git a/gclo.Cli/OrgsCommand.cs b/gclo.Cli/OrgsCommand.cs index f47e955..cec1117 100644 --- a/gclo.Cli/OrgsCommand.cs +++ b/gclo.Cli/OrgsCommand.cs @@ -33,8 +33,8 @@ internal static class OrgsCommand 0 listed successfully (even when only the account login is visible) 1 canceled (Ctrl+C) 2 fatal: bad arguments, or missing token - 3 the token was rejected (401) - 4 rate limited; retry later + 3 the token was rejected (401) or refused access (403) + 4 rate limit, network failure, timeout, or an error on GitHub's side; retry later Security: There is deliberately no '--token ' option: process command lines diff --git a/gclo.Cli/Program.cs b/gclo.Cli/Program.cs index a12dc6c..a1651b3 100644 --- a/gclo.Cli/Program.cs +++ b/gclo.Cli/Program.cs @@ -60,7 +60,8 @@ internal static class Program Exit codes: 0 success, 1 completed with failures or canceled, 2 usage or fatal error, - 3 token rejected or access denied, 4 rate limited (retry later), + 3 token rejected or access denied, 4 rate limit, network or GitHub-side + failure (retry later), 70 unexpected error (details in the activity log). """; diff --git a/gclo.Cli/ReposCommand.cs b/gclo.Cli/ReposCommand.cs index a2b00c3..df91851 100644 --- a/gclo.Cli/ReposCommand.cs +++ b/gclo.Cli/ReposCommand.cs @@ -42,7 +42,7 @@ internal static class ReposCommand 1 canceled (Ctrl+C) 2 fatal: bad arguments, missing token, organization not found 3 the token was rejected or may not see the organization - 4 rate limited; retry later + 4 rate limit, network failure, timeout, or an error on GitHub's side; retry later """; diff --git a/gclo.Cli/SyncCommand.cs b/gclo.Cli/SyncCommand.cs index fd1d41c..eca1bde 100644 --- a/gclo.Cli/SyncCommand.cs +++ b/gclo.Cli/SyncCommand.cs @@ -92,7 +92,8 @@ internal static class SyncCommand account or missing account token, unusable target folder 3 the token was rejected (401) or may not see the organization (403): fix the token, do not retry as is - 4 rate limited or temporarily throttled: retry later + 4 not your doing (rate limit, network failure, timeout, or an error on + GitHub's side): retry later 70 an unexpected error inside gclo; see the activity log With --json or --json-lines, exits 2-70 print no document on stdout. diff --git a/gclo.Engine.Tests/GitHubListerTests.cs b/gclo.Engine.Tests/GitHubListerTests.cs index 32e4031..0672307 100644 --- a/gclo.Engine.Tests/GitHubListerTests.cs +++ b/gclo.Engine.Tests/GitHubListerTests.cs @@ -125,9 +125,9 @@ public sealed class GitHubListerTests /// Octokit's Response type is internal; its IResponse is not. - private sealed class FakeResponse(HttpStatusCode status, RateLimit? rateLimit = null) : IResponse + private sealed class FakeResponse(HttpStatusCode status, RateLimit? rateLimit = null, string body = "") : IResponse { - public object? Body => ""; + public object? Body => body; @@ -513,7 +513,16 @@ public sealed class GitHubListerTests - private static HttpRequestException NetworkFailure() => new("No such host is known. (api.github.com:443)"); + private static HttpRequestException NetworkFailure() + => new(HttpRequestError.NameResolutionError, "No such host is known. (api.github.com:443)"); + + + + /// A refusal the way the API sends it: a status and a reason in a JSON document. + private static ForbiddenException ForbiddenBecause(string reason) + => new(new FakeResponse( + HttpStatusCode.Forbidden, + body: System.Text.Json.JsonSerializer.Serialize(new Dictionary(StringComparer.Ordinal) { ["message"] = reason }))); @@ -525,14 +534,17 @@ public sealed class GitHubListerTests [Fact] public async Task Repos_AccessRefused_IsForbidden_WithGitHubsReason() { - _gateway.OrgPages = (_, _) => throw Forbidden(); + _gateway.OrgPages = (_, _) => throw ForbiddenBecause("Resource protected by organization SAML enforcement"); GitHubAccessException ex = await Assert.ThrowsAsync( () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); Assert.Equal(GitHubAccessKind.Forbidden, ex.Kind); - Assert.StartsWith("GitHub refused access (403): ", ex.Message); - Assert.Contains("SAML single sign-on", ex.Message); + Assert.Equal( + "GitHub refused access (403): Resource protected by organization SAML enforcement. " + + "If the organization uses SAML single sign-on, authorize the token for it; " + + "otherwise check what the token is allowed to access.", + ex.Message); Assert.IsType(ex.InnerException); } @@ -578,7 +590,7 @@ public sealed class GitHubListerTests Assert.Equal(GitHubAccessKind.Unavailable, ex.Kind); Assert.Equal( - "GitHub could not be reached: No such host is known. (api.github.com:443). Check the network connection and retry.", + "GitHub could not be reached: its name could not be resolved (DNS). Check the network connection and retry.", ex.Message); } @@ -727,6 +739,8 @@ public sealed class GitHubListerTests (LoginAttemptsExceeded(), GitHubAccessKind.RateLimited), (Forbidden(), GitHubAccessKind.Forbidden), (new LegalRestrictionException("Repository access blocked", (HttpStatusCode)451), GitHubAccessKind.Forbidden), + (new ApiException("Too Many Requests", HttpStatusCode.TooManyRequests), GitHubAccessKind.RateLimited), + (new ApiException("Proxy Authentication Required", HttpStatusCode.ProxyAuthenticationRequired), GitHubAccessKind.Unavailable), (ServerError(), GitHubAccessKind.Unavailable), (new ApiException("Service Unavailable", HttpStatusCode.ServiceUnavailable), GitHubAccessKind.Unavailable), (NetworkFailure(), GitHubAccessKind.Unavailable), @@ -736,7 +750,7 @@ public sealed class GitHubListerTests Assert.All(known, k => { Assert.True(GitHubFailures.Knows(k.Failure, CancellationToken.None), k.Failure.GetType().Name); - GitHubAccessException translated = GitHubFailures.Translate(k.Failure, CancellationToken.None); + GitHubAccessException translated = GitHubFailures.Translate(k.Failure); Assert.Equal(k.Kind, translated.Kind); Assert.Same(k.Failure, translated.InnerException); }); @@ -747,24 +761,16 @@ public sealed class GitHubListerTests [Fact] public void Failures_ALegalRestriction_SaysItsStatusAndReason() { - GitHubAccessException translated = GitHubFailures.Translate( - new LegalRestrictionException("Repository access blocked.", (HttpStatusCode)451), CancellationToken.None); + var blocked = new LegalRestrictionException(new FakeResponse( + (HttpStatusCode)451, body: "{\"message\":\"Repository access blocked\"}")); + + GitHubAccessException translated = GitHubFailures.Translate(blocked); Assert.StartsWith("GitHub refused access (451): Repository access blocked. If the organization", translated.Message); } - [Fact] - public void Failures_AnEmptyReason_IsSaidAsSuch() - { - GitHubAccessException translated = GitHubFailures.Translate(new HttpRequestException(" . "), CancellationToken.None); - - Assert.StartsWith("GitHub could not be reached: no reason was given. ", translated.Message); - } - - - [Fact] public void Failures_WhatHasNoKnownMeaning_IsNotTranslated() { @@ -781,6 +787,158 @@ public sealed class GitHubListerTests Assert.All(unknown, failure => Assert.False(GitHubFailures.Knows(failure, CancellationToken.None), failure.GetType().Name)); // A cancellation the caller asked for is a cancellation, not a timeout. Assert.False(GitHubFailures.Knows(new OperationCanceledException(canceled.Token), canceled.Token)); - Assert.Throws(() => GitHubFailures.Translate(new InvalidOperationException("a bug"), CancellationToken.None)); + Assert.Throws(() => GitHubFailures.Translate(new InvalidOperationException("a bug"))); + } + + + + [Fact] + public void Failures_ACancellationThatArrivesAfterTheDecision_DoesNotChangeIt() + { + // The request timed out while the caller had not canceled: that is decided once. + using var caller = new CancellationTokenSource(); + TaskCanceledException timeout = TimedOut(); + + bool known = GitHubFailures.Knows(timeout, caller.Token); + caller.Cancel(); // lands between the decision and the translation + GitHubAccessException translated = GitHubFailures.Translate(timeout); + + Assert.True(known); + Assert.Equal(GitHubAccessKind.Unavailable, translated.Kind); + } + + + + [Fact] + public async Task Repos_AFailureThatArrivesWhenTheRequestEnds_IsTranslatedLikeOneThrownAtOnce() + { + // The real gateway fails the request it returned; it does not throw before returning one. + _gateway.OrgPages = (_, _) => Task.FromException>(ServerError()); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.Unavailable, ex.Kind); + } + + + + [Fact] + public async Task Repos_ARequestAbandonedOnCancellation_MayFailLater_WithoutAnyoneBeingTold() + { + using var cts = new CancellationTokenSource(); + var requested = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var request = new TaskCompletionSource>(TaskCreationOptions.RunContinuationsAsynchronously); + _gateway.OrgPages = (_, _) => + { + requested.SetResult(); + return request.Task; + }; + Task> listing = RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890", cts.Token); + await requested.Task.WaitAsync(TimeSpan.FromSeconds(30)); + cts.Cancel(); + await Assert.ThrowsAnyAsync(() => listing.WaitAsync(TimeSpan.FromSeconds(30))); + + request.SetException(NetworkFailure()); // the abandoned request ends on its own, badly + + // The listing ended as a cancellation and stays one; the late failure belongs to nobody. + Assert.True(listing.IsCanceled); + await Assert.ThrowsAsync(() => request.Task); + } + + + + [Fact] + public async Task Repos_TooManyRequests_IsARateLimit_AlsoAsStatus429() + { + _gateway.OrgPages = (_, _) => throw new ApiException("Too Many Requests", HttpStatusCode.TooManyRequests); + + GitHubAccessException ex = await Assert.ThrowsAsync( + () => RepoLister().ListOrganizationRepositoriesAsync("acme", "tok-1234567890")); + + Assert.Equal(GitHubAccessKind.RateLimited, ex.Kind); + Assert.Contains("429", ex.Message); + } + + + + [Theory] + [InlineData(HttpRequestError.NameResolutionError, "its name could not be resolved (DNS)")] + [InlineData(HttpRequestError.ConnectionError, "the connection could not be made")] + [InlineData(HttpRequestError.SecureConnectionError, "a secure (TLS) connection could not be established")] + [InlineData(HttpRequestError.ProxyTunnelError, "the proxy on the way refused the connection")] + [InlineData(HttpRequestError.Unknown, "the request failed on the way.")] + [InlineData(HttpRequestError.ResponseEnded, "the request failed on the way (ResponseEnded)")] + public void Failures_ANetworkFailure_IsDescribedByItsKind(HttpRequestError error, string expected) + { + GitHubAccessException translated = GitHubFailures.Translate(new HttpRequestException(error, "whatever the platform says")); + + Assert.StartsWith("GitHub could not be reached: ", translated.Message); + Assert.Contains(expected, translated.Message); + Assert.EndsWith(" Check the network connection and retry.", translated.Message); + } + + + + [Fact] + public void Failures_ANetworkFailure_NeverCopiesThePlatformsText_WhichCanNameAProxyWithItsPassword() + { + var failure = new HttpRequestException( + HttpRequestError.ProxyTunnelError, + "The proxy tunnel request to proxy 'http://proxyuser:PROXYPASSWORD@10.0.0.1:8080/' failed with status code '407'."); + + GitHubAccessException translated = GitHubFailures.Translate(failure); + + Assert.DoesNotContain("PROXYPASSWORD", translated.Message); + Assert.DoesNotContain("proxyuser", translated.Message); + Assert.Same(failure, translated.InnerException); // the full text stays with the cause, for the log + } + + + + [Fact] + public void Failures_ARefusalWithoutAReasonFromTheApi_SaysNone_AndCopiesNoPage() + { + // A web filter or a proxy answers 403 with a page of its own; that is not GitHub's reason. + var blockPage = new ForbiddenException(new FakeResponse( + HttpStatusCode.Forbidden, body: "

Blocked by policy

" + new string('x', 3000) + "")); + + GitHubAccessException translated = GitHubFailures.Translate(blockPage); + + Assert.Equal( + "GitHub refused access (403). If the organization uses SAML single sign-on, authorize the token for it; " + + "otherwise check what the token is allowed to access.", + translated.Message); + } + + + + [Theory] + [InlineData("")] + [InlineData("[]")] + [InlineData("{\"other\":1}")] + [InlineData("{\"message\":42}")] + [InlineData("{\"message\":\" \"}")] + public void Failures_ARefusalWhoseAnswerHasNoReasonInIt_SaysNone(string body) + { + GitHubAccessException translated = GitHubFailures.Translate( + new ForbiddenException(new FakeResponse(HttpStatusCode.Forbidden, body: body))); + + Assert.StartsWith("GitHub refused access (403). If the organization", translated.Message); + } + + + + [Fact] + public void Failures_AReasonFromTheApi_IsPutOnOneLine_AndCut() + { + string emoji = char.ConvertFromUtf32(0x1F600); + GitHubAccessException lines = GitHubFailures.Translate(ForbiddenBecause(" first line\r\n second\tline? ")); + GitHubAccessException longReason = GitHubFailures.Translate(ForbiddenBecause(new string('r', 400))); + GitHubAccessException cutAtAPair = GitHubFailures.Translate(ForbiddenBecause(new string('r', 299) + emoji + "tail")); + + Assert.StartsWith("GitHub refused access (403): first line second line? If the organization", lines.Message); + Assert.StartsWith("GitHub refused access (403): " + new string('r', 300) + "... If the organization", longReason.Message); + Assert.StartsWith("GitHub refused access (403): " + new string('r', 299) + "... If the organization", cutAtAPair.Message); } } diff --git a/gclo.Engine.Tests/OrgSyncEngineTests.cs b/gclo.Engine.Tests/OrgSyncEngineTests.cs index 0c93164..a743427 100644 --- a/gclo.Engine.Tests/OrgSyncEngineTests.cs +++ b/gclo.Engine.Tests/OrgSyncEngineTests.cs @@ -593,7 +593,7 @@ public sealed class OrgSyncEngineTests : IDisposable Assert.True(cts.IsCancellationRequested); Assert.Equal(new SyncSummary(Total: 1, Cloned: 1, Updated: 0, Failed: 0, Canceled: 0, WasCanceled: false), summary); -||||||| parent of e2289fd (fix(github): classify access, network and server failures; cancel a listing promptly) + } diff --git a/gclo.Engine.Tests/RunProgressTests.cs b/gclo.Engine.Tests/RunProgressTests.cs index 5379deb..2fb5a53 100644 --- a/gclo.Engine.Tests/RunProgressTests.cs +++ b/gclo.Engine.Tests/RunProgressTests.cs @@ -98,6 +98,35 @@ public sealed class RunProgressTests + /// A caller's progress that fails on the first final state it is given. + private sealed class ThrowsOnDone(RecordingProgress seen) : IProgress + { + public void Report(RepoProgress value) + { + if(value.Status == SyncStatus.Done) + { + throw new InvalidOperationException("the caller's progress failed"); + } + seen.Report(value); + } + } + + + + [Fact] + public void Report_AFinalStateTheCallerDidNotTake_DoesNotCount_SoTheNextOneGetsThrough() + { + var run = new RunProgress(new ThrowsOnDone(_seen)); + + Assert.Throws(() => run.Report(new RepoProgress("alpha", SyncStatus.Done))); + run.Report(new RepoProgress("alpha", SyncStatus.Failed, "the caller's progress failed")); + run.Report(new RepoProgress("alpha", SyncStatus.Cloning)); + + Assert.Equal([SyncStatus.Failed], _seen.StatusesFor("alpha")); + } + + + [Fact] public void Constructor_RefusesAMissingProgress() => Assert.Throws(() => new RunProgress(null!)); diff --git a/gclo.Engine/GitHubFailures.cs b/gclo.Engine/GitHubFailures.cs index 76d83f6..fa827f0 100644 --- a/gclo.Engine/GitHubFailures.cs +++ b/gclo.Engine/GitHubFailures.cs @@ -4,6 +4,7 @@ */ using System.Net; +using System.Text.Json; using Octokit; @@ -19,6 +20,29 @@ namespace gclo.Engine; /// internal static class GitHubFailures { + /// The longest reason from GitHub that is put into a message. + private const int MaxReasonLength = 300; + + + + /// + /// Waits for one gateway request, or for the caller's cancellation, whichever + /// comes first. The gateway cannot cancel a request, so a canceled one is left + /// to end on its own (at the HTTP timeout at the latest); whatever it ends with + /// is looked at here, so it never surfaces as a failure nobody observed. + /// + public static Task WaitFor(Task request, CancellationToken cancellationToken) + { + _ = request.ContinueWith( + static abandoned => _ = abandoned.Exception, + CancellationToken.None, + TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously, + TaskScheduler.Default); + return request.WaitAsync(cancellationToken); + } + + + /// /// True when is a failure with a known meaning, which /// then puts into words. @@ -26,25 +50,27 @@ internal static class GitHubFailures /// What a gateway call threw. /// /// The caller's token: a canceled request is a timeout only when the caller did - /// not cancel it. + /// not cancel it. This is the one moment the token is looked at. /// public static bool Knows(Exception exception, CancellationToken cancellationToken) - => Find(exception, cancellationToken) is not null; + => Find(exception, canceledByCaller: cancellationToken.IsCancellationRequested) is not null; /// /// The that says what - /// means to a caller. Only for a failure that accepted. + /// means to a caller. Only for a failure that accepted: the + /// decision whether a canceled request was the caller's doing was made there and + /// is not made again here, so a cancellation that arrives in between changes nothing. /// /// The failure has no known meaning. - public static GitHubAccessException Translate(Exception exception, CancellationToken cancellationToken) - => Find(exception, cancellationToken) + public static GitHubAccessException Translate(Exception exception) + => Find(exception, canceledByCaller: false) ?? throw new ArgumentException("This failure has no known meaning; ask Knows first.", nameof(exception)); - private static GitHubAccessException? Find(Exception exception, CancellationToken cancellationToken) + private static GitHubAccessException? Find(Exception exception, bool canceledByCaller) { /////////////////////////////////////////////////////////////////////////////// // Order matters: the throttling exceptions derive from ForbiddenException, // @@ -79,11 +105,19 @@ internal static class GitHubFailures "GitHub temporarily blocked this token after too many failed attempts; wait a few minutes and retry.", exception), + // GitHub answers a rate limit with 403 or with 429; Octokit has no type for the second. + ApiException { StatusCode: HttpStatusCode.TooManyRequests } => new GitHubAccessException( + GitHubAccessKind.RateLimited, + "GitHub is limiting requests from this token (429: too many requests); retry in a minute.", + exception), + ForbiddenException or LegalRestrictionException => new GitHubAccessException( - GitHubAccessKind.Forbidden, - $"GitHub refused access ({(int)((ApiException)exception).StatusCode}): {Sentence(exception.Message)} " - + "If the organization uses SAML single sign-on, authorize the token for it; " - + "otherwise check what the token is allowed to access.", + GitHubAccessKind.Forbidden, Refused((ApiException)exception), exception), + + ApiException { StatusCode: HttpStatusCode.ProxyAuthenticationRequired } => new GitHubAccessException( + GitHubAccessKind.Unavailable, + "A proxy on the way to GitHub asked gclo to sign in (HTTP 407), which it cannot do. " + + "Check the proxy settings of this machine.", exception), ApiException { StatusCode: >= HttpStatusCode.InternalServerError } server => new GitHubAccessException( @@ -91,12 +125,10 @@ internal static class GitHubFailures $"GitHub answered with an error on its side (HTTP {(int)server.StatusCode}); retry later.", exception), - HttpRequestException => new GitHubAccessException( - GitHubAccessKind.Unavailable, - $"GitHub could not be reached: {Sentence(exception.Message)} Check the network connection and retry.", - exception), + HttpRequestException network => new GitHubAccessException( + GitHubAccessKind.Unavailable, Unreachable(network.HttpRequestError), exception), - OperationCanceledException when !cancellationToken.IsCancellationRequested => new GitHubAccessException( + OperationCanceledException when !canceledByCaller => new GitHubAccessException( GitHubAccessKind.Unavailable, "GitHub did not answer in time; retry later.", exception), _ => null, @@ -105,10 +137,91 @@ internal static class GitHubFailures - /// A reason from elsewhere as one sentence: trimmed, ending in a period. - private static string Sentence(string text) + /// + /// The words for a refused request. GitHub's own reason is part of them only when + /// it came as the reason of an API answer; a page from something in between (a web + /// filter, a proxy) is not a reason and is left out. + /// + private static string Refused(ApiException refusal) { - string trimmed = text.Trim().TrimEnd('.'); - return trimmed.Length == 0 ? "no reason was given." : trimmed + "."; + const string Advice = "If the organization uses SAML single sign-on, authorize the token for it; " + + "otherwise check what the token is allowed to access."; + string reason = ReasonOf(refusal); + return reason.Length == 0 + ? $"GitHub refused access ({(int)refusal.StatusCode}). {Advice}" + : $"GitHub refused access ({(int)refusal.StatusCode}): {reason} {Advice}"; + } + + + + /// + /// The reason GitHub's API gave for a refusal: the "message" of the JSON document it + /// answered with. Anything else (no body, a page of HTML) has no reason in it; + /// Octokit would hand over such a body whole. + /// + private static string ReasonOf(ApiException refusal) + { + if(refusal.HttpResponse?.Body is not string body) + { + return ""; + } + + try + { + using var document = JsonDocument.Parse(body); + return document.RootElement.ValueKind == JsonValueKind.Object + && document.RootElement.TryGetProperty("message", out JsonElement message) + && message.ValueKind == JsonValueKind.String + ? OneLine(message.GetString()) + : ""; + } + catch(JsonException) + { + return ""; + } + } + + + + /// + /// The words for a request that never got an answer. Chosen by the kind of failure, + /// never copied from the failure's own text: that text can name a proxy together + /// with the user name and password used for it. + /// + private static string Unreachable(HttpRequestError error) + { + string what = error switch + { + HttpRequestError.NameResolutionError => "its name could not be resolved (DNS)", + HttpRequestError.ConnectionError => "the connection could not be made", + HttpRequestError.SecureConnectionError + => "a secure (TLS) connection could not be established; a proxy or a security product that inspects traffic is a common cause", + HttpRequestError.ProxyTunnelError => "the proxy on the way refused the connection", + HttpRequestError.Unknown => "the request failed on the way", + _ => $"the request failed on the way ({error})", + }; + return $"GitHub could not be reached: {what}. Check the network connection and retry."; + } + + + + /// + /// A reason from GitHub as one line of bounded length that ends a sentence; empty + /// when there is none. + /// + private static string OneLine(string? text) + { + string line = string.Join(' ', (text ?? "").Split((char[]?)null, StringSplitOptions.RemoveEmptyEntries)); + if(line.Length == 0) + { + return ""; + } + if(line.Length > MaxReasonLength) + { + // Cut between characters, never through a pair of code units. + int keep = char.IsHighSurrogate(line[MaxReasonLength - 1]) ? MaxReasonLength - 1 : MaxReasonLength; + return line[..keep] + "..."; + } + return line[^1] is '.' or '!' or '?' ? line : line + "."; } } diff --git a/gclo.Engine/GitHubOrganizationLister.cs b/gclo.Engine/GitHubOrganizationLister.cs index 1921cfb..6b4febb 100644 --- a/gclo.Engine/GitHubOrganizationLister.cs +++ b/gclo.Engine/GitHubOrganizationLister.cs @@ -46,11 +46,13 @@ public sealed class GitHubOrganizationLister : IOrganizationLister // live under /users, not /orgs), so it heads the list. The gateway cannot // be canceled, so each request is awaited with the token: one that hangs // is abandoned and ends on its own, at the HTTP timeout. - userLogin = await gateway.GetCurrentUserLoginAsync().WaitAsync(cancellationToken).ConfigureAwait(false); + userLogin = await GitHubFailures.WaitFor(gateway.GetCurrentUserLoginAsync(), cancellationToken) + .ConfigureAwait(false); try { - organizations = await gateway.GetOrganizationLoginsAsync().WaitAsync(cancellationToken).ConfigureAwait(false); + organizations = await GitHubFailures.WaitFor(gateway.GetOrganizationLoginsAsync(), cancellationToken) + .ConfigureAwait(false); } catch(ForbiddenException ex) when(ex is not RateLimitExceededException @@ -70,7 +72,7 @@ public sealed class GitHubOrganizationLister : IOrganizationLister catch(Exception ex) when(GitHubFailures.Knows(ex, cancellationToken)) { // What the failure means to a caller is decided in one place, for both listers. - throw GitHubFailures.Translate(ex, cancellationToken); + throw GitHubFailures.Translate(ex); } cancellationToken.ThrowIfCancellationRequested(); diff --git a/gclo.Engine/GitHubRepositoryLister.cs b/gclo.Engine/GitHubRepositoryLister.cs index 5206b25..046320b 100644 --- a/gclo.Engine/GitHubRepositoryLister.cs +++ b/gclo.Engine/GitHubRepositoryLister.cs @@ -62,7 +62,8 @@ public sealed class GitHubRepositoryLister : IRepositoryLister for(int page = 1; ; page++) { cancellationToken.ThrowIfCancellationRequested(); - IReadOnlyList batch = await fetchPage(page).WaitAsync(cancellationToken).ConfigureAwait(false); + IReadOnlyList batch = await GitHubFailures.WaitFor(fetchPage(page), cancellationToken) + .ConfigureAwait(false); all.AddRange(batch); if(batch.Count < IGitHubGateway.PageSize) { @@ -89,8 +90,8 @@ public sealed class GitHubRepositoryLister : IRepositoryLister // organization would list only its public repositories and report a // green sync that silently omitted the rest, so the account kind is // resolved first and an invisible organization is an error (#31). - GitHubAccountKind kind = await gateway.GetAccountKindAsync(organization) - .WaitAsync(cancellationToken).ConfigureAwait(false); + GitHubAccountKind kind = await GitHubFailures + .WaitFor(gateway.GetAccountKindAsync(organization), cancellationToken).ConfigureAwait(false); if(kind == GitHubAccountKind.Organization) { throw new GitHubAccessException( @@ -109,8 +110,8 @@ public sealed class GitHubRepositoryLister : IRepositoryLister // A user account: the token's own account gets its owned repos // (including private); any other user account yields the repos the // token can see there (public). - string currentUser = await gateway.GetCurrentUserLoginAsync() - .WaitAsync(cancellationToken).ConfigureAwait(false); + string currentUser = await GitHubFailures + .WaitFor(gateway.GetCurrentUserLoginAsync(), cancellationToken).ConfigureAwait(false); if(string.Equals(currentUser, organization, StringComparison.OrdinalIgnoreCase)) { repositories = await PageAsync(gateway.GetOwnRepositoriesPageAsync) @@ -136,7 +137,7 @@ public sealed class GitHubRepositoryLister : IRepositoryLister catch(Exception ex) when(GitHubFailures.Knows(ex, cancellationToken)) { // What the failure means to a caller is decided in one place, for both listers. - throw GitHubFailures.Translate(ex, cancellationToken); + throw GitHubFailures.Translate(ex); } cancellationToken.ThrowIfCancellationRequested(); diff --git a/gclo.Engine/RunProgress.cs b/gclo.Engine/RunProgress.cs index fe2f89c..4dc7736 100644 --- a/gclo.Engine/RunProgress.cs +++ b/gclo.Engine/RunProgress.cs @@ -43,11 +43,19 @@ internal sealed class RunProgress : IProgress /////////////////////////////////////////////////////////////////////////////// lock(_gate) { - bool isFinal = value.Status is SyncStatus.Done or SyncStatus.Failed or SyncStatus.Canceled; - bool alreadyFinished = isFinal ? !_finished.Add(value.RepoName) : _finished.Contains(value.RepoName); - if(!alreadyFinished) + if(_finished.Contains(value.RepoName)) { - _inner.Report(value); + return; + } + + _inner.Report(value); + + // Only a final state the caller has actually been given counts: if its + // progress threw, the repository has not finished as far as it knows, and + // the failure the engine then reports for it must still get through. + if(value.Status is SyncStatus.Done or SyncStatus.Failed or SyncStatus.Canceled) + { + _finished.Add(value.RepoName); } } } -- 2.54.0