Skip to content

Commit f6cbdbd

Browse files
authored
Merge pull request #3193 from erri120/fix/download-issues
More http download fixes
2 parents f6b15d0 + 1c7e217 commit f6cbdbd

File tree

7 files changed

+113
-43
lines changed

7 files changed

+113
-43
lines changed

src/NexusMods.Collections/CollectionDownloader.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using NexusMods.Abstractions.NexusWebApi;
1616
using NexusMods.Abstractions.NexusWebApi.Types;
1717
using NexusMods.Abstractions.NexusWebApi.Types.V2;
18+
using NexusMods.Abstractions.Settings;
1819
using NexusMods.Abstractions.Telemetry;
1920
using NexusMods.CrossPlatform.Process;
2021
using NexusMods.Extensions.BCL;
@@ -256,7 +257,6 @@ public async ValueTask DownloadItems(
256257
CollectionRevisionMetadata.ReadOnly revisionMetadata,
257258
ItemType itemType,
258259
IDb db,
259-
int maxDegreeOfParallelism = -1,
260260
CancellationToken cancellationToken = default)
261261
{
262262
var job = new DownloadCollectionJob
@@ -266,7 +266,7 @@ public async ValueTask DownloadItems(
266266
RevisionMetadata = revisionMetadata,
267267
Db = db,
268268
ItemType = itemType,
269-
MaxDegreeOfParallelism = maxDegreeOfParallelism,
269+
MaxDegreeOfParallelism = _serviceProvider.GetRequiredService<ISettingsManager>().Get<DownloadSettings>().MaxParallelDownloads,
270270
};
271271

272272
await _jobMonitor.Begin<DownloadCollectionJob, R3.Unit>(job);

src/NexusMods.Collections/DownloadCollectionJob.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public class DownloadCollectionJob : IJobDefinitionWithStart<DownloadCollectionJ
1212
public required CollectionDownloader.ItemType ItemType { get; init; }
1313
public required CollectionDownloader Downloader { get; init; }
1414
public required IDb Db { get; init; }
15-
public int MaxDegreeOfParallelism { get; init; } = -1;
15+
public required int MaxDegreeOfParallelism { get; init; }
1616

1717
public async ValueTask<R3.Unit> StartAsync(IJobContext<DownloadCollectionJob> context)
1818
{
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
using NexusMods.Abstractions.Settings;
2+
3+
namespace NexusMods.Collections;
4+
5+
public class DownloadSettings : ISettings
6+
{
7+
public int MaxParallelDownloads { get; set; } = Environment.ProcessorCount;
8+
9+
public static ISettingsBuilder Configure(ISettingsBuilder settingsBuilder)
10+
{
11+
return settingsBuilder.AddToUI<DownloadSettings>(builder => builder
12+
.AddPropertyToUI(x => x.MaxParallelDownloads, propertyBuilder => propertyBuilder
13+
.AddToSection(Sections.General)
14+
.WithDisplayName("Max Parallel Downloads")
15+
.WithDescription("Set the maximum number of downloads that can happen in parallel when downloading collections")
16+
.UseSingleValueMultipleChoiceContainer(
17+
valueComparer: EqualityComparer<int>.Default,
18+
allowedValues: Enumerable.Range(start: 1, Environment.ProcessorCount).ToArray(),
19+
valueToDisplayString: static i => i.ToString()
20+
)
21+
)
22+
);
23+
}
24+
}

src/NexusMods.Collections/Services.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using NexusMods.Abstractions.Collections;
33
using NexusMods.Abstractions.Loadouts;
44
using NexusMods.Abstractions.NexusModsLibrary;
5+
using NexusMods.Abstractions.Settings;
56

67
namespace NexusMods.Collections;
78

@@ -16,6 +17,7 @@ public static IServiceCollection AddNexusModsCollections(this IServiceCollection
1617
.AddNexusCollectionItemLoadoutGroupModel()
1718
.AddNexusCollectionReplicatedLoadoutGroupModel()
1819
.AddCollectionVerbs()
19-
.AddSingleton<CollectionDownloader>();
20+
.AddSingleton<CollectionDownloader>()
21+
.AddSettings<DownloadSettings>();
2022
}
2123
}

src/NexusMods.Networking.HttpDownloader/HttpDownloadJob.cs

Lines changed: 50 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
using System.Diagnostics.CodeAnalysis;
1+
using System.Collections.Immutable;
22
using System.Net;
33
using System.Net.Http.Headers;
4+
using System.Net.Sockets;
45
using DynamicData.Kernel;
56
using JetBrains.Annotations;
67
using Microsoft.Extensions.DependencyInjection;
7-
using Microsoft.Extensions.Http.Resilience;
88
using Microsoft.Extensions.Logging;
99
using NexusMods.Abstractions.HttpDownloads;
1010
using NexusMods.Abstractions.Jobs;
1111
using NexusMods.Abstractions.Library.Models;
12-
using NexusMods.App.BuildInfo;
1312
using NexusMods.MnemonicDB.Abstractions;
1413
using NexusMods.Paths;
1514
using Polly;
15+
using Polly.Retry;
1616

1717
namespace NexusMods.Networking.HttpDownloader;
1818

@@ -22,9 +22,12 @@ namespace NexusMods.Networking.HttpDownloader;
2222
[PublicAPI]
2323
public record HttpDownloadJob : IJobDefinitionWithStart<HttpDownloadJob, AbsolutePath>, IHttpDownloadJob
2424
{
25-
#pragma warning disable EXTEXP0001
26-
private static readonly HttpClient Client = BuildClient();
27-
#pragma warning restore EXTEXP0001
25+
private static readonly ResiliencePipeline<AbsolutePath> ResiliencePipeline = BuildResiliencePipeline();
26+
27+
/// <summary>
28+
/// Client.
29+
/// </summary>
30+
public required HttpClient Client { get; init; }
2831

2932
/// <summary>
3033
/// Logger.
@@ -76,15 +79,29 @@ public static IJobTask<HttpDownloadJob, AbsolutePath> Create(
7679
DownloadPageUri = downloadPage,
7780
Destination = destination,
7881
Logger = provider.GetRequiredService<ILogger<HttpDownloadJob>>(),
82+
Client = provider.GetRequiredService<HttpClient>(),
7983
};
8084

8185
return monitor.Begin<HttpDownloadJob, AbsolutePath>(job);
8286
}
8387

84-
/// <summary>
85-
/// Execute the job
86-
/// </summary>
88+
/// <inheritdoc/>
8789
public async ValueTask<AbsolutePath> StartAsync(IJobContext<HttpDownloadJob> context)
90+
{
91+
var result = await ResiliencePipeline.ExecuteAsync(
92+
callback: static (tuple, _) =>
93+
{
94+
var (self, context) = tuple;
95+
return self.StartAsyncImpl(context);
96+
},
97+
state: (this, context),
98+
cancellationToken: context.CancellationToken
99+
);
100+
101+
return result;
102+
}
103+
104+
private async ValueTask<AbsolutePath> StartAsyncImpl(IJobContext<HttpDownloadJob> context)
88105
{
89106
await context.YieldAsync();
90107
await FetchMetadata(context);
@@ -168,6 +185,11 @@ public async ValueTask<AbsolutePath> StartAsync(IJobContext<HttpDownloadJob> con
168185
{
169186
await response.Content.CopyToAsync(outputStream, context.CancellationToken);
170187
}
188+
catch (Exception e)
189+
{
190+
Logger.LogWarning(e, "Exception while downloading from `{PageUri}`, downloaded `{DownloadedBytes}` from `{TotalBytes}` bytes", DownloadPageUri, outputStream.Position, outputStream.Length);
191+
throw;
192+
}
171193
finally
172194
{
173195
TotalBytesDownloaded = Size.FromLong(outputStream.Position);
@@ -244,36 +266,27 @@ private async ValueTask FetchMetadata(IJobContext context)
244266
var contentLength = response.Content.Headers.ContentLength;
245267
ContentLength = contentLength is not null ? Size.FromLong(contentLength.Value) : Optional<Size>.None;
246268
}
247-
248-
[Experimental("EXTEXP0001")]
249-
private static HttpClient BuildClient()
250-
{
251-
// TODO: get values from settings, probably make this a singleton
252269

253-
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
254-
.AddRetry(new HttpRetryStrategyOptions())
255-
.Build();
256-
257-
HttpMessageHandler handler = new ResilienceHandler(pipeline)
258-
{
259-
InnerHandler = new SocketsHttpHandler
270+
private static ResiliencePipeline<AbsolutePath> BuildResiliencePipeline()
271+
{
272+
ImmutableArray<Type> networkExceptions =
273+
[
274+
typeof(HttpIOException),
275+
typeof(HttpRequestException),
276+
typeof(SocketException),
277+
];
278+
279+
var pipeline = new ResiliencePipelineBuilder<AbsolutePath>()
280+
.AddRetry(new RetryStrategyOptions<AbsolutePath>
260281
{
261-
ConnectTimeout = TimeSpan.FromSeconds(30),
262-
KeepAlivePingPolicy = HttpKeepAlivePingPolicy.WithActiveRequests,
263-
KeepAlivePingDelay = TimeSpan.FromSeconds(5),
264-
KeepAlivePingTimeout = TimeSpan.FromSeconds(20),
265-
},
266-
};
267-
268-
var client = new HttpClient(handler)
269-
{
270-
Timeout = TimeSpan.FromSeconds(20),
271-
DefaultRequestVersion = HttpVersion.Version11,
272-
DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher,
273-
};
274-
275-
client.DefaultRequestHeaders.UserAgent.ParseAdd(ApplicationConstants.UserAgent);
282+
ShouldHandle = args => ValueTask.FromResult(args.Outcome.Exception is not null && networkExceptions.Contains(args.Outcome.Exception.GetType())),
283+
BackoffType = DelayBackoffType.Exponential,
284+
UseJitter = true,
285+
MaxRetryAttempts = 3,
286+
Delay = TimeSpan.FromSeconds(3),
287+
})
288+
.Build();
276289

277-
return client;
290+
return pipeline;
278291
}
279292
}

src/NexusMods.Networking.HttpDownloader/Services.cs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
using System.Net;
12
using Microsoft.Extensions.DependencyInjection;
3+
using Microsoft.Extensions.Http.Resilience;
24
using NexusMods.App.BuildInfo;
5+
using Polly;
36

47
namespace NexusMods.Networking.HttpDownloader;
58

@@ -12,9 +15,36 @@ public static IServiceCollection AddHttpDownloader(this IServiceCollection servi
1215
{
1316
return services.AddSingleton<HttpClient>(_ =>
1417
{
15-
var client = new HttpClient();
16-
client.DefaultRequestHeaders.UserAgent.ParseAdd(ApplicationConstants.UserAgent);
18+
var client = BuildClient();
1719
return client;
1820
});
1921
}
22+
23+
private static HttpClient BuildClient()
24+
{
25+
var pipeline = new ResiliencePipelineBuilder<HttpResponseMessage>()
26+
.AddRetry(new HttpRetryStrategyOptions
27+
{
28+
BackoffType = DelayBackoffType.Exponential,
29+
MaxRetryAttempts = 3,
30+
Delay = TimeSpan.FromSeconds(3),
31+
UseJitter = true,
32+
})
33+
.Build();
34+
35+
HttpMessageHandler handler = new ResilienceHandler(pipeline)
36+
{
37+
InnerHandler = new SocketsHttpHandler(),
38+
};
39+
40+
var client = new HttpClient(handler)
41+
{
42+
DefaultRequestVersion = HttpVersion.Version11,
43+
DefaultVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher,
44+
};
45+
46+
client.DefaultRequestHeaders.UserAgent.ParseAdd(ApplicationConstants.UserAgent);
47+
48+
return client;
49+
}
2050
}

src/NexusMods.Networking.NexusWebApi/ExternalDownloadJob.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ public static IJobTask<ExternalDownloadJob, AbsolutePath> Create(IServiceProvide
5151
DownloadPageUri = uri,
5252
Destination = tempFileManager.CreateFile(),
5353
Uri = uri,
54+
Client = provider.GetRequiredService<HttpClient>(),
5455
};
5556

5657
return monitor.Begin<ExternalDownloadJob, AbsolutePath>(job);

0 commit comments

Comments
 (0)