Added operation cancel operation (#46330)

dotnet · Feb 16, 2023 · 9068fcf · 9068fcf
1 parent 6a7bcda
commit 9068fcf
Show file tree

Hide file tree

Showing 18 changed files with 96 additions and 15 deletions.
diff --git a/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt b/src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt
@@ -1,4 +1,5 @@
 #nullable enable
+const Microsoft.AspNetCore.Http.StatusCodes.Status499ClientClosedRequest = 499 -> int
 *REMOVED*Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.DefaultEndpointFilterInvocationContext(Microsoft.AspNetCore.Http.HttpContext! httpContext, params object![]! arguments) -> void
 Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.DefaultEndpointFilterInvocationContext(Microsoft.AspNetCore.Http.HttpContext! httpContext, params object?[]! arguments) -> void
 Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult

diff --git a/src/Http/Http.Abstractions/src/StatusCodes.cs b/src/Http/Http.Abstractions/src/StatusCodes.cs
@@ -283,6 +283,12 @@ public static class StatusCodes
     /// </summary>
     public const int Status451UnavailableForLegalReasons = 451;
 
+    /// <summary>
+    /// HTTP status code 499. This is an unofficial status code originally defined by Nginx and is commonly used
+    /// in logs when the client has disconnected.
+    /// </summary>
+    public const int Status499ClientClosedRequest = 499;
+
     /// <summary>
     /// HTTP status code 500.
     /// </summary>

diff --git a/src/Http/Http.Extensions/test/ProblemDetailsDefaultWriterTest.cs b/src/Http/Http.Extensions/test/ProblemDetailsDefaultWriterTest.cs
@@ -466,7 +466,7 @@ await writer.WriteAsync(new ProblemDetailsContext()
     [Theory]
     [InlineData(StatusCodes.Status400BadRequest, "Bad Request", "https://tools.ietf.org/html/rfc9110#section-15.5.1")]
     [InlineData(StatusCodes.Status418ImATeapot, "I'm a teapot", null)]
-    [InlineData(499, null, null)]
+    [InlineData(498, null, null)]
     public async Task WriteAsync_UsesStatusCode_FromProblemDetails_WhenSpecified(
         int statusCode,
         string title,

diff --git a/src/Http/Http.Results/test/ResultsTests.cs b/src/Http/Http.Results/test/ResultsTests.cs
@@ -1076,15 +1076,14 @@ public void Problem_WithArgs_ResultHasCorrectValues()
     [Theory]
     [InlineData(StatusCodes.Status400BadRequest, "Bad Request", "https://tools.ietf.org/html/rfc9110#section-15.5.1")]
     [InlineData(StatusCodes.Status418ImATeapot, "I'm a teapot", null)]
-    [InlineData(499, null, null)]
+    [InlineData(498, null, null)]
     public void Problem_WithOnlyHttpStatus_ResultHasCorrectValues(
         int statusCode,
         string title,
         string type)
     {
         // Act
         var result = Results.Problem(statusCode: statusCode) as ProblemHttpResult;
-
         // Assert
         Assert.Null(result.ProblemDetails.Detail);
         Assert.Null(result.ProblemDetails.Instance);

diff --git a/src/Http/WebUtilities/src/ReasonPhrases.cs b/src/Http/WebUtilities/src/ReasonPhrases.cs
@@ -66,6 +66,7 @@ public static class ReasonPhrases
         { 429, "Too Many Requests" },
         { 431, "Request Header Fields Too Large" },
         { 451, "Unavailable For Legal Reasons" },
+        { 499, "Client Closed Request" },
 
         { 500, "Internal Server Error" },
         { 501, "Not Implemented" },

diff --git a/...Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs b/...Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs
@@ -101,6 +101,18 @@ public async Task Invoke(HttpContext context)
         }
         catch (Exception ex)
         {
+            if ((ex is OperationCanceledException || ex is IOException) && context.RequestAborted.IsCancellationRequested)
+            {
+                _logger.RequestAbortedException();
+
+                if (!context.Response.HasStarted)
+                {
+                    context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
+                }
+
+                return;
+            }
+
             _logger.UnhandledException(ex);
 
             if (context.Response.HasStarted)

diff --git a/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs b/src/Middleware/Diagnostics/src/DiagnosticsLoggerExtensions.cs
@@ -11,6 +11,9 @@ internal static partial class DiagnosticsLoggerExtensions
     [LoggerMessage(1, LogLevel.Error, "An unhandled exception has occurred while executing the request.", EventName = "UnhandledException")]
     public static partial void UnhandledException(this ILogger logger, Exception exception);
 
+    [LoggerMessage(4, LogLevel.Debug, "The request was aborted by the client.", EventName = "RequestAborted")]
+    public static partial void RequestAbortedException(this ILogger logger);
+
     // ExceptionHandlerMiddleware
     [LoggerMessage(2, LogLevel.Warning, "The response has already started, the error handler will not be executed.", EventName = "ResponseStarted")]
     public static partial void ResponseStartedErrorHandler(this ILogger logger);

diff --git a/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs b/src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs
@@ -112,6 +112,18 @@ static async Task Awaited(ExceptionHandlerMiddlewareImpl middleware, HttpContext
 
     private async Task HandleException(HttpContext context, ExceptionDispatchInfo edi)
     {
+        if ((edi.SourceException is OperationCanceledException || edi.SourceException is IOException) && context.RequestAborted.IsCancellationRequested)
+        {
+            _logger.RequestAbortedException();
+
+            if (!context.Response.HasStarted)
+            {
+                context.Response.StatusCode = StatusCodes.Status499ClientClosedRequest;
+            }
+
+            return;
+        }
+
         _logger.UnhandledException(edi.SourceException);
         // We can't do anything if the response has already started, just abort.
         if (context.Response.HasStarted)

diff --git a/src/Servers/HttpSys/src/LoggerEventIds.cs b/src/Servers/HttpSys/src/LoggerEventIds.cs
@@ -53,4 +53,5 @@ internal static class LoggerEventIds
     public const int ListenerDisposing = 46;
     public const int RequestValidationFailed = 47;
     public const int CreateDisconnectTokenError = 48;
+    public const int RequestAborted = 49;
 }
diff --git a/src/Servers/HttpSys/src/RequestProcessing/RequestContextLog.cs b/src/Servers/HttpSys/src/RequestProcessing/RequestContextLog.cs
@@ -15,4 +15,7 @@ internal static partial class RequestContextLog
 
     [LoggerMessage(LoggerEventIds.RequestsDrained, LogLevel.Information, "All requests drained.", EventName = "RequestsDrained")]
     public static partial void RequestsDrained(ILogger logger);
+
+    [LoggerMessage(LoggerEventIds.RequestAborted, LogLevel.Debug, "The request was aborted by the client.", EventName = "RequestAborted")]
+    public static partial void RequestAborted(ILogger logger);
 }
diff --git a/src/Servers/HttpSys/src/RequestProcessing/RequestContextOfT.cs b/src/Servers/HttpSys/src/RequestProcessing/RequestContextOfT.cs
@@ -56,7 +56,14 @@ public override async Task ExecuteAsync()
             {
                 applicationException = ex;
 
-                Log.RequestProcessError(Logger, ex);
+                if ((ex is OperationCanceledException || ex is IOException) && DisconnectToken.IsCancellationRequested)
+                {
+                    Log.RequestAborted(Logger);
+                }
+                else
+                {
+                    Log.RequestProcessError(Logger, ex);
+                }
                 if (Response.HasStarted)
                 {
                     // Otherwise the default is Cancel = 0x8 (h2) or 0x010c (h3).
@@ -84,6 +91,10 @@ public override async Task ExecuteAsync()
                     {
                         SetFatalResponse(badHttpRequestException.StatusCode);
                     }
+                    else if ((ex is OperationCanceledException || ex is IOException) && DisconnectToken.IsCancellationRequested)
+                    {
+                        SetFatalResponse(StatusCodes.Status499ClientClosedRequest);
+                    }
                     else
                     {
                         SetFatalResponse(StatusCodes.Status500InternalServerError);

diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.Log.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.Log.cs
@@ -24,5 +24,8 @@ public static void ConnectionBadRequest(ILogger logger, string connectionId, Mic
 
         [LoggerMessage(4, LogLevel.Debug, @"Connection id ""{ConnectionId}"" bad request data: ""{message}""", EventName = nameof(ConnectionBadRequest))]
         private static partial void ConnectionBadRequest(ILogger logger, string connectionId, string message, Microsoft.AspNetCore.Http.BadHttpRequestException ex);
+
+        [LoggerMessage(5, LogLevel.Debug, @"Connection ID ""{ConnectionId}"", Request ID ""{TraceIdentifier}"": The request was aborted by the client.", EventName = "RequestAborted")]
+        public static partial void RequestAborted(ILogger logger, string connectionId, string traceIdentifier);
     }
 }
diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContext.cs
@@ -719,6 +719,11 @@ protected void ReportApplicationError(Exception ex)
         Log.ApplicationError(_logger, ((IHttpConnectionFeature)this).ConnectionId, ((IHttpRequestIdentifierFeature)this).TraceIdentifier, ex);
     }
 
+    protected void ReportRequestAborted()
+    {
+        Log.RequestAborted(_logger, ((IHttpConnectionFeature)this).ConnectionId, ((IHttpRequestIdentifierFeature)this).TraceIdentifier);
+    }
+
     public void PostCompletion(NativeMethods.REQUEST_NOTIFICATION_STATUS requestNotificationStatus)
     {
         NativeMethods.HttpSetCompletionStatus(_requestNativeHandle, requestNotificationStatus);

diff --git a/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs b/src/Servers/IIS/IIS/src/Core/IISHttpContextOfT.cs
@@ -4,6 +4,7 @@
 using System.Buffers;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Hosting.Server;
+using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Server.IIS.Core;
@@ -43,7 +44,15 @@ public override async Task<bool> ProcessRequestAsync()
             }
             catch (Exception ex)
             {
-                ReportApplicationError(ex);
+                if ((ex is OperationCanceledException || ex is IOException) && ClientDisconnected)
+                {
+                    ReportRequestAborted();
+                }
+                else
+                {
+                    ReportApplicationError(ex);
+                }
+
                 success = false;
             }
 
@@ -82,9 +91,8 @@ public override async Task<bool> ProcessRequestAsync()
             }
             else if (!HasResponseStarted && _requestRejectedException == null)
             {
-                // If the request was aborted and no response was sent, there's no
-                // meaningful status code to log.
-                StatusCode = 0;
+                // If the request was aborted and no response was sent, we use status code 499 for logging               
+                StatusCode = ClientDisconnected ? StatusCodes.Status499ClientClosedRequest : 0;
                 success = false;
             }
 

diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs
@@ -698,7 +698,14 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
             }
             catch (Exception ex)
             {
-                ReportApplicationError(ex);
+                if ((ex is OperationCanceledException || ex is IOException) && _connectionAborted)
+                {
+                    Log.RequestAborted(ConnectionId, TraceIdentifier);
+                }
+                else
+                {
+                    ReportApplicationError(ex);
+                }
             }
 
             KestrelEventSource.Log.RequestStop(this);
@@ -739,9 +746,8 @@ private async Task ProcessRequests<TContext>(IHttpApplication<TContext> applicat
                 }
                 else if (!HasResponseStarted)
                 {
-                    // If the request was aborted and no response was sent, there's no
-                    // meaningful status code to log.
-                    StatusCode = 0;
+                    // If the request was aborted and no response was sent, we use status code 499 for logging                    
+                    StatusCode = StatusCodes.Status499ClientClosedRequest;
                 }
             }
 

diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/ReasonPhrases.cs b/src/Servers/Kestrel/Core/src/Internal/Http/ReasonPhrases.cs
@@ -64,6 +64,7 @@ internal static class ReasonPhrases
     private static readonly byte[] _bytesStatus429 = CreateStatusBytes(StatusCodes.Status429TooManyRequests);
     private static readonly byte[] _bytesStatus431 = CreateStatusBytes(StatusCodes.Status431RequestHeaderFieldsTooLarge);
     private static readonly byte[] _bytesStatus451 = CreateStatusBytes(StatusCodes.Status451UnavailableForLegalReasons);
+    private static readonly byte[] _bytesStatus499 = CreateStatusBytes(StatusCodes.Status499ClientClosedRequest);
 
     private static readonly byte[] _bytesStatus500 = CreateStatusBytes(StatusCodes.Status500InternalServerError);
     private static readonly byte[] _bytesStatus501 = CreateStatusBytes(StatusCodes.Status501NotImplemented);
@@ -149,6 +150,7 @@ public static byte[] ToStatusBytes(int statusCode, string? reasonPhrase = null)
             StatusCodes.Status429TooManyRequests => _bytesStatus429,
             StatusCodes.Status431RequestHeaderFieldsTooLarge => _bytesStatus431,
             StatusCodes.Status451UnavailableForLegalReasons => _bytesStatus451,
+            StatusCodes.Status499ClientClosedRequest => _bytesStatus499,
 
             StatusCodes.Status500InternalServerError => _bytesStatus500,
             StatusCodes.Status501NotImplemented => _bytesStatus501,

diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.General.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/KestrelTrace.General.cs
@@ -64,6 +64,11 @@ public void Http3DisabledWithHttp1AndNoTls(EndPoint endPoint)
         GeneralLog.Http3DisabledWithHttp1AndNoTls(_generalLogger, endPoint);
     }
 
+    public void RequestAborted(string connectionId, string traceIdentifier)
+    {
+        GeneralLog.RequestAbortedException(_generalLogger, connectionId, traceIdentifier);
+    }
+
     private static partial class GeneralLog
     {
         [LoggerMessage(13, LogLevel.Error, @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": An unhandled exception was thrown by the application.", EventName = "ApplicationError")]
@@ -99,6 +104,9 @@ private static partial class GeneralLog
         [LoggerMessage(65, LogLevel.Warning, "HTTP/3 is not enabled for {Endpoint}. HTTP/3 requires TLS. Connections to this endpoint will use HTTP/1.1.", EventName = "Http3DisabledWithHttp1AndNoTls")]
         public static partial void Http3DisabledWithHttp1AndNoTls(ILogger logger, EndPoint endPoint);
 
-        // Highest shared ID is 65. New consecutive IDs start at 66
+        [LoggerMessage(66, LogLevel.Debug, @"Connection id ""{ConnectionId}"", Request id ""{TraceIdentifier}"": The request was aborted by the client.", EventName = "RequestAborted")]
+        public static partial void RequestAbortedException(ILogger logger, string connectionId, string traceIdentifier);
+
+        // Highest shared ID is 66. New consecutive IDs start at 67
     }
 }
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs
@@ -294,7 +294,7 @@ public Task ResponseStatusCodeSetBeforeHttpContextDisposeRequestAborted()
                 return Task.CompletedTask;
             },
             expectedClientStatusCode: null,
-            expectedServerStatusCode: 0);
+            expectedServerStatusCode: (HttpStatusCode)499);
     }
 
     [Fact]
@@ -309,7 +309,7 @@ public Task ResponseStatusCodeSetBeforeHttpContextDisposeRequestAbortedAppExcept
                 throw new Exception();
             },
             expectedClientStatusCode: null,
-            expectedServerStatusCode: 0);
+            expectedServerStatusCode: (HttpStatusCode)499);
     }
 
     [Fact]