Added QuicException.TransportErrorCode (#88550)

AlexRadch · ManickaP · web-flow · commit c0971e5e3e95 · 2023-07-17T16:21:36.000+02:00
* Added QuicException.TransportErrorCode * Code review by Tomas Weinfurt @wfurt * @ManickaP Marie Píchová code review * @ManickaP Marie Píchová code review * @ManickaP Marie Píchová code review * Feedback about error code casts --------- Co-authored-by: ManickaP <mapichov@microsoft.com>
diff --git a/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs b/src/libraries/System.Net.Quic/ref/System.Net.Quic.cs
@@ -64,6 +64,7 @@ public sealed partial class QuicException : System.IO.IOException
     {
         public QuicException(System.Net.Quic.QuicError error, long? applicationErrorCode, string message) { }
         public long? ApplicationErrorCode { get { throw null; } }
+        public long? TransportErrorCode { get { throw null; } }
         public System.Net.Quic.QuicError QuicError { get { throw null; } }
     }
     public sealed partial class QuicListener : System.IAsyncDisposable
diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicHelpers.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicHelpers.cs
@@ -67,11 +67,7 @@ internal static unsafe T GetMsQuicParameter<T>(MsQuicSafeHandle handle, uint par
             &length,
             (byte*)&value);
 
-        if (StatusFailed(status))
-        {
-            throw ThrowHelper.GetExceptionForMsQuicStatus(status, $"GetParam({handle}, {parameter}) failed");
-        }
-
+        ThrowHelper.ThrowIfMsQuicError(status, $"GetParam({handle}, {parameter}) failed");
         return value;
     }
 
@@ -84,9 +80,6 @@ internal static unsafe void SetMsQuicParameter<T>(MsQuicSafeHandle handle, uint
             (uint)sizeof(T),
             (byte*)&value);
 
-        if (StatusFailed(status))
-        {
-            throw ThrowHelper.GetExceptionForMsQuicStatus(status, $"SetParam({handle}, {parameter}) failed");
-        }
+        ThrowHelper.ThrowIfMsQuicError(status, $"SetParam({handle}, {parameter}) failed");
     }
 }
diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/ThrowHelper.cs
@@ -1,12 +1,12 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using Microsoft.Quic;
 using System.Security.Authentication;
 using System.Net.Security;
 using System.Net.Sockets;
 using static Microsoft.Quic.MsQuic;
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
 
 namespace System.Net.Quic;
 
@@ -38,21 +38,21 @@ internal static bool TryGetStreamExceptionForMsQuicStatus(int status, [NotNullWh
         else if (status == QUIC_STATUS_INVALID_STATE)
         {
             // If status == QUIC_STATUS_INVALID_STATE, we have closed the connection.
-            exception = ThrowHelper.GetOperationAbortedException();
+            exception = GetOperationAbortedException();
             return true;
         }
         else if (StatusFailed(status))
         {
-            exception = ThrowHelper.GetExceptionForMsQuicStatus(status);
+            exception = GetExceptionForMsQuicStatus(status);
             return true;
         }
         exception = null;
         return false;
     }
 
-    internal static Exception GetExceptionForMsQuicStatus(int status, string? message = null)
+    internal static Exception GetExceptionForMsQuicStatus(int status, long? errorCode = default, string? message = null)
     {
-        Exception ex = GetExceptionInternal(status, message);
+        Exception ex = GetExceptionInternal(status, errorCode, message);
         if (status != 0)
         {
             // Include the raw MsQuic status in the HResult property for better diagnostics
@@ -61,17 +61,17 @@ internal static Exception GetExceptionForMsQuicStatus(int status, string? messag
 
         return ex;
 
-        static Exception GetExceptionInternal(int status, string? message)
+        static Exception GetExceptionInternal(int status, long? errorCode, string? message)
         {
             //
             // Start by checking for statuses mapped to QuicError enum
             //
-            if (status == QUIC_STATUS_CONNECTION_REFUSED) return new QuicException(QuicError.ConnectionRefused, null, SR.net_quic_connection_refused);
-            if (status == QUIC_STATUS_CONNECTION_TIMEOUT) return new QuicException(QuicError.ConnectionTimeout, null, SR.net_quic_timeout);
-            if (status == QUIC_STATUS_VER_NEG_ERROR) return new QuicException(QuicError.VersionNegotiationError, null, SR.net_quic_ver_neg_error);
-            if (status == QUIC_STATUS_CONNECTION_IDLE) return new QuicException(QuicError.ConnectionIdle, null, SR.net_quic_connection_idle);
-            if (status == QUIC_STATUS_PROTOCOL_ERROR) return new QuicException(QuicError.TransportError, null, SR.net_quic_protocol_error);
-            if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, SR.net_quic_protocol_error);
+            if (status == QUIC_STATUS_CONNECTION_REFUSED) return new QuicException(QuicError.ConnectionRefused, null, errorCode, SR.net_quic_connection_refused);
+            if (status == QUIC_STATUS_CONNECTION_TIMEOUT) return new QuicException(QuicError.ConnectionTimeout, null, errorCode, SR.net_quic_timeout);
+            if (status == QUIC_STATUS_VER_NEG_ERROR) return new QuicException(QuicError.VersionNegotiationError, null, errorCode, SR.net_quic_ver_neg_error);
+            if (status == QUIC_STATUS_CONNECTION_IDLE) return new QuicException(QuicError.ConnectionIdle, null, errorCode, SR.net_quic_connection_idle);
+            if (status == QUIC_STATUS_PROTOCOL_ERROR) return new QuicException(QuicError.TransportError, null, errorCode, SR.net_quic_protocol_error);
+            if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, errorCode, SR.net_quic_protocol_error);
 
             //
             // Transport errors will throw SocketException
@@ -81,7 +81,7 @@ static Exception GetExceptionInternal(int status, string? message)
             if (status == QUIC_STATUS_UNREACHABLE) return new SocketException((int)SocketError.HostUnreachable);
 
             //
-            // TLS and certificate erros throw AuthenticationException to match SslStream
+            // TLS and certificate errors throw AuthenticationException to match SslStream
             //
             if (status == QUIC_STATUS_TLS_ERROR ||
                 status == QUIC_STATUS_CERT_EXPIRED ||
@@ -125,11 +125,12 @@ static Exception GetExceptionInternal(int status, string? message)
         }
     }
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
     internal static void ThrowIfMsQuicError(int status, string? message = null)
     {
         if (StatusFailed(status))
         {
-            throw GetExceptionForMsQuicStatus(status, message);
+            throw GetExceptionForMsQuicStatus(status, message: message);
         }
     }
 
diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs
@@ -42,7 +42,7 @@ public sealed partial class QuicConnection : IAsyncDisposable
     /// <summary>
     /// The actual secret structure wrapper passed to MsQuic.
     /// </summary>
-    private MsQuicTlsSecret? _tlsSecret;
+    private readonly MsQuicTlsSecret? _tlsSecret;
 #endif
 
     /// <summary>
@@ -304,7 +304,7 @@ private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options,
 
             // RFC 6066 forbids IP literals
             // DNI mapping is handled by MsQuic
-            var hostname = TargetHostNameHelper.IsValidAddress(options.ClientAuthenticationOptions.TargetHost)
+            string hostname = TargetHostNameHelper.IsValidAddress(options.ClientAuthenticationOptions.TargetHost)
                 ? string.Empty
                 : options.ClientAuthenticationOptions.TargetHost ?? string.Empty;
 
@@ -492,9 +492,7 @@ private unsafe int HandleEventConnected(ref CONNECTED_DATA data)
     }
     private unsafe int HandleEventShutdownInitiatedByTransport(ref SHUTDOWN_INITIATED_BY_TRANSPORT_DATA data)
     {
-        // TODO: we should propagate transport error code.
-        // https://github.com/dotnet/runtime/issues/72666
-        Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetExceptionForMsQuicStatus(data.Status));
+        Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(ThrowHelper.GetExceptionForMsQuicStatus(data.Status, (long)data.ErrorCode));
         _connectedTcs.TrySetException(exception);
         _acceptQueue.Writer.TryComplete(exception);
         return QUIC_STATUS_SUCCESS;
diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicException.cs
@@ -17,7 +17,18 @@ public sealed class QuicException : IOException
     /// <param name="applicationErrorCode">The application protocol error code associated with the error.</param>
     /// <param name="message">The message for the exception.</param>
     public QuicException(QuicError error, long? applicationErrorCode, string message)
-        : this(error, applicationErrorCode, message, null)
+        : this(error, applicationErrorCode, null, message, null)
+    { }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref='QuicException'/> class.
+    /// </summary>
+    /// <param name="error">The error associated with the exception.</param>
+    /// <param name="applicationErrorCode">The application protocol error code associated with the error.</param>
+    /// <param name="transportErrorCode">The transport protocol error code associated with the error.</param>
+    /// <param name="message">The message for the exception.</param>
+    internal QuicException(QuicError error, long? applicationErrorCode, long? transportErrorCode, string message)
+        : this(error, applicationErrorCode, transportErrorCode, message, null)
     { }
 
     /// <summary>
@@ -28,10 +39,23 @@ public QuicException(QuicError error, long? applicationErrorCode, string message
     /// <param name="message">The message for the exception.</param>
     /// <param name="innerException">The exception that is the cause of the current exception, or a null reference if no inner exception is specified.</param>
     internal QuicException(QuicError error, long? applicationErrorCode, string message, Exception? innerException)
+        : this(error, applicationErrorCode, null, message, innerException)
+    { }
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref='QuicException'/> class.
+    /// </summary>
+    /// <param name="error">The error associated with the exception.</param>
+    /// <param name="applicationErrorCode">The application protocol error code associated with the error.</param>
+    /// <param name="transportErrorCode">The transport protocol error code associated with the error.</param>
+    /// <param name="message">The message for the exception.</param>
+    /// <param name="innerException">The exception that is the cause of the current exception, or a null reference if no inner exception is specified.</param>
+    internal QuicException(QuicError error, long? applicationErrorCode, long? transportErrorCode, string message, Exception? innerException)
         : base(message, innerException)
     {
         QuicError = error;
         ApplicationErrorCode = applicationErrorCode;
+        TransportErrorCode = transportErrorCode;
     }
 
     /// <summary>
@@ -46,4 +70,9 @@ internal QuicException(QuicError error, long? applicationErrorCode, string messa
     /// This property contains the error code set by the application layer when closing the connection (<see cref="QuicError.ConnectionAborted"/>) or closing a read/write direction of a QUIC stream (<see cref="QuicError.StreamAborted"/>). Contains null for all other errors.
     /// </remarks>
     public long? ApplicationErrorCode { get; }
+
+    /// <summary>
+    /// The transport protocol error code associated with the error.
+    /// </summary>
+    public long? TransportErrorCode { get; }
 }
diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs
@@ -581,13 +581,9 @@ private unsafe int HandleEventShutdownComplete(ref SHUTDOWN_COMPLETE_DATA data)
                 // It's local shutdown by app, this side called QuicConnection.CloseAsync, throw QuicError.OperationAborted.
                 (shutdownByApp: true, closedRemotely: false) => ThrowHelper.GetOperationAbortedException(),
                 // It's remote shutdown by transport, we received a CONNECTION_CLOSE frame with a QUIC transport error code, throw error based on the status.
-                // TODO: we should propagate the transport error code
-                // https://github.com/dotnet/runtime/issues/72666
-                (shutdownByApp: false, closedRemotely: true) => ThrowHelper.GetExceptionForMsQuicStatus(data.ConnectionCloseStatus, $"Shutdown by transport {data.ConnectionErrorCode}"),
+                (shutdownByApp: false, closedRemotely: true) => ThrowHelper.GetExceptionForMsQuicStatus(data.ConnectionCloseStatus, (long)data.ConnectionErrorCode, $"Shutdown by transport {data.ConnectionErrorCode}"),
                 // It's local shutdown by transport, most likely due to a timeout, throw error based on the status.
-                // TODO: we should propagate the transport error code
-                // https://github.com/dotnet/runtime/issues/72666
-                (shutdownByApp: false, closedRemotely: false) => ThrowHelper.GetExceptionForMsQuicStatus(data.ConnectionCloseStatus),
+                (shutdownByApp: false, closedRemotely: false) => ThrowHelper.GetExceptionForMsQuicStatus(data.ConnectionCloseStatus, (long)data.ConnectionErrorCode),
             };
             _startedTcs.TrySetException(exception);
             _receiveTcs.TrySetException(exception, final: true);

Original file line number	Diff line number	Diff line change
`@@ -64,6 +64,7 @@ public sealed partial class QuicException : System.IO.IOException`
`64`	`64`	`{`
`65`	`65`	`public QuicException(System.Net.Quic.QuicError error, long? applicationErrorCode, string message) { }`
`66`	`66`	`public long? ApplicationErrorCode { get { throw null; } }`
	`67`	`+ public long? TransportErrorCode { get { throw null; } }`
`67`	`68`	`public System.Net.Quic.QuicError QuicError { get { throw null; } }`
`68`	`69`	`}`
`69`	`70`	`public sealed partial class QuicListener : System.IAsyncDisposable`
Original file line number	Diff line number	Diff line change
`@@ -1,12 +1,12 @@`
`1`	`1`	`// Licensed to the .NET Foundation under one or more agreements.`
`2`	`2`	`// The .NET Foundation licenses this file to you under the MIT license.`
`3`	`3`
`4`		`-using Microsoft.Quic;`
`5`	`4`	`using System.Security.Authentication;`
`6`	`5`	`using System.Net.Security;`
`7`	`6`	`using System.Net.Sockets;`
`8`	`7`	`using static Microsoft.Quic.MsQuic;`
`9`	`8`	`using System.Diagnostics.CodeAnalysis;`
	`9`	`+using System.Runtime.CompilerServices;`
`10`	`10`
`11`	`11`	`namespace System.Net.Quic;`
`12`	`12`
`@@ -38,21 +38,21 @@ internal static bool TryGetStreamExceptionForMsQuicStatus(int status, [NotNullWh`
`38`	`38`	`else if (status == QUIC_STATUS_INVALID_STATE)`
`39`	`39`	`{`
`40`	`40`	`// If status == QUIC_STATUS_INVALID_STATE, we have closed the connection.`
`41`		`- exception = ThrowHelper.GetOperationAbortedException();`
	`41`	`+ exception = GetOperationAbortedException();`
`42`	`42`	`return true;`
`43`	`43`	`}`
`44`	`44`	`else if (StatusFailed(status))`
`45`	`45`	`{`
`46`		`- exception = ThrowHelper.GetExceptionForMsQuicStatus(status);`
	`46`	`+ exception = GetExceptionForMsQuicStatus(status);`
`47`	`47`	`return true;`
`48`	`48`	`}`
`49`	`49`	`exception = null;`
`50`	`50`	`return false;`
`51`	`51`	`}`
`52`	`52`
`53`		`- internal static Exception GetExceptionForMsQuicStatus(int status, string? message = null)`
	`53`	`+ internal static Exception GetExceptionForMsQuicStatus(int status, long? errorCode = default, string? message = null)`
`54`	`54`	`{`
`55`		`- Exception ex = GetExceptionInternal(status, message);`
	`55`	`+ Exception ex = GetExceptionInternal(status, errorCode, message);`
`56`	`56`	`if (status != 0)`
`57`	`57`	`{`
`58`	`58`	`// Include the raw MsQuic status in the HResult property for better diagnostics`
`@@ -61,17 +61,17 @@ internal static Exception GetExceptionForMsQuicStatus(int status, string? messag`
`61`	`61`
`62`	`62`	`return ex;`
`63`	`63`
`64`		`- static Exception GetExceptionInternal(int status, string? message)`
	`64`	`+ static Exception GetExceptionInternal(int status, long? errorCode, string? message)`
`65`	`65`	`{`
`66`	`66`	`//`
`67`	`67`	`// Start by checking for statuses mapped to QuicError enum`
`68`	`68`	`//`
`69`		`- if (status == QUIC_STATUS_CONNECTION_REFUSED) return new QuicException(QuicError.ConnectionRefused, null, SR.net_quic_connection_refused);`
`70`		`- if (status == QUIC_STATUS_CONNECTION_TIMEOUT) return new QuicException(QuicError.ConnectionTimeout, null, SR.net_quic_timeout);`
`71`		`- if (status == QUIC_STATUS_VER_NEG_ERROR) return new QuicException(QuicError.VersionNegotiationError, null, SR.net_quic_ver_neg_error);`
`72`		`- if (status == QUIC_STATUS_CONNECTION_IDLE) return new QuicException(QuicError.ConnectionIdle, null, SR.net_quic_connection_idle);`
`73`		`- if (status == QUIC_STATUS_PROTOCOL_ERROR) return new QuicException(QuicError.TransportError, null, SR.net_quic_protocol_error);`
`74`		`- if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, SR.net_quic_protocol_error);`
	`69`	`+ if (status == QUIC_STATUS_CONNECTION_REFUSED) return new QuicException(QuicError.ConnectionRefused, null, errorCode, SR.net_quic_connection_refused);`
	`70`	`+ if (status == QUIC_STATUS_CONNECTION_TIMEOUT) return new QuicException(QuicError.ConnectionTimeout, null, errorCode, SR.net_quic_timeout);`
	`71`	`+ if (status == QUIC_STATUS_VER_NEG_ERROR) return new QuicException(QuicError.VersionNegotiationError, null, errorCode, SR.net_quic_ver_neg_error);`
	`72`	`+ if (status == QUIC_STATUS_CONNECTION_IDLE) return new QuicException(QuicError.ConnectionIdle, null, errorCode, SR.net_quic_connection_idle);`
	`73`	`+ if (status == QUIC_STATUS_PROTOCOL_ERROR) return new QuicException(QuicError.TransportError, null, errorCode, SR.net_quic_protocol_error);`
	`74`	`+ if (status == QUIC_STATUS_ALPN_IN_USE) return new QuicException(QuicError.AlpnInUse, null, errorCode, SR.net_quic_protocol_error);`
`75`	`75`
`76`	`76`	`//`
`77`	`77`	`// Transport errors will throw SocketException`
`@@ -81,7 +81,7 @@ static Exception GetExceptionInternal(int status, string? message)`
`81`	`81`	`if (status == QUIC_STATUS_UNREACHABLE) return new SocketException((int)SocketError.HostUnreachable);`
`82`	`82`
`83`	`83`	`//`
`84`		`- // TLS and certificate erros throw AuthenticationException to match SslStream`
	`84`	`+ // TLS and certificate errors throw AuthenticationException to match SslStream`
`85`	`85`	`//`
`86`	`86`	`if (status == QUIC_STATUS_TLS_ERROR \|\|`
`87`	`87`	`status == QUIC_STATUS_CERT_EXPIRED \|\|`
`@@ -125,11 +125,12 @@ static Exception GetExceptionInternal(int status, string? message)`
`125`	`125`	`}`
`126`	`126`	`}`
`127`	`127`
	`128`	`+ [MethodImpl(MethodImplOptions.AggressiveInlining)]`
`128`	`129`	`internal static void ThrowIfMsQuicError(int status, string? message = null)`
`129`	`130`	`{`
`130`	`131`	`if (StatusFailed(status))`
`131`	`132`	`{`
`132`		`- throw GetExceptionForMsQuicStatus(status, message);`
	`133`	`+ throw GetExceptionForMsQuicStatus(status, message: message);`
`133`	`134`	`}`
`134`	`135`	`}`
`135`	`136`