Skip to content

Align SqlException Numbers across platforms #3461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
+69 −3
Draft

Align SqlException Numbers across platforms #3461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b77ddb8
Align SqlException Numbers across platforms
David-Engel Jul 3, 2025
79703ec
Apply suggested changes
David-Engel Jul 7, 2025
f55b272
Better capture error scenarios in TCP managed SNI.
David-Engel Jul 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniError.netcore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#if NET

using System;
using System.ComponentModel;
using System.Net.Sockets;

namespace Microsoft.Data.SqlClient.ManagedSni
{
Expand Down Expand Up @@ -41,6 +43,18 @@ public SniError(SniProviders provider, uint sniErrorCode, Exception sniException
function = string.Empty;
this.provider = provider;
nativeError = nativeErrorCode;
if (nativeErrorCode == 0)
{
if (sniException is SocketException socketException)
{
// SocketErrorCode values are cross-plat consistent in .NET (matching native Windows error codes)
nativeError = (uint)socketException.SocketErrorCode;
Copy link
Member

@cheenamalhotra cheenamalhotra Jul 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little concerning, as it will translate SocketError -1 to a positive long (4294967295) and will likely throw an exception here, as there is no unchecked block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same negative int -> uint cast is happening below with Win32Exception too. But in my test program, there is no unchecked exception thrown. The int -1 value happily becomes uint 4294967295. There is no loss of data since both types are 32-bit. I can't tell from this code whether or not the caller expects a large unsigned value for nativeError - none of these public fields are documented. It looks like nativeError ends up in SNIErrorDetails.nativeError, which is also a uint, so I guess that's fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But can we consider moving away from uint? Since ErrorCodes can be negative too, I wonder if we should adjust accordingly now that we have the opportunity.

We also seem to cast it back to int later when creating SqlError:

SqlClient/src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines 1636 to 1637 in 9e5eb97

return new SqlError((int)details.NativeError, 0x00, TdsEnums.FATAL_ERROR_CLASS,
_server, errorMessage, details.Function, (int)details.LineNumber, win32ErrorCode);

}
else if (sniException is Win32Exception win32Exception)
{
nativeError = (uint)win32Exception.NativeErrorCode; // Replicates native SNI behavior
}
}
sniError = sniErrorCode;
errorMessage = string.Empty;
exception = sniException;
Expand Down
56 changes: 54 additions & 2 deletions src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,8 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,

IEnumerable<IPAddress> ipAddresses = GetHostAddressesSortedByPreference(serverName, ipPreference);

SocketException lastSocketException = null;

foreach (IPAddress ipAddress in ipAddresses)
{
bool isSocketSelected = false;
Expand Down Expand Up @@ -426,7 +428,7 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,
{
if (timeout.IsExpired)
{
return null;
throw new Win32Exception(258, "The operation has timed out.");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Localize error message?

}

int socketSelectTimeout =
Expand All @@ -442,10 +444,24 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,

Socket.Select(checkReadLst, checkWriteLst, checkErrorLst, socketSelectTimeout);
// nothing selected means timeout
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO,
"Socket.Select results: checkReadLst.Count: {0}, checkWriteLst.Count: {1}, checkErrorLst.Count: {2}",
checkReadLst.Count, checkWriteLst.Count, checkErrorLst.Count);
} while (checkReadLst.Count == 0 && checkWriteLst.Count == 0 && checkErrorLst.Count == 0);

// workaround: false positive socket.Connected on linux: https://github.com/dotnet/runtime/issues/55538
isConnected = socket.Connected && checkErrorLst.Count == 0;
if (!isConnected)
{
// Retrieve the socket error code
int socketErrorCode = (int)socket.GetSocketOption(SocketOptionLevel.Socket, SocketOptionName.Error);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird that GetSocketOption() returns object? , but then the docs don't describe when it could be null.

SocketError socketError = (SocketError)socketErrorCode;

SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.ERR,
"Socket connection failed. SocketError: {0} ({1})", socketError, socketErrorCode);

lastSocketException = new SocketException(socketErrorCode);
}
}

if (isConnected)
Expand All @@ -463,6 +479,8 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,
}
pendingDNSInfo = new SQLDNSInfo(cachedFQDN, iPv4String, iPv6String, port.ToString());
isSocketSelected = true;
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO,
"Connected to socket: {0}", socket.RemoteEndPoint);
return socket;
}
}
Expand All @@ -471,6 +489,7 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,
SqlClientEventSource.Log.TryAdvancedTraceEvent(
"{0}.{1}{2}THIS EXCEPTION IS BEING SWALLOWED: {3}",
nameof(SniTcpHandle), nameof(Connect), EventType.ERR, e);
lastSocketException = e;
}
finally
{
Expand All @@ -479,6 +498,14 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout,
}
}

if (lastSocketException != null)
{
SqlClientEventSource.Log.TryAdvancedTraceEvent(
"{0}.{1}{2}Last Socket Exception: {3}",
nameof(SniTcpHandle), nameof(Connect), EventType.ERR, lastSocketException);
throw lastSocketException;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to see some tests for these changes.

}

return null;
}
}
Expand Down Expand Up @@ -574,6 +601,20 @@ private static Socket ParallelConnect(IPAddress[] serverAddresses, int port, Tim
Socket.Select(checkReadLst, checkWriteLst, checkErrorLst, socketSelectTimeout);
// nothing selected means select timed out
} while (checkReadLst.Count == 0 && checkWriteLst.Count == 0 && checkErrorLst.Count == 0 && !timeout.IsExpired);
foreach (Socket socket in checkErrorLst)
{
// Retrieve the socket error code
int socketErrorCode = (int)socket.GetSocketOption(SocketOptionLevel.Socket, SocketOptionName.Error);
SocketError socketError = (SocketError)socketErrorCode;

// Log any failed sockets
SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO,
"Socket connection failed for {0}. SocketError: {1} ({2})",
sockets[socket], socketError, socketErrorCode);

lastError = new SocketException(socketErrorCode);
}

}
catch (SocketException e)
{
Expand All @@ -588,6 +629,7 @@ private static Socket ParallelConnect(IPAddress[] serverAddresses, int port, Tim
{
SqlClientEventSource.Log.TryAdvancedTraceEvent(
"{0}.{1}{2}ParallelConnect timeout expired.", nameof(SniTcpHandle), nameof(ParallelConnect), EventType.INFO);
// We will throw below after cleanup
break;
}

Expand Down Expand Up @@ -654,9 +696,19 @@ private static Socket ParallelConnect(IPAddress[] serverAddresses, int port, Tim

if (connectedSocket == null)
{
if (timeout.IsExpired)
{
throw new Win32Exception(258, "The operation has timed out.");
}

SqlClientEventSource.Log.TryAdvancedTraceEvent(
"{0}.{1}{2}No socket connections succeeded. Last error: {3}",
"{0}.{1}{2} No socket connections succeeded. Last error: {3}",
nameof(SniTcpHandle), nameof(ParallelConnect), EventType.ERR, lastError);

if (lastError != null)
{
throw lastError;
}
}

return connectedSocket;
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlClientEventSource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ internal long TryScopeEnterEvent(string className, [System.Runtime.CompilerServi
{
StringBuilder sb = new StringBuilder(className);
sb.Append(".").Append(memberName).Append(" | INFO | SCOPE | Entering Scope {0}");
return SNIScopeEnter(sb.ToString());
return ScopeEnter(sb.ToString());
}
return 0;
}
Expand Down
Loading