Skip to content

Commit

Permalink
Avoid unobserved task exceptions in Http tests (dotnet#104384)
Browse files Browse the repository at this point in the history 
* Improve HttpProtocolTests

* Avoid unobserved exceptions from H2's read loop

* More test changes

* Actually do what the test name suggests

* Fix Dispose_DisposingHandlerCancelsActiveOperationsWithoutResponses

* Revert some changes that Browser doesn't like
  • Loading branch information
@MihaZupan
MihaZupan committed Jul 5, 2024
1 parent 116f5fd commit bd752c3
Show file tree
Hide file tree
Showing 15 changed files with 134 additions and 325 deletions.
6 changes: 5 additions & 1 deletion src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.AutoRedirect.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,13 @@ public async Task GetAsync_AllowAutoRedirectTrue_NonRedirectStatusCode_LocationH
{
await LoopbackServer.CreateServerAsync(async (origServer, origUrl) =>
{
Task redirectTask = null;
await LoopbackServer.CreateServerAsync(async (redirectServer, redirectUrl) =>
{
Task<HttpResponseMessage> getResponseTask = client.GetAsync(TestAsync, origUrl);
Task redirectTask = redirectServer.AcceptConnectionSendResponseAndCloseAsync();
redirectTask = redirectServer.AcceptConnectionSendResponseAndCloseAsync();
await TestHelper.WhenAllCompletedOrAnyFailed(
getResponseTask,
Expand All @@ -199,6 +201,8 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
Assert.False(redirectTask.IsCompleted, "Should not have redirected to Location");
}
});
await IgnoreExceptions(redirectTask);
});
}
}
Expand Down
85 changes: 20 additions & 65 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Cancellation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
}
}, async server =>
{
try
{
await server.AcceptConnectionAsync(connection => serverRelease.Task);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(server.AcceptConnectionAsync(connection => serverRelease.Task));
});
}

Expand Down Expand Up @@ -142,15 +135,9 @@ await ValidateClientCancellationAsync(async () =>
await getResponse;
});
try
{
clientFinished.SetResult(true);
await serverTask;
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
clientFinished.SetResult(true);
await IgnoreExceptions(serverTask);
});
}
}
Expand Down Expand Up @@ -203,15 +190,9 @@ await ValidateClientCancellationAsync(async () =>
await getResponse;
});
try
{
clientFinished.SetResult(true);
await serverTask;
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
clientFinished.SetResult(true);
await IgnoreExceptions(serverTask);
});
}
}
Expand Down Expand Up @@ -279,15 +260,10 @@ await ValidateClientCancellationAsync(async () =>
cts.Cancel();
await readTask;
}).WaitAsync(TimeSpan.FromSeconds(30));
try
{
clientFinished.SetResult(true);
await serverTask;
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
clientFinished.SetResult(true);
await IgnoreExceptions(serverTask);
});
}
}
Expand Down Expand Up @@ -425,39 +401,18 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
[Fact]
public async Task SendAsync_Cancel_CancellationTokenPropagates()
{
TaskCompletionSource<bool> clientCanceled = new TaskCompletionSource<bool>();
await LoopbackServerFactory.CreateClientAndServerAsync(
async uri =>
{
var cts = new CancellationTokenSource();
cts.Cancel();
var cts = new CancellationTokenSource();
cts.Cancel();

using (HttpClient client = CreateHttpClient())
{
OperationCanceledException ex = null;
try
{
await client.GetAsync(uri, cts.Token);
}
catch (OperationCanceledException e)
{
ex = e;
}
Assert.True(ex != null, "Expected OperationCancelledException, but no exception was thrown.");
using HttpClient client = CreateHttpClient();

Assert.True(cts.Token.IsCancellationRequested, "cts token IsCancellationRequested");
OperationCanceledException oce = await Assert.ThrowsAnyAsync<OperationCanceledException>(
() => client.GetAsync(TestAsync, InvalidUri, cancellationToken: cts.Token));

// .NET Framework has bug where it doesn't propagate token information.
Assert.True(ex.CancellationToken.IsCancellationRequested, "exception token IsCancellationRequested");
clientCanceled.SetResult(true);
}
},
async server =>
{
Task serverTask = server.HandleRequestAsync();
await clientCanceled.Task;
});
// .NET Framework has bug where it doesn't propagate token information.
#if !NETFRAMEWORK
Assert.Equal(cts.Token, oce.CancellationToken);
#endif
}

public static IEnumerable<object[]> PostAsync_Cancel_CancellationTokenPassedToContent_MemberData()
Expand Down
27 changes: 3 additions & 24 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.Proxy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,14 +573,7 @@ await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
using (HttpClient client = CreateHttpClient(handler))
{
handler.Proxy = new WebProxy(proxyUri);
try
{
await client.GetAsync(uri);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(client.GetAsync(uri));
}
}, server => server.AcceptConnectionAsync(async connection =>
{
Expand Down Expand Up @@ -613,14 +606,7 @@ await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
using (HttpClient client = CreateHttpClient(handler))
{
handler.Proxy = new WebProxy(proxyUri);
try
{
await client.GetAsync(addressUri);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(client.GetAsync(addressUri));
}
}, server => server.AcceptConnectionAsync(async connection =>
{
Expand All @@ -647,14 +633,7 @@ await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
using (HttpClient client = CreateHttpClient(handler))
{
handler.Proxy = new WebProxy(proxyUri);
try
{
await client.GetAsync(addressUri);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(client.GetAsync(addressUri));
}
}, server => server.AcceptConnectionAsync(async connection =>
{
Expand Down
52 changes: 19 additions & 33 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,7 @@ await LoopbackServer.CreateClientAndServerAsync(async proxyUri =>
using (HttpClient client = CreateHttpClient(handler))
{
handler.Proxy = new WebProxy(proxyUri);
try
{
await client.GetAsync(ipv6Address);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(client.GetAsync(ipv6Address));
}
}, server => server.AcceptConnectionAsync(async connection =>
{
Expand Down Expand Up @@ -299,14 +292,8 @@ await LoopbackServer.CreateClientAndServerAsync(async url =>
// we could not create SslStream in browser, [ActiveIssue("https://github.com/dotnet/runtime/issues/37669", TestPlatforms.Browser)]
handler.ServerCertificateCustomValidationCallback = TestHelper.AllowAllCertificates;
}
try
{
await client.GetAsync(url);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await IgnoreExceptions(client.GetAsync(url));
}
}, server => server.AcceptConnectionAsync(async connection =>
{
Expand Down Expand Up @@ -909,18 +896,15 @@ await LoopbackServer.CreateServerAsync(async (server, url) =>
Task serverTask = server.AcceptConnectionAsync(async connection =>
{
await connection.ReadRequestHeaderAndSendCustomResponseAsync("HTTP/1.1 200 OK\r\n" + LoopbackServer.CorsHeaders + "Transfer-Encoding: chunked\r\n\r\n");
try
await IgnoreExceptions(async () =>
{
while (!cts.IsCancellationRequested) // infinite to make sure implementation doesn't OOM
{
await connection.WriteStringAsync(new string(' ', 10000));
await Task.Delay(1);
}
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
});
});
await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync(url));
Expand Down Expand Up @@ -1437,6 +1421,11 @@ public async Task Dispose_DisposingHandlerCancelsActiveOperationsWithoutResponse
return;
}

// Tasks are stored and awaited outside of the CreateServerAsync callbacks
// to let server disposal abort any pending operations.
Task serverTask1 = null;
Task serverTask2 = null;

await LoopbackServerFactory.CreateServerAsync(async (server1, url1) =>
{
await LoopbackServerFactory.CreateServerAsync(async (server2, url2) =>
Expand All @@ -1446,13 +1435,13 @@ await LoopbackServerFactory.CreateServerAsync(async (server3, url3) =>
var unblockServers = new TaskCompletionSource<bool>(TaskCreationOptions.RunContinuationsAsynchronously);
// First server connects but doesn't send any response yet
Task serverTask1 = server1.AcceptConnectionAsync(async connection1 =>
serverTask1 = server1.AcceptConnectionAsync(async connection1 =>
{
await unblockServers.Task;
});
// Second server connects and sends some but not all headers
Task serverTask2 = server2.AcceptConnectionAsync(async connection2 =>
serverTask2 = server2.AcceptConnectionAsync(async connection2 =>
{
await connection2.ReadRequestDataAsync();
await connection2.SendPartialResponseHeadersAsync(HttpStatusCode.OK);
Expand Down Expand Up @@ -1489,9 +1478,14 @@ await LoopbackServerFactory.CreateServerAsync(async (server3, url3) =>
{
Assert.Equal("12345678901234567890", await response3.Content.ReadAsStringAsync());
}
await serverTask3;
});
});
});

await IgnoreExceptions(serverTask1);
await IgnoreExceptions(serverTask2);
}

[Theory]
Expand Down Expand Up @@ -1806,15 +1800,7 @@ await LoopbackServerFactory.CreateClientAndServerAsync(async uri =>
{
await server.AcceptConnectionAsync(async connection =>
{
try
{
await connection.ReadRequestDataAsync(readBody: true);
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
await clientFinished.Task.WaitAsync(TimeSpan.FromMinutes(2));
await IgnoreExceptions(connection.ReadRequestDataAsync(readBody: true));
});
});
}
Expand Down
16 changes: 16 additions & 0 deletions src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace System.Net.Http.Functional.Tests

public abstract partial class HttpClientHandlerTestBase : FileCleanupTestBase
{
protected static readonly Uri InvalidUri = new("http://nosuchhost.invalid");

// This file is shared with the WinHttpHandler implementation, which supports .NET Framework
// So, define this so derived tests can use it.
public static readonly Version HttpVersion30 = new Version(3, 0);
Expand All @@ -31,6 +33,20 @@ public HttpClientHandlerTestBase(ITestOutputHelper output)
_output = output;
}

protected async Task IgnoreExceptions(Func<Task> func)
{
try
{
await func();
}
catch (Exception ex)
{
_output.WriteLine($"Ignored exception:{Environment.NewLine}{ex}");
}
}

protected Task IgnoreExceptions(Task task) => IgnoreExceptions(() => task);

protected virtual HttpClient CreateHttpClient() => CreateHttpClient(CreateHttpClientHandler());

protected HttpClient CreateHttpClient(HttpMessageHandler handler) =>
Expand Down
Loading

0 comments on commit bd752c3

Please sign in to comment.