From bb81cf06ae2599259a77634758b44d9aa8326402 Mon Sep 17 00:00:00 2001 From: Oleksii Holub <1935960+Tyrrrz@users.noreply.github.com> Date: Tue, 12 Apr 2022 23:55:03 +0300 Subject: [PATCH] Rework retry policy Fixes #832 --- .../Discord/DiscordClient.cs | 3 +- .../Exporting/MediaDownloader.cs | 63 +++++++++---------- .../Utils/Extensions/ExceptionExtensions.cs | 44 +++++++++++++ DiscordChatExporter.Core/Utils/Http.cs | 50 +++++++-------- 4 files changed, 96 insertions(+), 64 deletions(-) create mode 100644 DiscordChatExporter.Core/Utils/Extensions/ExceptionExtensions.cs diff --git a/DiscordChatExporter.Core/Discord/DiscordClient.cs b/DiscordChatExporter.Core/Discord/DiscordClient.cs index 932bc16..6086728 100644 --- a/DiscordChatExporter.Core/Discord/DiscordClient.cs +++ b/DiscordChatExporter.Core/Discord/DiscordClient.cs @@ -3,7 +3,6 @@ using System.Collections.Generic; using System.Linq; using System.Net; using System.Net.Http; -using System.Net.Http.Headers; using System.Runtime.CompilerServices; using System.Text.Json; using System.Threading; @@ -52,7 +51,7 @@ public class DiscordClient string url, CancellationToken cancellationToken = default) { - return await Http.ResponsePolicy.ExecuteAsync(async innerCancellationToken => + return await Http.ResponseResiliencePolicy.ExecuteAsync(async innerCancellationToken => { if (_tokenKind == TokenKind.User) return await GetResponseAsync(url, false, innerCancellationToken); diff --git a/DiscordChatExporter.Core/Exporting/MediaDownloader.cs b/DiscordChatExporter.Core/Exporting/MediaDownloader.cs index b648489..1fa5fc5 100644 --- a/DiscordChatExporter.Core/Exporting/MediaDownloader.cs +++ b/DiscordChatExporter.Core/Exporting/MediaDownloader.cs @@ -35,45 +35,42 @@ internal partial class MediaDownloader var filePath = Path.Combine(_workingDirPath, fileName); // Reuse existing files if we're allowed to - if (_reuseMedia && File.Exists(filePath)) - return _pathCache[url] = filePath; - - Directory.CreateDirectory(_workingDirPath); - - // This retries on IOExceptions which is dangerous as we're also working with files - await Http.ExceptionPolicy.ExecuteAsync(async () => + if (!_reuseMedia || !File.Exists(filePath)) { - // Download the file - using var response = await Http.Client.GetAsync(url, cancellationToken); - await using (var output = File.Create(filePath)) - { - await response.Content.CopyToAsync(output, cancellationToken); - } + Directory.CreateDirectory(_workingDirPath); - // Try to set the file date according to the last-modified header - try + await Http.ResiliencePolicy.ExecuteAsync(async () => { - var lastModified = response.Content.Headers.TryGetValue("Last-Modified")?.Pipe(s => - DateTimeOffset.TryParse(s, CultureInfo.InvariantCulture, DateTimeStyles.None, out var date) - ? date - : (DateTimeOffset?) null - ); + // Download the file + using var response = await Http.Client.GetAsync(url, cancellationToken); + await using (var output = File.Create(filePath)) + await response.Content.CopyToAsync(output, cancellationToken); - if (lastModified is not null) + // Try to set the file date according to the last-modified header + try { - File.SetCreationTimeUtc(filePath, lastModified.Value.UtcDateTime); - File.SetLastWriteTimeUtc(filePath, lastModified.Value.UtcDateTime); - File.SetLastAccessTimeUtc(filePath, lastModified.Value.UtcDateTime); + var lastModified = response.Content.Headers.TryGetValue("Last-Modified")?.Pipe(s => + DateTimeOffset.TryParse(s, CultureInfo.InvariantCulture, DateTimeStyles.None, out var date) + ? date + : (DateTimeOffset?) null + ); + + if (lastModified is not null) + { + File.SetCreationTimeUtc(filePath, lastModified.Value.UtcDateTime); + File.SetLastWriteTimeUtc(filePath, lastModified.Value.UtcDateTime); + File.SetLastAccessTimeUtc(filePath, lastModified.Value.UtcDateTime); + } } - } - catch - { - // This can apparently fail for some reason. - // https://github.com/Tyrrrz/DiscordChatExporter/issues/585 - // Updating file dates is not a critical task, so we'll just - // ignore exceptions thrown here. - } - }); + catch + { + // This can apparently fail for some reason. + // https://github.com/Tyrrrz/DiscordChatExporter/issues/585 + // Updating file dates is not a critical task, so we'll just + // ignore exceptions thrown here. + } + }); + } return _pathCache[url] = filePath; } diff --git a/DiscordChatExporter.Core/Utils/Extensions/ExceptionExtensions.cs b/DiscordChatExporter.Core/Utils/Extensions/ExceptionExtensions.cs new file mode 100644 index 0000000..3515559 --- /dev/null +++ b/DiscordChatExporter.Core/Utils/Extensions/ExceptionExtensions.cs @@ -0,0 +1,44 @@ +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Net; +using System.Net.Http; +using System.Text.RegularExpressions; + +namespace DiscordChatExporter.Core.Utils.Extensions; + +public static class ExceptionExtensions +{ + private static void PopulateChildren(this Exception exception, ICollection children) + { + if (exception is AggregateException aggregateException) + { + foreach (var innerException in aggregateException.InnerExceptions) + { + children.Add(innerException); + PopulateChildren(innerException, children); + } + } + else if (exception.InnerException is not null) + { + children.Add(exception.InnerException); + PopulateChildren(exception.InnerException, children); + } + } + + public static IReadOnlyList GetSelfAndChildren(this Exception exception) + { + var children = new List {exception}; + PopulateChildren(exception, children); + return children; + } + + public static HttpStatusCode? TryGetStatusCode(this HttpRequestException ex) => + // This is extremely frail, but there's no other way + Regex + .Match(ex.Message, @": (\d+) \(") + .Groups[1] + .Value + .NullIfWhiteSpace()? + .Pipe(s => (HttpStatusCode) int.Parse(s, CultureInfo.InvariantCulture)); +} \ No newline at end of file diff --git a/DiscordChatExporter.Core/Utils/Http.cs b/DiscordChatExporter.Core/Utils/Http.cs index 9de8107..3ec0ff9 100644 --- a/DiscordChatExporter.Core/Utils/Http.cs +++ b/DiscordChatExporter.Core/Utils/Http.cs @@ -1,9 +1,8 @@ using System; -using System.Globalization; -using System.IO; +using System.Linq; using System.Net; using System.Net.Http; -using System.Text.RegularExpressions; +using System.Net.Sockets; using System.Threading.Tasks; using DiscordChatExporter.Core.Utils.Extensions; using Polly; @@ -14,13 +13,26 @@ public static class Http { public static HttpClient Client { get; } = new(); - public static IAsyncPolicy ResponsePolicy { get; } = + private static bool IsRetryableStatusCode(HttpStatusCode statusCode) => statusCode is + HttpStatusCode.TooManyRequests or + HttpStatusCode.RequestTimeout or + HttpStatusCode.InternalServerError; + + private static bool IsRetryableException(Exception exception) => + exception.GetSelfAndChildren().Any(ex => + ex is TimeoutException or SocketException || + ex is HttpRequestException hrex && IsRetryableStatusCode(hrex.TryGetStatusCode() ?? HttpStatusCode.OK) + ); + + public static IAsyncPolicy ResiliencePolicy { get; } = + Policy + .Handle(IsRetryableException) + .WaitAndRetryAsync(4, i => TimeSpan.FromSeconds(Math.Pow(2, i) + 1)); + + public static IAsyncPolicy ResponseResiliencePolicy { get; } = Policy - .Handle() - .Or() - .OrResult(m => m.StatusCode == HttpStatusCode.TooManyRequests) - .OrResult(m => m.StatusCode == HttpStatusCode.RequestTimeout) - .OrResult(m => m.StatusCode >= HttpStatusCode.InternalServerError) + .Handle(IsRetryableException) + .OrResult(m => IsRetryableStatusCode(m.StatusCode)) .WaitAndRetryAsync( 8, (i, result, _) => @@ -43,24 +55,4 @@ public static class Http }, (_, _, _, _) => Task.CompletedTask ); - - private static HttpStatusCode? TryGetStatusCodeFromException(HttpRequestException ex) => - // This is extremely frail, but there's no other way - Regex - .Match(ex.Message, @": (\d+) \(") - .Groups[1] - .Value - .NullIfWhiteSpace()? - .Pipe(s => (HttpStatusCode) int.Parse(s, CultureInfo.InvariantCulture)); - - public static IAsyncPolicy ExceptionPolicy { get; } = - Policy - .Handle() // dangerous - .Or(ex => - TryGetStatusCodeFromException(ex) is - HttpStatusCode.TooManyRequests or - HttpStatusCode.RequestTimeout or - HttpStatusCode.InternalServerError - ) - .WaitAndRetryAsync(4, i => TimeSpan.FromSeconds(Math.Pow(2, i) + 1)); } \ No newline at end of file