Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Cuemon.Core/Threading/AsyncOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class AsyncOptions : IAsyncOptions, IParameterObject
/// </remarks>
public AsyncOptions()
{
CancellationToken = default;
CancellationToken = CancellationToken.None;
}

/// <summary>
Expand Down
49 changes: 49 additions & 0 deletions src/Cuemon.Core/Threading/AsyncRunOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
using System;

namespace Cuemon.Threading
{
/// <summary>
/// Provides options that are related to asynchronous run operations.
/// </summary>
/// <seealso cref="AsyncOptions"/>
public class AsyncRunOptions : AsyncOptions
{
/// <summary>
/// Initializes a new instance of the <see cref="AsyncRunOptions"/> class.
/// </summary>
/// <remarks>
/// The following table shows the initial property values for an instance of <see cref="AsyncRunOptions"/>.
/// <list type="table">
/// <listheader>
/// <term>Property</term>
/// <description>Initial Value</description>
/// </listheader>
/// <item>
/// <term><see cref="Timeout"/></term>
/// <description><c>00:00:05</c> (5 seconds)</description>
/// </item>
/// <item>
/// <term><see cref="Delay"/></term>
/// <description><c>00:00:00.1000000</c> (100 milliseconds)</description>
/// </item>
/// </list>
/// </remarks>
public AsyncRunOptions()
{
Timeout = TimeSpan.FromSeconds(5);
Delay = TimeSpan.FromMilliseconds(100);
}
Comment on lines +31 to +35
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Validate Timeout and Delay at construction time

A negative Timeout/Delay (or a Delay greater than Timeout) will make Awaiter behave unpredictably or throw deep inside Task.Delay.
Sanitise once in the ctor to fail-fast.

-    public AsyncRunOptions()
-    {
-        Timeout = TimeSpan.FromSeconds(5);
-        Delay = TimeSpan.FromMilliseconds(100);
-    }
+    public AsyncRunOptions()
+    {
+        Timeout = TimeSpan.FromSeconds(5);
+        Delay = TimeSpan.FromMilliseconds(100);
+    }
+
+    private void Validate()
+    {
+        if (Timeout < TimeSpan.Zero)  throw new ArgumentOutOfRangeException(nameof(Timeout));
+        if (Delay   < TimeSpan.Zero)  throw new ArgumentOutOfRangeException(nameof(Delay));
+        if (Delay   > Timeout)        throw new ArgumentException("Delay cannot exceed Timeout.");
+    }

Call Validate() at the end of the ctor and from both property setters (see next comment).

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/Cuemon.Core/Threading/AsyncRunOptions.cs around lines 31 to 35, the
constructor sets Timeout and Delay without validation, allowing negative values
or Delay greater than Timeout, which causes unpredictable behavior. Add a
Validate() method that checks Timeout and Delay for non-negative values and
ensures Delay is not greater than Timeout. Call Validate() at the end of the
constructor to fail fast if invalid values are set.


/// <summary>
/// Gets or sets the timeout for the asynchronous operation.
/// </summary>
/// <value>The timeout for the asynchronous operation. The default is 5 seconds.</value>
public TimeSpan Timeout { get; set; }

/// <summary>
/// Gets or sets the delay between asynchronous operation attempts.
/// </summary>
/// <value>The delay between asynchronous operation attempts. The default is 100 milliseconds.</value>
public TimeSpan Delay { get; set; }
Comment on lines +41 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add guards in property setters

Without checks, a caller can later inject invalid values after construction.

-public TimeSpan Timeout { get; set; }
+private TimeSpan _timeout;
+public TimeSpan Timeout
+{
+    get => _timeout;
+    set
+    {
+        if (value < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(value));
+        _timeout = value;
+    }
+}
@@
-public TimeSpan Delay { get; set; }
+private TimeSpan _delay;
+public TimeSpan Delay
+{
+    get => _delay;
+    set
+    {
+        if (value < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(value));
+        _delay = value;
+    }
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public TimeSpan Timeout { get; set; }
/// <summary>
/// Gets or sets the delay between asynchronous operation attempts.
/// </summary>
/// <value>The delay between asynchronous operation attempts. The default is 100 milliseconds.</value>
public TimeSpan Delay { get; set; }
private TimeSpan _timeout;
public TimeSpan Timeout
{
get => _timeout;
set
{
if (value < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(value));
_timeout = value;
}
}
/// <summary>
/// Gets or sets the delay between asynchronous operation attempts.
/// </summary>
/// <value>The delay between asynchronous operation attempts. The default is 100 milliseconds.</value>
private TimeSpan _delay;
public TimeSpan Delay
{
get => _delay;
set
{
if (value < TimeSpan.Zero) throw new ArgumentOutOfRangeException(nameof(value));
_delay = value;
}
}
🤖 Prompt for AI Agents
In src/Cuemon.Core/Threading/AsyncRunOptions.cs between lines 41 and 47, the
Timeout and Delay property setters lack validation, allowing invalid values to
be set. Add guard clauses in the setters to validate the input, for example
ensuring the TimeSpan values are not negative or zero where inappropriate, and
throw appropriate exceptions if invalid values are assigned.

}
}
64 changes: 64 additions & 0 deletions src/Cuemon.Core/Threading/Awaiter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Threading.Tasks;

namespace Cuemon.Threading
{
/// <summary>
/// Provides a set of static methods for awaiting asynchronous operations.
/// </summary>
public static class Awaiter
{
/// <summary>
/// Repeatedly invokes the specified asynchronous <paramref name="method"/> until it succeeds or the configured <see cref="AsyncRunOptions.Timeout"/> is reached.
/// </summary>
/// <param name="method">The asynchronous function delegate to execute, returning a <see cref="ConditionalValue"/> indicating success or failure.</param>
/// <param name="setup">The <see cref="AsyncRunOptions"/> which may be configured.</param>
/// <returns>
/// A task that represents the asynchronous operation. The task result contains the <see cref="ConditionalValue"/> returned by the last invocation of <paramref name="method"/>, or an unsuccessful value if the timeout is reached.
/// </returns>
/// <remarks>
/// The <paramref name="method"/> is invoked repeatedly with a delay specified by <see cref="AsyncRunOptions.Delay"/> until it returns a successful <see cref="ConditionalValue"/> or the timeout specified by <see cref="AsyncRunOptions.Timeout"/> is reached.
/// <br/>
/// Potential exceptions thrown by <paramref name="method"/> are caught and collected. If the operation does not succeed before the timeout, <see cref="UnsuccessfulValue"/> will be conditionally initialized:<br/>
/// 1: No caught exceptions; initialized with default constructor,<br/>
/// 2: One caught exception; initialized with caught exception,<br/>
/// 3: Two or more exceptions; initialized with <see cref="AggregateException"/> containing all exceptions.
/// </remarks>
public static async Task<ConditionalValue> RunUntilSucceededOrTimeoutAsync(Func<Task<ConditionalValue>> method, Action<AsyncRunOptions> setup = null)
{
var options = Patterns.Configure(setup);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Guard against null method delegate

Awaiter.RunUntilSucceededOrTimeoutAsync(null) currently throws a null-deref several lines later.
Add quick validation for clearer exception semantics.

-public static async Task<ConditionalValue> RunUntilSucceededOrTimeoutAsync(Func<Task<ConditionalValue>> method, Action<AsyncRunOptions> setup = null)
+public static async Task<ConditionalValue> RunUntilSucceededOrTimeoutAsync(Func<Task<ConditionalValue>> method, Action<AsyncRunOptions> setup = null)
 {
+    if (method == null) throw new ArgumentNullException(nameof(method));
🤖 Prompt for AI Agents
In src/Cuemon.Core/Threading/Awaiter.cs around lines 29 to 31, the method
parameter is not checked for null, causing a null reference exception later. Add
a guard clause at the start of RunUntilSucceededOrTimeoutAsync to throw an
ArgumentNullException if method is null, providing clearer and earlier error
feedback.

var stopwatch = Stopwatch.StartNew();
var exceptions = new List<Exception>();

ConditionalValue conditionalValue = null;
while (stopwatch.Elapsed <= options.Timeout)
{
try
{
options.CancellationToken.ThrowIfCancellationRequested();
conditionalValue = await method().ConfigureAwait(false);
if (conditionalValue.Succeeded) { break; }
}
catch (Exception ex)
Comment on lines +41 to +45
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Possible NullReferenceException when delegate returns null

conditionalValue.Succeeded is accessed inside the loop without a null-check, yet the delegate contract allows returning null.
Either document that method must never return null or defensively handle it:

-conditionalValue = await method().ConfigureAwait(false);
-if (conditionalValue.Succeeded) { break; }
+conditionalValue = await method().ConfigureAwait(false);
+if (conditionalValue?.Succeeded == true) { break; }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
options.CancellationToken.ThrowIfCancellationRequested();
conditionalValue = await method().ConfigureAwait(false);
if (conditionalValue.Succeeded) { break; }
}
catch (Exception ex)
options.CancellationToken.ThrowIfCancellationRequested();
conditionalValue = await method().ConfigureAwait(false);
if (conditionalValue?.Succeeded == true) { break; }
}
catch (Exception ex)
🤖 Prompt for AI Agents
In src/Cuemon.Core/Threading/Awaiter.cs around lines 40 to 44, the code accesses
conditionalValue.Succeeded without checking if conditionalValue is null, which
can cause a NullReferenceException since the delegate method may return null. To
fix this, add a null check for conditionalValue before accessing its Succeeded
property, or update the method's contract to ensure it never returns null and
document this requirement clearly.

{
exceptions.Add(ex);
}

await Task.Delay(options.Delay).ConfigureAwait(false);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Propagate cancellation to the delay

Task.Delay(options.Delay) ignores the provided cancellation token, postponing cancellation until the next loop turn.

-await Task.Delay(options.Delay).ConfigureAwait(false);
+await Task.Delay(options.Delay, options.CancellationToken).ConfigureAwait(false);

This ensures that a cancelled token breaks out immediately.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await Task.Delay(options.Delay).ConfigureAwait(false);
await Task.Delay(options.Delay, options.CancellationToken).ConfigureAwait(false);
🤖 Prompt for AI Agents
In src/Cuemon.Core/Threading/Awaiter.cs at line 49, the call to Task.Delay does
not pass the cancellation token from options, causing delays to ignore
cancellation requests. Modify the Task.Delay call to include the cancellation
token from options so that the delay can be cancelled immediately when
requested, allowing the loop to break out promptly.

}

return (conditionalValue?.Succeeded ?? false)
? conditionalValue
: GetUnsuccessfulValue(exceptions);
}

private static ConditionalValue GetUnsuccessfulValue(IList<Exception> exceptions)
{
if (exceptions.Count == 0) { return new UnsuccessfulValue(); }
if (exceptions.Count == 1) { return new UnsuccessfulValue(exceptions[0]); }
return new UnsuccessfulValue(new AggregateException(exceptions));
}
}
}
125 changes: 125 additions & 0 deletions test/Cuemon.Core.Tests/Threading/AwaiterTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using Codebelt.Extensions.Xunit;
using Xunit;

namespace Cuemon.Threading
{
public class AwaiterTest : Test
{
[Fact]
public async Task RunUntilSucceededOrTimeoutAsync_ShouldReturnOnImmediateSuccess()
{
// Arrange
var callCount = 0;
Task<ConditionalValue> Method()
{
callCount++;
return Task.FromResult<ConditionalValue>(new SuccessfulValue());
}

// Act
var result = await Awaiter.RunUntilSucceededOrTimeoutAsync(Method);

// Assert
Assert.True(result.Succeeded);
Assert.Equal(1, callCount);
}

[Fact]
public async Task RunUntilSucceededOrTimeoutAsync_ShouldRetryUntilSuccess()
{
// Arrange
var callCount = 0;
Task<ConditionalValue> Method()
{
callCount++;
if (callCount < 3)
return Task.FromResult<ConditionalValue>(new UnsuccessfulValue());
return Task.FromResult<ConditionalValue>(new SuccessfulValue());
}

// Act
var result = await Awaiter.RunUntilSucceededOrTimeoutAsync(Method, o =>
{
o.Timeout = TimeSpan.FromSeconds(2);
o.Delay = TimeSpan.FromMilliseconds(10);
});

// Assert
Assert.True(result.Succeeded);
Assert.Equal(3, callCount);
}

[Fact]
public async Task RunUntilSucceededOrTimeoutAsync_ShouldReturnUnsuccessfulOnTimeout_NoExceptions()
{
// Arrange
Task<ConditionalValue> Method() => Task.FromResult<ConditionalValue>(new UnsuccessfulValue());

// Act
var result = await Awaiter.RunUntilSucceededOrTimeoutAsync(Method, o =>
{
o.Timeout = TimeSpan.FromMilliseconds(50);
o.Delay = TimeSpan.FromMilliseconds(10);
});

// Assert
Assert.False(result.Succeeded);
Assert.Null(result.Failure);
}

[Fact]
public async Task RunUntilSucceededOrTimeoutAsync_ShouldReturnUnsuccessfulWithSingleException()
{
// Arrange
var ct = new CancellationTokenSource(TimeSpan.FromMilliseconds(25)).Token;

;

// Act
var result = await Awaiter.RunUntilSucceededOrTimeoutAsync(() => Task.FromResult<ConditionalValue>(new UnsuccessfulValue()), o =>
{
o.Timeout = TimeSpan.FromMilliseconds(75);
o.Delay = TimeSpan.FromMilliseconds(50);
o.CancellationToken = ct;
});

// Assert
Assert.False(result.Succeeded);
Assert.IsType<OperationCanceledException>(result.Failure);
}

[Fact]
public async Task RunUntilSucceededOrTimeoutAsync_ShouldReturnUnsuccessfulWithAggregateException()
{
// Arrange
var exceptions = new List<Exception>
{
new InvalidOperationException("fail1"),
new ArgumentException("fail2")
};
var callCount = 0;
Task<ConditionalValue> Method()
{
throw exceptions[callCount++];
}
Comment on lines +102 to +110
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

exceptions[callCount++] can overrun the list

When the timeout allows more than two iterations, an IndexOutOfRangeException will be added, defeating the intent of verifying an AggregateException with exactly the two predefined failures.

Consider looping the provided exceptions instead:

-Task<ConditionalValue> Method()
-{
-    throw exceptions[callCount++];
-}
+Task<ConditionalValue> Method()
+{
+    var ex = exceptions[callCount % exceptions.Count];
+    callCount++;
+    throw ex;
+}

This keeps the test deterministic irrespective of how many retries occur.


#!/bin/bash
# Locate the AwaiterTest.cs file and inspect the exceptions declaration and Method()
file=$(fd --glob 'AwaiterTest.cs' -t f)
echo "Found file: $file"
echo "---- Head of file ----"
sed -n '1,200p' "$file"
echo "---- Search for exceptions declaration ----"
grep -Rn "exceptions *=" "$file"
echo "---- Search for Method() definition ----"
grep -Rn "Task<ConditionalValue> Method" -n "$file"

Cycle through exceptions in Method() to prevent overruns

When retries exceed the two predefined failures, exceptions[callCount++] will throw an IndexOutOfRangeException. Use the modulo operator on the array’s Length to keep calls within bounds:

  • File: test/Cuemon.Core.Tests/Threading/AwaiterTest.cs (lines 100–108)
-Task<ConditionalValue> Method()
-{
-    throw exceptions[callCount++];
-}
+Task<ConditionalValue> Method()
+{
+    var ex = exceptions[callCount % exceptions.Length];
+    callCount++;
+    throw ex;
+}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
new InvalidOperationException("fail1"),
new ArgumentException("fail2")
};
var callCount = 0;
Task<ConditionalValue> Method()
{
throw exceptions[callCount++];
}
Task<ConditionalValue> Method()
{
var ex = exceptions[callCount % exceptions.Length];
callCount++;
throw ex;
}
🤖 Prompt for AI Agents
In test/Cuemon.Core.Tests/Threading/AwaiterTest.cs around lines 100 to 108, the
code accessing exceptions[callCount++] can cause an IndexOutOfRangeException if
retries exceed the exceptions array length. To fix this, modify the index access
to use callCount modulo the length of the exceptions array, ensuring the index
wraps around and prevents overruns during repeated calls.


// Act
var result = await Awaiter.RunUntilSucceededOrTimeoutAsync(Method, o =>
{
o.Timeout = TimeSpan.FromMilliseconds(50);
o.Delay = TimeSpan.FromMilliseconds(10);
});

// Assert
Assert.False(result.Succeeded);
Assert.IsType<AggregateException>(result.Failure);
var agg = (AggregateException)result.Failure;
Assert.Contains(exceptions[0], agg.InnerExceptions);
Assert.Contains(exceptions[1], agg.InnerExceptions);
}
}
}
Loading