diff --git a/src/Octoshift/Services/SecondaryRateLimitHandler.cs b/src/Octoshift/Services/SecondaryRateLimitHandler.cs new file mode 100644 index 000000000..8305bcd8d --- /dev/null +++ b/src/Octoshift/Services/SecondaryRateLimitHandler.cs @@ -0,0 +1,156 @@ +// src/Octoshift/Services/SecondaryRateLimitHandler.cs +using System; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Security.Cryptography; +using System.Threading; +using System.Threading.Tasks; + +namespace OctoshiftCLI.Services +{ + /// + /// Handles GitHub secondary rate-limit / abuse-detection 403s by + /// respecting Retry-After (when present) and otherwise using + /// exponential backoff with jitter. Clones the request to safely + /// resend POSTs with content. + /// + public sealed class SecondaryRateLimitHandler : DelegatingHandler + { + private readonly int _maxAttempts; + private readonly TimeSpan _initialBackoff; + private readonly TimeSpan _maxBackoff; + + public SecondaryRateLimitHandler( + HttpMessageHandler innerHandler, + int maxAttempts = 6, + int initialBackoffSeconds = 15, + int maxBackoffSeconds = 900) + : base(innerHandler) + { + _maxAttempts = maxAttempts; + _initialBackoff = TimeSpan.FromSeconds(initialBackoffSeconds); + _maxBackoff = TimeSpan.FromSeconds(maxBackoffSeconds); + } + + protected override async Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + if (request == null) + { + throw new ArgumentNullException(nameof(request)); + } + + var attempt = 0; + var delay = _initialBackoff; + + // Buffer original content (if any) so we can safely clone the request for retries. + byte[] bufferedContent = null; + string contentType = null; + + if (request.Content != null) + { + bufferedContent = await request.Content.ReadAsByteArrayAsync(cancellationToken).ConfigureAwait(false); + contentType = request.Content.Headers.ContentType?.ToString(); + } + + while (true) + { + cancellationToken.ThrowIfCancellationRequested(); + + using var cloned = CloneRequest(request, bufferedContent, contentType); + var response = await base.SendAsync(cloned, cancellationToken).ConfigureAwait(false); + + if (response.StatusCode != HttpStatusCode.Forbidden) + { + return response; + } + + // Look for known secondary rate limit / abuse messages in body. + var body = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); + var looksLikeSecondary = + body.Contains("secondary rate limit", StringComparison.OrdinalIgnoreCase) || + body.Contains("abuse detection", StringComparison.OrdinalIgnoreCase) || + body.Contains("rate limit", StringComparison.OrdinalIgnoreCase); + + if (!looksLikeSecondary) + { + return response; // Some other 403 — don't loop. + } + + attempt++; + if (attempt >= _maxAttempts) + { + return response; // Give up; caller/policy will surface it. + } + + // Prefer server-provided delay if present. + if (response.Headers.TryGetValues("Retry-After", out var values) && + int.TryParse(values.FirstOrDefault(), out var secs) && + secs > 0) + { + delay = TimeSpan.FromSeconds(secs); + } + + // Add secure jitter (0–1000ms) to spread retries. + var jitterMs = RandomNumberGenerator.GetInt32(0, 1000); + var totalDelay = delay + TimeSpan.FromMilliseconds(jitterMs); + + await Task.Delay(totalDelay, cancellationToken).ConfigureAwait(false); + + // Exponential backoff, capped. + var nextSeconds = Math.Min(delay.TotalSeconds * 2, _maxBackoff.TotalSeconds); + delay = TimeSpan.FromSeconds(nextSeconds); + // Loop and retry with a freshly cloned request. + } + } + + private static HttpRequestMessage CloneRequest(HttpRequestMessage original, byte[] bufferedContent, string contentType) + { + var clone = new HttpRequestMessage(original.Method, original.RequestUri) + { + Version = original.Version, + VersionPolicy = original.VersionPolicy + }; + + // Copy headers + foreach (var header in original.Headers) + { + clone.Headers.TryAddWithoutValidation(header.Key, header.Value); + } + + // Copy content + if (bufferedContent != null) + { + var content = new ByteArrayContent(bufferedContent); + if (!string.IsNullOrEmpty(contentType)) + { + content.Headers.TryAddWithoutValidation("Content-Type", contentType); + } + + if (original.Content != null) + { + foreach (var h in original.Content.Headers) + { + if (!string.Equals(h.Key, "Content-Type", StringComparison.OrdinalIgnoreCase)) + { + content.Headers.TryAddWithoutValidation(h.Key, h.Value); + } + } + } + + clone.Content = content; + } + +#if NET6_0_OR_GREATER + foreach (var opt in original.Options) + { + clone.Options.Set(new(opt.Key), opt.Value); + } +#endif + + return clone; + } + } +} diff --git a/src/OctoshiftCLI.IntegrationTests/AdoBasicToGithub.cs b/src/OctoshiftCLI.IntegrationTests/AdoBasicToGithub.cs index 0f4ddce37..fb43ac171 100644 --- a/src/OctoshiftCLI.IntegrationTests/AdoBasicToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/AdoBasicToGithub.cs @@ -34,6 +34,9 @@ await retryPolicy.Retry(async () => var commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject1, adoRepo1); await Helper.CreatePipeline(adoOrg, teamProject1, adoRepo1, pipeline1, commitId); + // tiny pause to smooth bursts + await Task.Delay(500); + await Helper.CreateTeamProject(adoOrg, teamProject2); commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject2, adoRepo2); await Helper.CreatePipeline(adoOrg, teamProject2, adoRepo2, pipeline2, commitId); diff --git a/src/OctoshiftCLI.IntegrationTests/AdoCsvToGithub.cs b/src/OctoshiftCLI.IntegrationTests/AdoCsvToGithub.cs index 5ddca49d4..5294817f2 100644 --- a/src/OctoshiftCLI.IntegrationTests/AdoCsvToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/AdoCsvToGithub.cs @@ -34,6 +34,9 @@ await retryPolicy.Retry(async () => var commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject1, adoRepo1); await Helper.CreatePipeline(adoOrg, teamProject1, adoRepo1, pipeline1, commitId); + // tiny pause to smooth bursts + await Task.Delay(500); + await Helper.CreateTeamProject(adoOrg, teamProject2); commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject2, adoRepo2); await Helper.CreatePipeline(adoOrg, teamProject2, adoRepo2, pipeline2, commitId); diff --git a/src/OctoshiftCLI.IntegrationTests/AdoServerToGithub.cs b/src/OctoshiftCLI.IntegrationTests/AdoServerToGithub.cs index c57689a91..035776c6d 100644 --- a/src/OctoshiftCLI.IntegrationTests/AdoServerToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/AdoServerToGithub.cs @@ -9,7 +9,8 @@ public class AdoServerToGithub : AdoToGithub { private const string ADO_SERVER_URL = "http://octoshift-ado-server-2022.eastus.cloudapp.azure.com/"; - public AdoServerToGithub(ITestOutputHelper output) : base(output, ADO_SERVER_URL, "ADO_SERVER_PAT") + public AdoServerToGithub(ITestOutputHelper output) + : base(output, adoServerUrl: ADO_SERVER_URL, adoPatEnvVar: "ADO_SERVER_PAT") { } @@ -36,6 +37,9 @@ await retryPolicy.Retry(async () => var commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject1, adoRepo1, ADO_SERVER_URL); await Helper.CreatePipeline(adoOrg, teamProject1, adoRepo1, pipeline1, commitId, ADO_SERVER_URL); + // tiny pause to smooth bursts + await Task.Delay(500); + await Helper.CreateTeamProject(adoOrg, teamProject2, ADO_SERVER_URL); commitId = await Helper.InitializeAdoRepo(adoOrg, teamProject2, adoRepo2, ADO_SERVER_URL); await Helper.CreatePipeline(adoOrg, teamProject2, adoRepo2, pipeline2, commitId, ADO_SERVER_URL); diff --git a/src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs b/src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs index 09743b34f..adca53d80 100644 --- a/src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/AdoToGithub.cs @@ -18,14 +18,15 @@ public abstract class AdoToGithub : IDisposable protected Dictionary Tokens { get; } protected DateTime StartTime { get; } - protected AdoToGithub(ITestOutputHelper output, string adoServerUrl = "https://dev.azure.com", string adoPatEnvVar = "ADO_PAT") + + protected AdoToGithub(ITestOutputHelper output, HttpClient githubHttpClient, string adoServerUrl = "https://dev.azure.com", string adoPatEnvVar = "ADO_PAT") { StartTime = DateTime.Now; _output = output; var logger = new OctoLogger(x => { }, x => _output.WriteLine(x), x => { }, x => { }); - _versionClient = new HttpClient(); + _versionClient = HttpClientFactory.CreateSrlClient(); var adoToken = Environment.GetEnvironmentVariable(adoPatEnvVar); _adoHttpClient = new HttpClient(); var retryPolicy = new RetryPolicy(logger); @@ -33,7 +34,8 @@ protected AdoToGithub(ITestOutputHelper output, string adoServerUrl = "https://d var adoApi = new AdoApi(adoClient, adoServerUrl, logger); var githubToken = Environment.GetEnvironmentVariable("GHEC_PAT"); - _githubHttpClient = new HttpClient(); + _githubHttpClient = githubHttpClient ?? HttpClientFactory.CreateSrlClient(); + var githubClient = new GithubClient(logger, _githubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), githubToken); var githubApi = new GithubApi(githubClient, "https://api.github.com", new RetryPolicy(logger), null); @@ -46,6 +48,11 @@ protected AdoToGithub(ITestOutputHelper output, string adoServerUrl = "https://d Helper = new TestHelper(_output, adoApi, githubApi, adoClient, githubClient); } + protected AdoToGithub(ITestOutputHelper output, string adoServerUrl = "https://dev.azure.com", string adoPatEnvVar = "ADO_PAT") + : this(output, githubHttpClient: null, adoServerUrl: adoServerUrl, adoPatEnvVar: adoPatEnvVar) + { + } + protected virtual void Dispose(bool disposing) { if (!disposedValue) @@ -63,7 +70,6 @@ protected virtual void Dispose(bool disposing) public void Dispose() { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method Dispose(disposing: true); GC.SuppressFinalize(this); } diff --git a/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs b/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs index b30602139..fcf603d07 100644 --- a/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/BbsToGithub.cs @@ -53,12 +53,12 @@ public BbsToGithub(ITestOutputHelper output) ["GH_PAT"] = targetGithubToken }; - _versionClient = new HttpClient(); + _versionClient = HttpClientFactory.CreateSrlClient(); _sourceBbsHttpClient = new HttpClient(); _sourceBbsClient = new BbsClient(_logger, _sourceBbsHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger), sourceBbsUsername, sourceBbsPassword); - _targetGithubHttpClient = new HttpClient(); + _targetGithubHttpClient = HttpClientFactory.CreateSrlClient(); _targetGithubClient = new GithubClient(_logger, _targetGithubHttpClient, new VersionChecker(_versionClient, _logger), new RetryPolicy(_logger), new DateTimeProvider(), targetGithubToken); var retryPolicy = new RetryPolicy(_logger); _archiveUploader = new ArchiveUploader(_targetGithubClient, UPLOADS_URL, _logger, retryPolicy); @@ -97,6 +97,10 @@ await retryPolicy.Retry(async () => await sourceHelper.CreateBbsProject(bbsProjectKey); await sourceHelper.CreateBbsRepo(bbsProjectKey, repo1); await sourceHelper.InitializeBbsRepo(bbsProjectKey, repo1); + + // tiny pause to smooth bursts + await Task.Delay(500); + await sourceHelper.CreateBbsRepo(bbsProjectKey, repo2); await sourceHelper.InitializeBbsRepo(bbsProjectKey, repo2); }); @@ -154,7 +158,6 @@ public async Task MigrateRepo_MultipartUpload() var sshKey = Environment.GetEnvironmentVariable(GetSshKeyName(bbsServer)); await File.WriteAllTextAsync(Path.Join(TestHelper.GetOsDistPath(), SSH_KEY_FILE), sshKey); - var retryPolicy = new RetryPolicy(null); await retryPolicy.Retry(async () => { diff --git a/src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs b/src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs index 59b7afbb6..60070252c 100644 --- a/src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/GhesToGithub.cs @@ -47,15 +47,18 @@ public GhesToGithub(ITestOutputHelper output) ["GH_PAT"] = targetGithubToken, }; - _versionClient = new HttpClient(); + _versionClient = HttpClientFactory.CreateSrlClient(); var retryPolicy = new RetryPolicy(logger); _archiveUploader = new ArchiveUploader(_targetGithubClient, UPLOADS_URL, logger, retryPolicy); - _sourceGithubHttpClient = new HttpClient(); + // Source GitHub client (GHES) + _sourceGithubHttpClient = HttpClientFactory.CreateSrlClient(); _sourceGithubClient = new GithubClient(logger, _sourceGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), sourceGithubToken); + _archiveUploader = new ArchiveUploader(_sourceGithubClient, UPLOADS_URL, logger, retryPolicy); _sourceGithubApi = new GithubApi(_sourceGithubClient, GHES_API_URL, new RetryPolicy(logger), _archiveUploader); - _targetGithubHttpClient = new HttpClient(); + // Target GitHub client (GHEC) + _targetGithubHttpClient = HttpClientFactory.CreateSrlClient(); _targetGithubClient = new GithubClient(logger, _targetGithubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), targetGithubToken); _targetGithubApi = new GithubApi(_targetGithubClient, "https://api.github.com", new RetryPolicy(logger), _archiveUploader); @@ -93,10 +96,13 @@ await retryPolicy.Retry(async () => await _targetHelper.ResetGithubTestEnvironment(githubTargetOrg); await _sourceHelper.CreateGithubRepo(githubSourceOrg, repo1); + + // tiny pause to smooth bursts + await Task.Delay(500); + await _sourceHelper.CreateGithubRepo(githubSourceOrg, repo2); }); - // Build the command with conditional option var command = $"generate-script --github-source-org {githubSourceOrg} --github-target-org {githubTargetOrg} --ghes-api-url {GHES_API_URL} --download-migration-logs"; if (useGithubStorage) { diff --git a/src/OctoshiftCLI.IntegrationTests/GithubToGithub.cs b/src/OctoshiftCLI.IntegrationTests/GithubToGithub.cs index ef22f4cc6..3208cdf16 100644 --- a/src/OctoshiftCLI.IntegrationTests/GithubToGithub.cs +++ b/src/OctoshiftCLI.IntegrationTests/GithubToGithub.cs @@ -31,9 +31,16 @@ public GithubToGithub(ITestOutputHelper output) var githubToken = Environment.GetEnvironmentVariable("GHEC_PAT"); _tokens = new Dictionary { ["GH_PAT"] = githubToken }; - _githubHttpClient = new HttpClient(); - _versionClient = new HttpClient(); - _githubClient = new GithubClient(logger, _githubHttpClient, new VersionChecker(_versionClient, logger), new RetryPolicy(logger), new DateTimeProvider(), githubToken); + _githubHttpClient = HttpClientFactory.CreateSrlClient(); + _versionClient = HttpClientFactory.CreateSrlClient(); + _githubClient = new GithubClient( + logger, + _githubHttpClient, + new VersionChecker(_versionClient, logger), + new RetryPolicy(logger), + new DateTimeProvider(), + githubToken); + _githubApi = new GithubApi(_githubClient, "https://api.github.com", new RetryPolicy(logger), null); _helper = new TestHelper(_output, _githubApi, _githubClient); @@ -55,10 +62,16 @@ await retryPolicy.Retry(async () => await _helper.ResetGithubTestEnvironment(githubTargetOrg); await _helper.CreateGithubRepo(githubSourceOrg, repo1); + + // tiny pause to smooth bursts + await Task.Delay(500); + await _helper.CreateGithubRepo(githubSourceOrg, repo2); }); - await _helper.RunGeiCliMigration($"generate-script --github-source-org {githubSourceOrg} --github-target-org {githubTargetOrg} --download-migration-logs", _tokens); + await _helper.RunGeiCliMigration( + $"generate-script --github-source-org {githubSourceOrg} --github-target-org {githubTargetOrg} --download-migration-logs", + _tokens); _helper.AssertNoErrorInLogs(_startTime); @@ -87,7 +100,6 @@ protected virtual void Dispose(bool disposing) public void Dispose() { - // Do not change this code. Put cleanup code in 'Dispose(bool disposing)' method Dispose(disposing: true); GC.SuppressFinalize(this); } diff --git a/src/OctoshiftCLI.IntegrationTests/HttpClientFactory.cs b/src/OctoshiftCLI.IntegrationTests/HttpClientFactory.cs new file mode 100644 index 000000000..5746d4b15 --- /dev/null +++ b/src/OctoshiftCLI.IntegrationTests/HttpClientFactory.cs @@ -0,0 +1,43 @@ +using System.Diagnostics.CodeAnalysis; +using System.Net.Http; +using OctoshiftCLI.Services; + +namespace OctoshiftCLI.IntegrationTests +{ + internal static class HttpClientFactory + { + // Handlers are disposed by HttpClient(disposeHandler: true); suppress CA2000 here once. + [SuppressMessage( + "Reliability", + "CA2000:Dispose objects before losing scope", + Justification = "Ownership transferred to HttpClient via disposeHandler: true")] + internal static HttpClient CreateSrlClient() + { +#if NET6_0_OR_GREATER + var sockets = new SocketsHttpHandler + { + AutomaticDecompression = System.Net.DecompressionMethods.Deflate | System.Net.DecompressionMethods.GZip, + PooledConnectionLifetime = System.TimeSpan.FromMinutes(5), + MaxConnectionsPerServer = 2, // keep concurrency low to avoid bursty 403s + UseCookies = false + }; + var srl = new SecondaryRateLimitHandler(sockets); +#else + var inner = new HttpClientHandler + { + AutomaticDecompression = System.Net.DecompressionMethods.Deflate | System.Net.DecompressionMethods.GZip + }; + var srl = new SecondaryRateLimitHandler(inner); +#endif + var client = new HttpClient(srl, disposeHandler: true); + + // Ensure we always send a UA (GitHub requires it) — your GithubClient also sets one, but this is harmless. + if (!client.DefaultRequestHeaders.UserAgent.TryParseAdd("octoshift-cli-integration-tests")) + { + client.DefaultRequestHeaders.Add("User-Agent", "octoshift-cli-integration-tests"); + } + + return client; + } + } +}