[API Proposal]: `TimeSpan` validation methods

[aetos382]

Background and motivation

In many use cases, negative TimeSpan values are invalid, so you will want to make them an error during parameter validation.
However, Timeout.Infinite is an exceptional case.
If you want to perform parameter validation like this, you will need to write boilerplate code like the following.

void Foo(TimeSpan value)
{
    if (value != Timeout.Infinite)
    {
        ArgumentOutOfRangeException.ThrowIfLessThan(value, TimeSpan.Zero);
    }
}

You should not write this as follows.
This is because there are negative values that are larger than Timeout.InfiniteTimeSpan.

void Foo(TimeSpan value)
{
    ArgumentOutOfRangeException.ThrowIfLessThan(value, Timeout.InfiniteTimeSpan);

    // Foo(TimeSpan.FromTicks(-1)) prints "-00:00:00.0000001: Valid".
    Console.WriteLine($"{value}: Valid");
}

Also, ArgumentOutOfRangeException.ThrowIfNegative cannot be used when comparing with Timeout.Zero because TimeSpan does not implement INumberBase<T>.
But it is not a good idea to add a method specific to TimeSpan validation to the ArgumentOutOfRangeException class.

And, .NET timers support timeout values in milliseconds, and the valid range is from 1 to UInt32.MaxValue - 1 (0xfffffffe). 0xffffffff is not valid because it is Timeout.Infinite.
It may be useful to have a method that checks whether or not the specified value is within this valid range.
For example, it can be used by someone implementing a class that uses timers internally and wants to check the range of values in the constructor.

So, we propose adding a validation methods to the TimeSpan class.
Using these methods will make the meaning of the code clearer and also help you avoid errors in value validation.

It may also be a good idea to provide an analyzer that recommends specifications for methods like this.

API Proposal

namespace System;

public struct TimeSpan
{
    // Throws `ArgumentOutOfRangeException` if the value is negative (even if it is `Timeout.InifiniteTimeSpan`).
    public static void ThrowIfNegative(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws `ArgumentOutOfRangeException` if the value is negative (except `Timeout.InifiniteTimeSpan`).
    public static void ThrowIfNegativeExceptInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws `ArgumentOutOfRangeException` if the value is negative or `TimeSpan.Zero` (even if it is `Timeout.InifiniteTimeSpan`).
    public static void ThrowIfNegativeOrZero(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws `ArgumentOutOfRangeException` if the value is negative or `TimeSpan.Zero` (except `Timeout.InifiniteTimeSpan`).
    public static void ThrowIfNegativeOrZeroExceptInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 1 and 4294967294ms.
    public static void ThrowIfOutOfTimeoutRange(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 0 and 4294967294ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsZero(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 1 and 4294967295ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 0 and 4294967295ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsZeroAndInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);
}

API Usage

void Foo(TimeSpan value)
{
    // `Foo(TimeSpan.FromTicks(-1))` will result in an error.
    TimeSpan.ThrowIfNegativeExceptInfinite(value);
}

Alternative Designs

The timer itself can also have a method to check whether the value is within the valid range of the timer.
Since some of the timer implementations in .NET eventually end up with the internal TimerQueueTimer class that implements ITimer interface, this method could also be a static method of ITimer.
The upper limit supported by the timer-related methods we have proposed is currently defined in the MaxSupportedTimeout field of the System.Threading.Timer class. The ITimer interface was introduced later, along with the TimerProvider.

public interface ITimer
{
    // Throws ArgumentOutOfRangeException if the value is not between 1 and 4294967294ms.
    public static void ThrowIfOutOfTimeoutRange(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 0 and 4294967294ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsZero(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 1 and 4294967295ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);

    // Throws ArgumentOutOfRangeException if the value is not between 0 and 4294967295ms.
    public static void ThrowIfOutOfTimeoutRangeAllowsZeroAndInfinite(TimeSpan value, [CallerArgumentExpression] string? paramName = null);
}

System.Windows.Forms.Timer and System.Windows.Threading.DispatcherTimer are implemented using the Win32 API SetTimer rather than TimerQueueTimer, and in this case the upper limit is different.

Risks

No response

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-datetime
See info in area-owners.md if you want to be subscribed.

[LWChris]

I agree that ArgumentOutOfRangeException should not be cluttered with 8 methods, but it also doesn't look right that TimeSpan gets methods "in case it's meant to be a timeout", like ThrowIfOutOfTimeoutRangeAllowsZeroAndInfinite for example.

As I see you realized in your proposal, a TimeSpan of "-1 ms" is a perfectly valid negative time span without any special meaning, so ThrowIfNegative must throw for Timeout.InfiniteTimeSpan in a TimeSpan context but shouldn't in a Timeout context. That -1 ms is a sentinel value for Timeout.Infinite is only relevant in case TimeSpan is meant to be a timeout.

Hence you provided ThrowIfOutOfTimeoutRange... methods to cover both use cases.

My idea: wouldn't it make sense to add those methods to Timeout, since that's a static class anyway, currently without any use except for providing Infinite and InfiniteTimeSpan? This would solve the whole interpretation issue - if you want to use TimeSpan as is and don't care about its potential use as a timeout, you will not be bothered with the "-1 ms" sentinel value anomaly. But if you intend to use the TimeSpan as timeout, why not use Timeout to validate it:

• Timeout.ThrowIfInvalid(TimeSpan t) → throws if t < TimeSpan.Zero && t != Timeout.InfiniteTimeSpan
• Timeout.ThrowIfInvalidOrInfinite(TimeSpan t) → throws if t < TimeSpan.Zero
• Timeout.ThrowIfInvalidOrInfiniteOrZero(TimeSpan t) → throws if t <= TimeSpan.Zero
• Timeout.ThrowIfInvalidOrZero(TimeSpan t) → throws if t <= TimeSpan.Zero && t != Timeout.InfiniteTimeSpan

[aetos382]

Putting them in the Timeout class is a good idea!

[julealgon]

@LWChris TimeSpan values don't always represent "timeouts" conceptually. Putting the guard methods on the Timeout class would be too inflexible in that sense (it would be misleading/confusing to use in places where the value is not meant to be used as a timeout, but still needs similar validations).

[LWChris]

@julealgon I think you misunderstood my intentions, since your arguments are precisely the reason why I suggested to put the validations I mentioned in Timeout.

To clarify, I did not say anything about "normal" validations for TimeSpan. I'd suggest to add overloads to ArgumentOutOfRangeException, such as ThrowIfNegative(TimeSpan t) that throws if t < TimeSpan.Zero, i.e. including Timeout.InfiniteTimeSpan.

But for cases where a TimeSpan is a timeout, those wouldn't be very useful. The validations in Timeout are precisely meant to cover that. Most notably, all negative values for TimeSpan are considered invalid, with the exception of "-1 ms", which is considered to be "Positive Infinity".

[julealgon]

@julealgon I think you misunderstood my intentions

As I re-read your post again @LWChris , I absolutely did, my bad.

The only contention point I have now with your proposal is the inconsistency factor: some guard methods in exception classes, and some other guard methods in types that are not exception types (like Timeout). I'm not a big fan of the inconsistency itself, even if I do like where you were coming from conceptually.

Which to me highlights a flaw with the original concept of having guard methods be called from the exception types themselves: that idea will not scale as we keep adding more and more validations such as this, as you well put it that the relationship is not really 1:1 between the exception type and the type of validation being performed (i.e. a "timeout" is more specific than a "timespan").

Unfortunately, I assume the ship has sailed now and that design will not be changed anymore (i.e. guard methods on exception types).

My personal take is that this (guard methods) should be completely redesigned as proper language-level contracts, but of course that is a much larger ask and doesn't seem to be a priority for Microsoft (most contract-related issues are in the backlog for several years at this point without much movement if at all).

On another note, I could also argue that it would be preferred to have a dedicated type to represent timeouts that are backed by a TimeSpan value, and have those common validation rules embedded into that type (at construction or parsing time). Again, unfortunately, the language is not really designed to cleanly support these "value types" so I don't see that happening in the near future either.

[LWChris]

This is how a proposed solution would behave with TimeSpan overloads in ArgumentOutOfRangeException and "specialized" treatment of "-1 ms" within Timeout:

The only contention point I have now with your proposal is the inconsistency factor: some guard methods in exception classes, and some other guard methods in types that are not exception types (like Timeout). I'm not a big fan of the inconsistency itself, even if I do like where you were coming from conceptually.

The only solution for that would be to not use Timeout throwing ArgumentOutOfRangeException, but create a new TimeoutArgumentOutOfRangeException. That could even inherit ArgumentOutOfRangeException as far as I can tell. Additional benefit would be that we can also add ThrowIfGreaterThan that can throw for "-1 ms" which is another thing ArgumentOutOfRangeException couldn't handle well (-1 ms being greater than 10 seconds).

[julealgon]

The only solution for that would be to not use Timeout throwing ArgumentOutOfRangeException, but create a new TimeoutArgumentOutOfRangeException. That could even inherit ArgumentOutOfRangeException as far as I can tell.

Would that only serve to differentiate the source of the guard methods? If that's the case, I don't see it being approved. There would need to be a stronger need to create a brand-new exception type that would justify the type by itself IMHO.

Additional benefit would be that we can also add ThrowIfGreaterThan that can throw for "-1 ms" which is another thing ArgumentOutOfRangeException couldn't handle well (-1 ms being greater than 10 seconds).

To be honest this -1 nonsense should die in a fire. This is a remnant of old-style C programming where someone hacked their way into giving a special meaning to a magic number. This design should have no place in C# in my opinion. Yes, I do realize the boat has sailed on this as well (unfortunately), but a "timeout" should be a restricted discriminated union eventually (I hope one day this will be possible):

public type Timeout = TimeSpan t where t > 0 | Infinite;