Clean up async timeout extension methods

[JamesNK]

There are async timeout extension methods available in Microsoft.AspNetCore.Testing, e.g. await task.TimeoutAfter().

Many ASP.NET Core projects have there own copies of these timeout methods, or wrap them in there own extension methods (probably done to remove a copy while minimizing changes), e.g. https://github.com/aspnet/AspNetCore/blob/master/src/Hosting/TestHost/test/Utilities.cs

We should go through aspnetcore/extensions and clean up the copies and wrappers.

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[JunTaoLuo]

This seems like a simple clean up item but also seem like it's of low priority.

[davidfowl]

We should replace this with the new built in Task.WaitAsync methods.

[ladeak]

It is itchy to replace TimeoutAfter (including DefaultTimeout, OrTimeout, WithTimeout) extension methods, but some of them tend to create a custom timeout exception message using filePath, lineNumber.

[SuppressMessage("ApiDesign", "RS0026:Do not add multiple public overloads with optional parameters", Justification = "Required to maintain compatibility")]
private static async Task TimeoutAfter(this Task task, TimeSpan timeout,
    [CallerFilePath] string filePath = null,
    [CallerLineNumber] int lineNumber = default)

and message created as:

private static string CreateMessage(TimeSpan timeout, string filePath, int lineNumber)
            => string.IsNullOrEmpty(filePath)
            ? $"The operation timed out after reaching the limit of {timeout.TotalMilliseconds}ms."
            : $"The operation at {filePath}:{lineNumber} timed out after reaching the limit of {timeout.TotalMilliseconds}ms.";

The suggested WaitAsync does not seem to have an overload that takes a message parameter.

Can I remove this custom error message with the filePath and lineNumber, or they must stay?

[davidfowl]

Hmmm, good point. Maybe we keep the extension methods and replace to implementation and catch the error thrown by WaitAsync and rethrow our own.

[ladeak]

Ok, so my understanding of this project, I am planning to do the following:

• I found src\Shared\Http2cat\TaskTimeoutExtensions. As I see it is only referenced by <server>.FunctionalTests.csproj projects, so I assume it is safe to use it by other test projects too. Not sure if that file would be better to be moved under src\Shared\test.
• I will consolidate TimeoutAfter calls to use the one defined above and remove all duplicate definitions. Also update implementation to try-catch rethrow exception.
• I replace WithTimeout() and OrTimeout() extensions methods with WaitAsync(), as those do not have CallerFilePath and CallerLineNumber attributed arguments.
• I keep the DefaultTimeout() extension method, but the implementation would be consolidated in the above mentioned TaskTimeoutExtensions using WaitAsync() and the value of 30 sec (except for IIS.Common.TestLib for integration testing which is 300 sec).

Let's see how it goes, my machine is struggling a bit with 8GB RAM and the full solution. 🥵

[Tratcher]

Let's see how it goes, my machine is struggling a bit with 8GB RAM and the full solution. 🥵

@ladeak If you use startvs.cmd in an area folder then it only opens those projects. You can work through it in sections.
https://github.com/dotnet/aspnetcore/blob/main/src/Http/startvs.cmd