Skip to content

Commit a090b2d

Browse files
jamesmcroftjamesmcroft
authored
fix: resolve Timer.Start dueTime bugs and hardening (#363)
* fix: resolve Timer.Start dueTime bugs and v3.0 hardening Fix Timer.Start(TimeSpan) using .Milliseconds (component only) instead of .TotalMilliseconds, and both overloads not storing the dueTime parameter to the DueTime property. The int overload also used the stale DueTime value in the else branch instead of the parameter. Add Timer tests covering all Start overloads, stop, and dispose. Add test projects for MADE.Foundation and MADE.Runtime. Fix nullable annotations in MADE.Networking and add async safety to AppDiagnostics event handlers. Remove x64/x86 platform configurations from solution. * fix: address PR review feedback from Copilot and CodeQL Replace generic catch clauses with catch (Exception) in AppDiagnostics and NetworkRequestManager. Remove redundant null check on successCallback in NetworkRequestManager catch block. Guard nullable Data property in Post/Put/Patch requests with fallback to empty string. Add null checks after JsonSerializer.Deserialize in all JSON request types. Use Assume.That for non-deterministic GC tests in WeakReferenceCallbackTests. Remove unused detached variable and add explicit assertion in WeakReferenceEventListenerWithEventArgsTests. Move helper types into PlatformApiHelperTests as private nested classes. Restore solution file BOM and remove leading blank line. Update misleading catch comment in timer callback.
1 parent 5488d1f commit a090b2d

27 files changed

Lines changed: 962 additions & 86 deletions

‎MADE.NET.sln‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
2-
Microsoft Visual Studio Solution File, Format Version 12.00
1+
Microsoft Visual Studio Solution File, Format Version 12.00
32
# Visual Studio Version 17
43
VisualStudioVersion = 17.1.32228.430
54
MinimumVisualStudioVersion = 10.0.40219.1
@@ -59,6 +58,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MADE.Testing.Tests", "tests
5958
EndProject
6059
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MADE.Web.Mvc.Tests", "tests\MADE.Web.Mvc.Tests\MADE.Web.Mvc.Tests.csproj", "{F994F941-474A-4FDD-A9CB-280EB0D78407}"
6160
EndProject
61+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MADE.Foundation.Tests", "tests\MADE.Foundation.Tests\MADE.Foundation.Tests.csproj", "{EE7B8716-5B54-45EC-91AD-4764F4AC863B}"
62+
EndProject
63+
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "MADE.Runtime.Tests", "tests\MADE.Runtime.Tests\MADE.Runtime.Tests.csproj", "{D360BA13-4DAD-4C62-AE4A-737553F34E01}"
64+
EndProject
6265
Global
6366
GlobalSection(SolutionConfigurationPlatforms) = preSolution
6467
Debug|Any CPU = Debug|Any CPU
@@ -169,6 +172,14 @@ Global
169172
{F994F941-474A-4FDD-A9CB-280EB0D78407}.Debug|Any CPU.Build.0 = Debug|Any CPU
170173
{F994F941-474A-4FDD-A9CB-280EB0D78407}.Release|Any CPU.ActiveCfg = Release|Any CPU
171174
{F994F941-474A-4FDD-A9CB-280EB0D78407}.Release|Any CPU.Build.0 = Release|Any CPU
175+
{EE7B8716-5B54-45EC-91AD-4764F4AC863B}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
176+
{EE7B8716-5B54-45EC-91AD-4764F4AC863B}.Debug|Any CPU.Build.0 = Debug|Any CPU
177+
{EE7B8716-5B54-45EC-91AD-4764F4AC863B}.Release|Any CPU.ActiveCfg = Release|Any CPU
178+
{EE7B8716-5B54-45EC-91AD-4764F4AC863B}.Release|Any CPU.Build.0 = Release|Any CPU
179+
{D360BA13-4DAD-4C62-AE4A-737553F34E01}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
180+
{D360BA13-4DAD-4C62-AE4A-737553F34E01}.Debug|Any CPU.Build.0 = Debug|Any CPU
181+
{D360BA13-4DAD-4C62-AE4A-737553F34E01}.Release|Any CPU.ActiveCfg = Release|Any CPU
182+
{D360BA13-4DAD-4C62-AE4A-737553F34E01}.Release|Any CPU.Build.0 = Release|Any CPU
172183
EndGlobalSection
173184
GlobalSection(SolutionProperties) = preSolution
174185
HideSolutionNode = FALSE
@@ -200,6 +211,8 @@ Global
200211
{865FBD49-C64B-4B36-AEFC-FD960DDC4CF8} = {69149D0F-BB09-411B-88F0-A1E845058D70}
201212
{40B5F4EB-45DD-410A-B0FB-2384C863FC33} = {69149D0F-BB09-411B-88F0-A1E845058D70}
202213
{F994F941-474A-4FDD-A9CB-280EB0D78407} = {69149D0F-BB09-411B-88F0-A1E845058D70}
214+
{EE7B8716-5B54-45EC-91AD-4764F4AC863B} = {69149D0F-BB09-411B-88F0-A1E845058D70}
215+
{D360BA13-4DAD-4C62-AE4A-737553F34E01} = {69149D0F-BB09-411B-88F0-A1E845058D70}
203216
EndGlobalSection
204217
GlobalSection(ExtensibilityGlobals) = postSolution
205218
SolutionGuid = {3921AD86-E6C0-4436-8880-2D9EDFAD6151}

‎src/MADE.Diagnostics/AppDiagnostics.cs‎

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -78,38 +78,52 @@ public void StopRecordingDiagnostics()
7878

7979
private async void OnTaskUnobservedException(object? sender, UnobservedTaskExceptionEventArgs args)
8080
{
81-
args.SetObserved();
82-
83-
var correlationId = Guid.NewGuid();
81+
try
82+
{
83+
args.SetObserved();
8484

85-
await this.EventLogger.WriteCritical(
86-
args.Exception != null
87-
? $"An unobserved task exception was thrown. Correlation ID: {correlationId}. Error: {args.Exception}."
88-
: $"An unobserved task exception was thrown. Correlation ID: {correlationId}. Error: No exception information was available.").ConfigureAwait(false);
85+
var correlationId = Guid.NewGuid();
8986

90-
if (args.Exception != null)
87+
await this.EventLogger.WriteCritical(
88+
args.Exception != null
89+
? $"An unobserved task exception was thrown. Correlation ID: {correlationId}. Error: {args.Exception}."
90+
: $"An unobserved task exception was thrown. Correlation ID: {correlationId}. Error: No exception information was available.").ConfigureAwait(false);
91+
92+
if (args.Exception != null)
93+
{
94+
this.ExceptionObserved?.Invoke(this, new ExceptionObservedEventArgs(correlationId, args.Exception));
95+
}
96+
}
97+
catch (Exception)
9198
{
92-
this.ExceptionObserved?.Invoke(this, new ExceptionObservedEventArgs(correlationId, args.Exception));
99+
// Swallow exceptions in last-resort exception handlers to prevent crashing the process.
93100
}
94101
}
95102

96103
private async void OnAppUnhandledException(object sender, UnhandledExceptionEventArgs args)
97104
{
98-
if (args.IsTerminating)
105+
try
99106
{
100-
await this.EventLogger.WriteCritical(
101-
"The application is terminating due to an unhandled exception being thrown.").ConfigureAwait(false);
102-
}
107+
if (args.IsTerminating)
108+
{
109+
await this.EventLogger.WriteCritical(
110+
"The application is terminating due to an unhandled exception being thrown.").ConfigureAwait(false);
111+
}
103112

104-
if (args.ExceptionObject is not Exception ex)
105-
{
106-
return;
107-
}
113+
if (args.ExceptionObject is not Exception ex)
114+
{
115+
return;
116+
}
108117

109-
var correlationId = Guid.NewGuid();
118+
var correlationId = Guid.NewGuid();
110119

111-
await this.EventLogger.WriteCritical($"An unhandled exception was thrown. Correlation ID: {correlationId}. Error: {ex}").ConfigureAwait(false);
120+
await this.EventLogger.WriteCritical($"An unhandled exception was thrown. Correlation ID: {correlationId}. Error: {ex}").ConfigureAwait(false);
112121

113-
this.ExceptionObserved?.Invoke(this, new ExceptionObservedEventArgs(correlationId, ex));
122+
this.ExceptionObserved?.Invoke(this, new ExceptionObservedEventArgs(correlationId, ex));
123+
}
124+
catch (Exception)
125+
{
126+
// Swallow exceptions in last-resort exception handlers to prevent crashing the process.
127+
}
114128
}
115129
}

‎src/MADE.Networking/Extensions/UriExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public static class UriExtensions
1717
/// <param name="uri">The <see cref="Uri"/> to extract a query value from.</param>
1818
/// <param name="queryParam">The key of the parameter in the query to extract the value for.</param>
1919
/// <returns>The value for the query parameter.</returns>
20-
public static string GetQueryValue(this Uri uri, string queryParam)
20+
public static string? GetQueryValue(this Uri uri, string queryParam)
2121
{
2222
NameValueCollection queryDictionary = System.Web.HttpUtility.ParseQueryString(uri.Query);
2323
return queryDictionary.Get(queryParam);

‎src/MADE.Networking/Http/INetworkRequestManager.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// See the LICENSE file in the project root for more information.
33

44
using System.Collections.Concurrent;
5+
using System.Threading.Tasks;
56
using MADE.Networking.Http.Requests;
67

78
namespace MADE.Networking.Http;
@@ -100,5 +101,6 @@ void AddOrUpdate<TRequest, TResponse, TErrorResponse>(
100101
/// <summary>
101102
/// Processes the current queue of network requests.
102103
/// </summary>
103-
void ProcessCurrentQueue();
104+
/// <returns>An asynchronous operation.</returns>
105+
Task ProcessCurrentQueueAsync();
104106
}

‎src/MADE.Networking/Http/NetworkRequestFactory.cs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,43 +33,43 @@ private NetworkRequestFactory(IHttpClientFactory httpClientFactory, string? clie
3333
/// <inheritdoc/>
3434
public JsonGetNetworkRequest Get(string url, Dictionary<string, string>? headers = null)
3535
{
36-
return new JsonGetNetworkRequest(this.CreateClient(), url, headers!);
36+
return new JsonGetNetworkRequest(this.CreateClient(), url, headers);
3737
}
3838

3939
/// <inheritdoc/>
4040
public JsonPostNetworkRequest Post(string url, string? jsonData = null, Dictionary<string, string>? headers = null)
4141
{
42-
return new JsonPostNetworkRequest(this.CreateClient(), url, jsonData!, headers!);
42+
return new JsonPostNetworkRequest(this.CreateClient(), url, jsonData, headers);
4343
}
4444

4545
/// <inheritdoc/>
4646
public JsonPutNetworkRequest Put(string url, string? jsonData = null, Dictionary<string, string>? headers = null)
4747
{
48-
return new JsonPutNetworkRequest(this.CreateClient(), url, jsonData!, headers!);
48+
return new JsonPutNetworkRequest(this.CreateClient(), url, jsonData, headers);
4949
}
5050

5151
/// <inheritdoc/>
5252
public JsonPatchNetworkRequest Patch(string url, string? jsonData = null, Dictionary<string, string>? headers = null)
5353
{
54-
return new JsonPatchNetworkRequest(this.CreateClient(), url, jsonData!, headers!);
54+
return new JsonPatchNetworkRequest(this.CreateClient(), url, jsonData, headers);
5555
}
5656

5757
/// <inheritdoc/>
5858
public JsonDeleteNetworkRequest Delete(string url, Dictionary<string, string>? headers = null)
5959
{
60-
return new JsonDeleteNetworkRequest(this.CreateClient(), url, headers!);
60+
return new JsonDeleteNetworkRequest(this.CreateClient(), url, headers);
6161
}
6262

6363
/// <inheritdoc/>
6464
public StreamGetNetworkRequest GetStream(string url, Dictionary<string, string>? headers = null)
6565
{
66-
return new StreamGetNetworkRequest(this.CreateClient(), url, headers!);
66+
return new StreamGetNetworkRequest(this.CreateClient(), url, headers);
6767
}
6868

6969
/// <inheritdoc/>
7070
public MultipartFormDataPostNetworkRequest PostMultipart(string url, Dictionary<string, string>? headers = null)
7171
{
72-
return new MultipartFormDataPostNetworkRequest(this.CreateClient(), url, headers!);
72+
return new MultipartFormDataPostNetworkRequest(this.CreateClient(), url, headers);
7373
}
7474

7575
/// <inheritdoc/>

‎src/MADE.Networking/Http/NetworkRequestManager.cs‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public void Dispose()
8484
/// <summary>
8585
/// Processes the current queue of network requests.
8686
/// </summary>
87-
public void ProcessCurrentQueue()
87+
/// <returns>An asynchronous operation.</returns>
88+
public async Task ProcessCurrentQueueAsync()
8889
{
8990
if (this.CurrentQueue.Count == 0 || this.isProcessingRequests)
9091
{
@@ -102,13 +103,13 @@ public void ProcessCurrentQueue()
102103
{
103104
if (this.CurrentQueue.TryRemove(
104105
this.CurrentQueue.FirstOrDefault().Key,
105-
out NetworkRequestCallback request))
106+
out NetworkRequestCallback? request))
106107
{
107108
requestTasks.Add(ExecuteRequestsAsync(this.CurrentQueue, request, cts.Token));
108109
}
109110
}
110111

111-
Task.WhenAll(requestTasks).GetAwaiter().GetResult();
112+
await Task.WhenAll(requestTasks).ConfigureAwait(false);
112113
}
113114
finally
114115
{
@@ -220,23 +221,37 @@ private static async Task ExecuteRequestsAsync(
220221
}
221222

222223
NetworkRequest request = requestCallback.Request;
223-
WeakReferenceCallback successCallback = requestCallback.SuccessCallback;
224-
WeakReferenceCallback errorCallback = requestCallback.ErrorCallback;
224+
WeakReferenceCallback? successCallback = requestCallback.SuccessCallback;
225+
WeakReferenceCallback? errorCallback = requestCallback.ErrorCallback;
225226

226227
try
227228
{
229+
if (successCallback is null)
230+
{
231+
return;
232+
}
233+
228234
object response = await request.ExecuteAsync(successCallback.Type, cancellationToken).ConfigureAwait(false);
229235
successCallback.Invoke(response);
230236
}
231237
catch (Exception ex)
232238
{
233-
successCallback.Invoke(Activator.CreateInstance(successCallback.Type));
239+
successCallback.Invoke(Activator.CreateInstance(successCallback.Type)!);
240+
234241
errorCallback?.Invoke(ex);
235242
}
236243
}
237244

238-
private void OnProcessTimerTick(object sender, object e)
245+
private async void OnProcessTimerTick(object sender, object e)
239246
{
240-
this.ProcessCurrentQueue();
247+
try
248+
{
249+
await this.ProcessCurrentQueueAsync().ConfigureAwait(false);
250+
}
251+
catch (Exception)
252+
{
253+
// Swallow exceptions in the async void timer callback to prevent them from
254+
// escaping as unhandled exceptions and crashing the process.
255+
}
241256
}
242257
}

‎src/MADE.Networking/Http/Requests/Json/JsonDeleteNetworkRequest.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ namespace MADE.Networking.Http.Requests.Json;
1515
/// </summary>
1616
public sealed class JsonDeleteNetworkRequest : NetworkRequest
1717
{
18+
private static readonly JsonSerializerOptions DefaultJsonOptions = new() { PropertyNameCaseInsensitive = true };
19+
1820
private readonly HttpClient client;
1921

2022
/// <summary>
@@ -43,7 +45,7 @@ public JsonDeleteNetworkRequest(HttpClient client, string url)
4345
/// <param name="headers">
4446
/// The additional headers.
4547
/// </param>
46-
public JsonDeleteNetworkRequest(HttpClient client, string url, Dictionary<string, string> headers)
48+
public JsonDeleteNetworkRequest(HttpClient client, string url, Dictionary<string, string>? headers)
4749
: base(url, headers)
4850
{
4951
this.client = client ?? throw new ArgumentNullException(nameof(client));
@@ -64,7 +66,8 @@ public JsonDeleteNetworkRequest(HttpClient client, string url, Dictionary<string
6466
public override async Task<TResponse> ExecuteAsync<TResponse>(CancellationToken cancellationToken = default)
6567
{
6668
string json = await this.GetJsonResponseAsync(cancellationToken).ConfigureAwait(false);
67-
return JsonSerializer.Deserialize<TResponse>(json, new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
69+
return JsonSerializer.Deserialize<TResponse>(json, DefaultJsonOptions)
70+
?? throw new InvalidOperationException($"Failed to deserialize response to {typeof(TResponse).Name}.");
6871
}
6972

7073
/// <summary>
@@ -84,7 +87,8 @@ public override async Task<object> ExecuteAsync(
8487
CancellationToken cancellationToken = default)
8588
{
8689
string json = await this.GetJsonResponseAsync(cancellationToken).ConfigureAwait(false);
87-
return JsonSerializer.Deserialize(json, expectedResponse, new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
90+
return JsonSerializer.Deserialize(json, expectedResponse, DefaultJsonOptions)
91+
?? throw new InvalidOperationException($"Failed to deserialize response to {expectedResponse.Name}.");
8892
}
8993

9094
private async Task<string> GetJsonResponseAsync(CancellationToken cancellationToken = default)
@@ -118,6 +122,6 @@ private async Task<string> GetJsonResponseAsync(CancellationToken cancellationTo
118122

119123
response.EnsureSuccessStatusCode();
120124

121-
return await response.Content.ReadAsStringAsync().ConfigureAwait(false);
125+
return await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
122126
}
123127
}

‎src/MADE.Networking/Http/Requests/Json/JsonGetNetworkRequest.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ namespace MADE.Networking.Http.Requests.Json;
1515
/// </summary>
1616
public sealed class JsonGetNetworkRequest : NetworkRequest
1717
{
18+
private static readonly JsonSerializerOptions DefaultJsonOptions = new() { PropertyNameCaseInsensitive = true };
19+
1820
private readonly HttpClient client;
1921

2022
/// <summary>
@@ -43,7 +45,7 @@ public JsonGetNetworkRequest(HttpClient client, string url)
4345
/// <param name="headers">
4446
/// The additional headers.
4547
/// </param>
46-
public JsonGetNetworkRequest(HttpClient client, string url, Dictionary<string, string> headers)
48+
public JsonGetNetworkRequest(HttpClient client, string url, Dictionary<string, string>? headers)
4749
: base(url, headers)
4850
{
4951
this.client = client ?? throw new ArgumentNullException(nameof(client));
@@ -64,7 +66,8 @@ public JsonGetNetworkRequest(HttpClient client, string url, Dictionary<string, s
6466
public override async Task<TResponse> ExecuteAsync<TResponse>(CancellationToken cancellationToken = default)
6567
{
6668
string json = await this.GetJsonResponseAsync(cancellationToken).ConfigureAwait(false);
67-
return JsonSerializer.Deserialize<TResponse>(json, new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
69+
return JsonSerializer.Deserialize<TResponse>(json, DefaultJsonOptions)
70+
?? throw new InvalidOperationException($"Failed to deserialize response to {typeof(TResponse).Name}.");
6871
}
6972

7073
/// <summary>
@@ -84,7 +87,8 @@ public override async Task<object> ExecuteAsync(
8487
CancellationToken cancellationToken = default)
8588
{
8689
string json = await this.GetJsonResponseAsync(cancellationToken).ConfigureAwait(false);
87-
return JsonSerializer.Deserialize(json, expectedResponse, new JsonSerializerOptions { PropertyNameCaseInsensitive = true });
90+
return JsonSerializer.Deserialize(json, expectedResponse, DefaultJsonOptions)
91+
?? throw new InvalidOperationException($"Failed to deserialize response to {expectedResponse.Name}.");
8892
}
8993

9094
private async Task<string> GetJsonResponseAsync(CancellationToken cancellationToken = default)
@@ -118,6 +122,6 @@ private async Task<string> GetJsonResponseAsync(CancellationToken cancellationTo
118122

119123
response.EnsureSuccessStatusCode();
120124

121-
return await response.Content.ReadAsStringAsync().ConfigureAwait(false);
125+
return await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false);
122126
}
123127
}

0 commit comments

Comments
 (0)