Clarify websocket activity failure (#2325)

microsoft · Nov 15, 2023 · 47231e9 · 47231e9
1 parent 48e3f7e
commit 47231e9
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 6 deletions.
diff --git a/src/ReverseProxy/Forwarder/ForwarderError.cs b/src/ReverseProxy/Forwarder/ForwarderError.cs
@@ -103,4 +103,9 @@ public enum ForwarderError : int
     /// Failed while creating the request message.
     /// </summary>
     RequestCreation,
+
+    /// <summary>
+    /// An upgraded request was idle and canceled due to the activity timeout.
+    /// </summary>
+    UpgradeActivityTimeout,
 }
diff --git a/src/ReverseProxy/Forwarder/HttpForwarder.cs b/src/ReverseProxy/Forwarder/HttpForwarder.cs
@@ -754,7 +754,7 @@ private async ValueTask<ForwarderError> HandleUpgradedResponse(HttpContext conte
         var (firstResult, firstException) = await firstTask;
         if (firstResult != StreamCopyResult.Success)
         {
-            error = ReportResult(context, requestFinishedFirst, firstResult, firstException);
+            error = ReportResult(context, requestFinishedFirst, firstResult, firstException, activityCancellationSource);
             // Cancel the other direction
             activityCancellationSource.Cancel();
             // Wait for this to finish before exiting so the resources get cleaned up properly.
@@ -772,7 +772,7 @@ private async ValueTask<ForwarderError> HandleUpgradedResponse(HttpContext conte
             var (secondResult, secondException) = await secondTask;
             if (!cancelReads && secondResult != StreamCopyResult.Success)
             {
-                error = ReportResult(context, !requestFinishedFirst, secondResult, secondException!);
+                error = ReportResult(context, !requestFinishedFirst, secondResult, secondException!, activityCancellationSource);
             }
             else
             {
@@ -782,7 +782,7 @@ private async ValueTask<ForwarderError> HandleUpgradedResponse(HttpContext conte
 
         return error;
 
-        ForwarderError ReportResult(HttpContext context, bool request, StreamCopyResult result, Exception exception)
+        ForwarderError ReportResult(HttpContext context, bool request, StreamCopyResult result, Exception exception, ActivityCancellationTokenSource activityCancellationTokenSource)
         {
             var error = result switch
             {
@@ -791,6 +791,10 @@ ForwarderError ReportResult(HttpContext context, bool request, StreamCopyResult
                 StreamCopyResult.Canceled => request ? ForwarderError.UpgradeRequestCanceled : ForwarderError.UpgradeResponseCanceled,
                 _ => throw new NotImplementedException(result.ToString()),
             };
+            if (activityCancellationSource.IsCancellationRequested && !activityCancellationSource.CancelledByLinkedToken)
+            {
+                error = ForwarderError.UpgradeActivityTimeout;
+            }
             ReportProxyError(context, error, exception);
             return error;
         }
@@ -1046,6 +1050,7 @@ private static string GetMessage(ForwarderError error)
                 ForwarderError.UpgradeResponseCanceled => "Copying the upgraded response body was canceled.",
                 ForwarderError.UpgradeResponseClient => "The client reported an error when copying the upgraded response body.",
                 ForwarderError.UpgradeResponseDestination => "The destination reported an error when copying the upgraded response body.",
+                ForwarderError.UpgradeActivityTimeout => "The WebSocket connection was closed after being idle longer than the Activity Timeout.",
                 ForwarderError.NoAvailableDestinations => throw new NotImplementedException(), // Not used in this class
                 _ => throw new NotImplementedException(error.ToString()),
             };

diff --git a/src/ReverseProxy/WebSocketsTelemetry/WebSocketCloseReason.cs b/src/ReverseProxy/WebSocketsTelemetry/WebSocketCloseReason.cs
@@ -10,4 +10,5 @@ internal enum WebSocketCloseReason : int
     ServerGracefulClose,
     ClientDisconnect,
     ServerDisconnect,
+    ActivityTimeout,
 }
diff --git a/src/ReverseProxy/WebSocketsTelemetry/WebSocketsTelemetryStream.cs b/src/ReverseProxy/WebSocketsTelemetry/WebSocketsTelemetryStream.cs
@@ -59,6 +59,9 @@ public WebSocketCloseReason GetCloseReason(HttpContext context)
             ForwarderError.UpgradeRequestDestination => WebSocketCloseReason.ServerDisconnect,
             ForwarderError.UpgradeResponseDestination => WebSocketCloseReason.ServerDisconnect,
 
+            // Activity Timeout
+            ForwarderError.UpgradeActivityTimeout => WebSocketCloseReason.ActivityTimeout,
+
             // Both sides gracefully closed the underlying connection without sending a WebSocket close,
             // or the server closed the connection and we canceled the client and suppressed the errors.
             null => WebSocketCloseReason.ServerDisconnect,

diff --git a/test/ReverseProxy.Tests/Forwarder/HttpForwarderTests.cs b/test/ReverseProxy.Tests/Forwarder/HttpForwarderTests.cs
@@ -701,9 +701,7 @@ public async Task UpgradableRequest_CancelsIfIdle()
 
         Assert.Equal(StatusCodes.Status101SwitchingProtocols, httpContext.Response.StatusCode);
 
-        // When both are idle it's a race which gets reported as canceled first.
-        Assert.True(ForwarderError.UpgradeRequestClient == result
-            || ForwarderError.UpgradeResponseDestination == result);
+        Assert.Equal(ForwarderError.UpgradeActivityTimeout, result);
 
         events.AssertContainProxyStages(upgrade: true);
     }