Merge pull request #87 from Cysharp/feature/AbortOnCanceled

mayuki · web-flow · commit 623e2608262c · 2024-08-05T15:46:53.000+09:00
Fixed to cancel requests correctly when a request is canceled before receiving a response.
diff --git a/src/YetAnotherHttpHandler/ResponseContext.cs b/src/YetAnotherHttpHandler/ResponseContext.cs
@@ -150,6 +150,7 @@ public void Cancel()
 
             lock (_writeLock)
             {
+                _requestContext.TryAbort();
                 _responseTask.TrySetCanceled(_cancellationToken);
                 _pipe.Writer.Complete(new OperationCanceledException(_cancellationToken));
                 _completed = true;
diff --git a/test/YetAnotherHttpHandler.Test/Http2TestBase.cs b/test/YetAnotherHttpHandler.Test/Http2TestBase.cs
@@ -365,6 +365,37 @@ public async Task DisposeHttpResponseMessage_Post_SendingBody_Duplex()
         Assert.IsAssignableFrom<IOException>(ex.InnerException);
     }
 
+    [ConditionalFact]
+    public async Task Cancel_Get_BeforeReceivingResponseHeaders()
+    {
+        // Arrange
+        using var httpHandler = CreateHandler();
+        var httpClient = new HttpClient(httpHandler);
+        await using var server = await LaunchServerAsync<TestServerForHttp1AndHttp2>();
+        var id = Guid.NewGuid().ToString();
+
+        // Act
+        var request = new HttpRequestMessage(HttpMethod.Get, $"{server.BaseUri}/slow-response-headers")
+        {
+            Version = HttpVersion.Version20,
+            Headers = {  { TestServerForHttp1AndHttp2.SessionStateHeaderKey, id } }
+        };
+        
+        // The server responds after one second. But the client cancels the request before receiving response headers.
+        var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(300));
+        var ex = await Record.ExceptionAsync(async () => await httpClient.SendAsync(request, cts.Token).WaitAsync(TimeoutToken));
+        await Task.Delay(100);
+        var isCanceled = await httpClient.GetStringAsync($"{server.BaseUri}/session-state?id={id}&key=IsCanceled").WaitAsync(TimeoutToken);
+
+        // Assert
+        var operationCanceledException = Assert.IsAssignableFrom<OperationCanceledException>(ex);
+#if !UNITY_2021_1_OR_NEWER
+        // NOTE: Unity's Mono HttpClient internally creates a new CancellationTokenSource.
+        Assert.Equal(cts.Token, operationCanceledException.CancellationToken);
+#endif
+        Assert.Equal("True", isCanceled);
+    }
+
     [ConditionalFact]
     public async Task Cancel_Post_BeforeRequest()
     {
@@ -593,7 +624,11 @@ public async Task Grpc_Error_TimedOut_With_HttpClientTimeout()
         // Assert
         Assert.IsType<RpcException>(ex);
         Assert.Equal(StatusCode.Cancelled, ((RpcException)ex).StatusCode);
+#if UNITY_2021_1_OR_NEWER
+        Assert.IsType<OperationCanceledException>(((RpcException)ex).Status.DebugException);
+#else
         Assert.IsType<TaskCanceledException>(((RpcException)ex).Status.DebugException);
+#endif
     }
 
     [ConditionalFact]
diff --git a/test/YetAnotherHttpHandler.Test/TestServerForHttp1AndHttp2.cs b/test/YetAnotherHttpHandler.Test/TestServerForHttp1AndHttp2.cs
@@ -1,20 +1,60 @@
 using System.Buffers;
+using System.Collections.Concurrent;
 using System.IO.Pipelines;
 using System.Net;
 using Grpc.Core;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.Extensions.DependencyInjection;
 using TestWebApp;
 
 namespace _YetAnotherHttpHandler.Test;
 
 class TestServerForHttp1AndHttp2 : ITestServerBuilder
 {
+    private const string SessionStateKey = "SessionState";
+    public const string SessionStateHeaderKey = "x-yahatest-session-id";
+
+    record SessionStateFeature(ConcurrentDictionary<string, object> Items);
+
     public static WebApplication BuildApplication(WebApplicationBuilder builder)
     {
+        builder.Services.AddKeyedSingleton(SessionStateKey, new ConcurrentDictionary<string, ConcurrentDictionary<string, object>>());
+
         var app = builder.Build();
 
+        // SessionState
+        app.Use((ctx, next) =>
+        {
+            if (ctx.Request.Headers.TryGetValue(SessionStateHeaderKey, out var headerValues))
+            {
+                var sessionStates = ctx.RequestServices.GetRequiredKeyedService<ConcurrentDictionary<string, ConcurrentDictionary<string, object>>>(SessionStateKey);
+                var sessionStateItems = sessionStates.GetOrAdd(headerValues.ToString(), _ => new ConcurrentDictionary<string, object>());
+                ctx.Features.Set(new SessionStateFeature(sessionStateItems));
+            }
+            else
+            {
+                ctx.Features.Set(new SessionStateFeature(new ConcurrentDictionary<string, object>()));
+            }
+
+            return next(ctx);
+        });
+        app.MapGet("/session-state", (HttpContext ctx, string id, string key) =>
+        {
+            var sessionStates = ctx.RequestServices.GetRequiredKeyedService<ConcurrentDictionary<string, ConcurrentDictionary<string, object>>>(SessionStateKey);
+            if (sessionStates.TryGetValue(id, out var items))
+            {
+                if (items.TryGetValue(key, out var value))
+                {
+                    return Results.Content(value.ToString());
+                }
+                return Results.Content(string.Empty);
+            }
+
+            return Results.NotFound();
+        });
+
         // HTTP/1 and HTTP/2
         app.MapGet("/", () => Results.Content("__OK__"));
         app.MapGet("/not-found", () => Results.Content("__Not_Found__", statusCode: 404));
@@ -23,6 +63,18 @@ public static WebApplication BuildApplication(WebApplicationBuilder builder)
             httpContext.Response.Headers["x-test"] = "foo";
             return Results.Content("__OK__");
         });
+        app.MapGet("/slow-response-headers", async (HttpContext httpContext) =>
+        {
+            using var _ = httpContext.RequestAborted.Register(() =>
+            {
+                httpContext.Features.GetRequiredFeature<SessionStateFeature>().Items["IsCanceled"] = true;
+            });
+
+            await Task.Delay(1000);
+            httpContext.Response.Headers["x-test"] = "foo";
+
+            return Results.Content("__OK__");
+        });
         app.MapGet("/ハロー", () => Results.Content("Konnichiwa"));
         app.MapPost("/slow-upload", async (HttpContext ctx, PipeReader reader) =>
         {
diff --git a/test/YetAnotherHttpHandler.Unity.Test/Assets/Tests/Http2ClearTextTest.cs b/test/YetAnotherHttpHandler.Unity.Test/Assets/Tests/Http2ClearTextTest.cs
@@ -18,7 +18,7 @@
 
 public class Http2ClearTextTest : YahaUnityTestBase
 {
-    [Fact]
+    [ConditionalFact]
     public async Task FailedToConnect_VersionMismatch()
     {
         // Arrange
@@ -343,7 +343,7 @@ public async Task Cancel_Post_SendingBody_Duplex()
         // Assert
         var operationCanceledException = Assert.IsAssignableFrom<OperationCanceledException>(ex);
         Assert.Equal(cts.Token, operationCanceledException.CancellationToken);
-}
+    }
 #endif
 
     [ConditionalFact]
@@ -376,6 +376,37 @@ public async Task DisposeHttpResponseMessage_Post_SendingBody_Duplex()
         Assert.IsAssignableFrom<IOException>(ex.InnerException);
     }
 
+    [ConditionalFact]
+    public async Task Cancel_Get_BeforeReceivingResponseHeaders()
+    {
+        // Arrange
+        using var httpHandler = CreateHandler();
+        var httpClient = new HttpClient(httpHandler);
+        await using var server = await LaunchServerAsync<TestServerForHttp1AndHttp2>();
+        var id = Guid.NewGuid().ToString();
+
+        // Act
+        var request = new HttpRequestMessage(HttpMethod.Get, $"{server.BaseUri}/slow-response-headers")
+        {
+            Version = HttpVersion.Version20,
+            Headers = { { TestServerForHttp1AndHttp2.SessionStateHeaderKey, id } }
+        };
+
+        // The server responds after one second. But the client cancels the request before receiving response headers.
+        var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(300));
+        var ex = await Record.ExceptionAsync(async () => await httpClient.SendAsync(request, cts.Token).WaitAsync(TimeoutToken));
+        await Task.Delay(100);
+        var isCanceled = await httpClient.GetStringAsync($"{server.BaseUri}/session-state?id={id}&key=IsCanceled").WaitAsync(TimeoutToken);
+
+        // Assert
+        var operationCanceledException = Assert.IsAssignableFrom<OperationCanceledException>(ex);
+#if !UNITY_2021_1_OR_NEWER
+        // NOTE: Unity's Mono HttpClient internally creates a new CancellationTokenSource.
+        Assert.Equal(cts.Token, operationCanceledException.CancellationToken);
+#endif
+        Assert.Equal("True", isCanceled);
+    }
+
     [ConditionalFact]
     public async Task Cancel_Post_BeforeRequest()
     {
@@ -646,6 +677,33 @@ public async Task Grpc_Error_TimedOut_With_CancellationToken()
         Assert.Equal(StatusCode.Cancelled, ((RpcException)ex).StatusCode);
     }
 
+    [ConditionalFact]
+    public async Task Enable_Http2KeepAlive()
+    {
+        // Arrange
+        using var httpHandler = CreateHandler();
+        httpHandler.Http2KeepAliveInterval = TimeSpan.FromSeconds(5);
+        httpHandler.Http2KeepAliveTimeout = TimeSpan.FromSeconds(5);
+        httpHandler.Http2KeepAliveWhileIdle = true;
+
+        var httpClient = new HttpClient(httpHandler);
+        await using var server = await LaunchServerAsync<TestServerForHttp1AndHttp2>();
+
+        // Act
+        var request = new HttpRequestMessage(HttpMethod.Get, $"{server.BaseUri}/")
+        {
+            Version = HttpVersion.Version20,
+        };
+        var response = await httpClient.SendAsync(request).WaitAsync(TimeoutToken);
+        var responseBody = await response.Content.ReadAsStringAsync().WaitAsync(TimeoutToken);
+
+        // Assert
+        Assert.Equal("__OK__", responseBody);
+        Assert.Equal(HttpVersion.Version20, response.Version);
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+    }
+
+
     // Content with default value of true for AllowDuplex because AllowDuplex is internal.
     class DuplexStreamContent : HttpContent
     {
diff --git a/test/YetAnotherHttpHandler.Unity.Test/Assets/Tests/YahaUnityTestBase.cs b/test/YetAnotherHttpHandler.Unity.Test/Assets/Tests/YahaUnityTestBase.cs
@@ -7,7 +7,7 @@
 
 public abstract class YahaUnityTestBase
 {
-    protected HttpMessageHandler CreateHandler()
+    protected YetAnotherHttpHandler CreateHandler()
         => new YetAnotherHttpHandler() { Http2Only = true };
 
     protected CancellationToken TimeoutToken { get; private set; }
@@ -43,7 +43,10 @@ public enum TestWebAppServerListenMode
         SecureHttp1AndHttp2,
     }
 
-    protected class TestServerForHttp1AndHttp2 { }
+    protected class TestServerForHttp1AndHttp2
+    {
+        public const string SessionStateHeaderKey = "x-yahatest-session-id";
+    }
 
     [SetUp]
     public void Setup()

Original file line number	Diff line number	Diff line change
`@@ -150,6 +150,7 @@ public void Cancel()`
`150`	`150`
`151`	`151`	`lock (_writeLock)`
`152`	`152`	`{`
	`153`	`+ _requestContext.TryAbort();`
`153`	`154`	`_responseTask.TrySetCanceled(_cancellationToken);`
`154`	`155`	`_pipe.Writer.Complete(new OperationCanceledException(_cancellationToken));`
`155`	`156`	`_completed = true;`