diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs index e62541137c..93c7f4a7f4 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Connection/SqlConnectionInternal.cs @@ -424,19 +424,14 @@ internal SqlConnectionInternal( break; } catch (SqlException sqlex) + when ( + connectionEstablishCount != i + 1 + && applyTransientFaultHandling + && !_timeout.IsExpired + && _timeout.MillisecondsRemaining >= transientRetryIntervalInMilliSeconds + && IsTransientError(sqlex)) { - if (connectionEstablishCount == i + 1 - || !applyTransientFaultHandling - || _timeout.IsExpired - || _timeout.MillisecondsRemaining < transientRetryIntervalInMilliSeconds - || !IsTransientError(sqlex)) - { - throw; - } - else - { - Thread.Sleep(transientRetryIntervalInMilliSeconds); - } + Thread.Sleep(transientRetryIntervalInMilliSeconds); } } } @@ -2073,13 +2068,8 @@ protected override void Deactivate() } } // @TODO: CER Exception Handling was removed here (see GH#3581) - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // If an exception occurred, the inner connection will be marked as unusable and // destroyed upon returning to the pool DoomThisConnection(); @@ -3835,13 +3825,9 @@ private void OpenLoginEnlist( _timeoutErrorInternal.EndPhase(SqlConnectionTimeoutErrorPhase.PostLogin); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - LoginFailure(); - } - + LoginFailure(); throw; } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs index 4243e777ba..fa704ab438 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/WaitHandleDbConnectionPool.cs @@ -22,7 +22,7 @@ namespace Microsoft.Data.SqlClient.ConnectionPool /// /// A concrete implementation of used by Microsoft.Data.SqlClient /// to efficiently manage a pool of reusable objects backing ADO.NET SqlConnection instances. - /// + /// /// Primary Responsibilities: /// /// Connection Reuse and Pooling: Uses two stacks (_stackNew and _stackOld) to manage idle connections. Ensures efficient reuse and limits new connection creation. @@ -36,7 +36,7 @@ namespace Microsoft.Data.SqlClient.ConnectionPool /// Pending Request Queue: Queues unresolved connection requests in _pendingOpens and processes them using background threads. /// Identity and Authentication Context: Manages identity-based reuse via a dictionary of DbConnectionPoolAuthenticationContext keyed by user identity. /// - /// + /// /// Key Concepts in Design: /// /// Stacks and queues for free and pending connections @@ -379,7 +379,7 @@ private void CleanupCallback(object state) // ONLY touch obj after lock release if shouldDestroy is false!!! Otherwise, it may be destroyed // by transaction-end thread! - // Note that there is a minor race condition between this task and the transaction end event, if the latter runs + // Note that there is a minor race condition between this task and the transaction end event, if the latter runs // between the lock above and the SetInStasis call below. The result is that the stasis counter may be // incremented without a corresponding decrement (the transaction end task is normally expected // to decrement, but will only do so if the stasis flag is set when it runs). I've minimized the size @@ -547,14 +547,8 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio // Reset the error wait: _errorWait = ERROR_WAIT_DEFAULT; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - // UNDONE - should not be catching all exceptions!!! - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); if (!IsBlockingPeriodEnabled()) @@ -577,7 +571,7 @@ private DbConnectionInternal CreateObject(DbConnection owningObject, DbConnectio Timer t = new Timer(new TimerCallback(this.ErrorCallback), null, Timeout.Infinite, Timeout.Infinite); bool timerIsNotDisposed; - + _waitHandles.ErrorEvent.Set(); _errorOccurred = true; @@ -624,16 +618,16 @@ private void DeactivateObject(DbConnectionInternal obj) // be returned to a different customer until the transaction // actually completes, so we send it into Stasis -- the SysTx // transaction object will ensure that it is owned (not lost), - // and it will be certain to put it back into the pool. + // and it will be certain to put it back into the pool. if (State is ShuttingDown) { if (obj.IsTransactionRoot) { - // SQLHotfix# 50003503 - connections that are affiliated with a - // root transaction and that also happen to be in a connection - // pool that is being shutdown need to be put in stasis so that - // the root transaction isn't effectively orphaned with no + // SQLHotfix# 50003503 - connections that are affiliated with a + // root transaction and that also happen to be in a connection + // pool that is being shutdown need to be put in stasis so that + // the root transaction isn't effectively orphaned with no // means to promote itself to a full delegated transaction or // Commit or Rollback obj.SetInStasis(); @@ -666,7 +660,7 @@ private void DeactivateObject(DbConnectionInternal obj) { // NOTE: we're not locking on _state, so it's possible that its // value could change between the conditional check and here. - // Although perhaps not ideal, this is OK because the + // Although perhaps not ideal, this is OK because the // DelegatedTransactionEnded event will clean up the // connection appropriately regardless of the pool state. Debug.Assert(_transactedConnectionPool != null, "Transacted connection pool was not expected to be null."); @@ -685,13 +679,13 @@ private void DeactivateObject(DbConnectionInternal obj) { // SQLHotfix# 50003503 - if the object cannot be pooled but is a transaction // root, then we must have hit one of two race conditions: - // 1) PruneConnectionPoolGroups shutdown the pool and marked this connection + // 1) PruneConnectionPoolGroups shutdown the pool and marked this connection // as non-poolable while we were processing within this lock // 2) The LoadBalancingTimeout expired on this connection and marked this // connection as DoNotPool. // // This connection needs to be put in stasis so that the root transaction isn't - // effectively orphaned with no means to promote itself to a full delegated + // effectively orphaned with no means to promote itself to a full delegated // transaction or Commit or Rollback obj.SetInStasis(); rootTxn = true; @@ -716,7 +710,7 @@ private void DeactivateObject(DbConnectionInternal obj) } else if (destroyObject) { - // Connections that have been marked as no longer + // Connections that have been marked as no longer // poolable (e.g. exceeded their connection lifetime) are not, in fact, // returned to the general pool DestroyObject(obj); @@ -783,7 +777,7 @@ private void ErrorCallback(object state) private Exception TryCloneCachedException() // Cached exception can be of any type, so is not always cloneable. - // This functions clones SqlException + // This functions clones SqlException // OleDb and Odbc connections are not passing throw this code => _resError is SqlException sqlEx ? sqlEx.InternalClone() : _resError; @@ -796,7 +790,7 @@ private void WaitForPendingOpen() do { bool started = false; - + try { started = Interlocked.CompareExchange(ref _pendingOpensWaiting, 1, 0) == 0; @@ -825,7 +819,7 @@ private void WaitForPendingOpen() DbConnectionInternal connection = null; bool timeout = false; Exception caughtException = null; - + try { ADP.SetCurrentTransaction(next.Completion.Task.AsyncState as Transaction); @@ -1165,8 +1159,8 @@ private DbConnectionInternal GetFromGeneralPool() Debug.Assert(obj != null, "null connection is not expected"); } - // When another thread is clearing this pool, - // it will remove all connections in this pool which causes the + // When another thread is clearing this pool, + // it will remove all connections in this pool which causes the // following assert to fire, which really mucks up stress against // checked bits. @@ -1275,7 +1269,7 @@ private void PoolCreateRequest(object state) } catch { - // Catch all the exceptions occuring during CreateObject so that they + // Catch all the exceptions occuring during CreateObject so that they // don't emerge as unhandled on the thread pool and don't crash applications // The error is handled in CreateObject and surfaced to the caller of the Connection Pool // using the ErrorEvent. Hence it is OK to swallow all exceptions here. @@ -1301,15 +1295,10 @@ private void PoolCreateRequest(object state) QueuePoolCreateRequest(); } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // Now that CreateObject can throw, we need to catch the exception and discard it. - // There is no further action we can take beyond tracing. The error will be + // There is no further action we can take beyond tracing. The error will be // thrown to the user the next time they request a connection. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, PoolCreateRequest called CreateConnection which threw an exception: {1}", Id, e); } @@ -1494,14 +1483,14 @@ public void Shutdown() // TransactionEnded merely provides the plumbing for DbConnectionInternal to access the transacted pool // that is implemented inside DbConnectionPool. This method's counterpart (PutTransactedObject) should - // only be called from DbConnectionPool.DeactivateObject and thus the plumbing to provide access to + // only be called from DbConnectionPool.DeactivateObject and thus the plumbing to provide access to // other objects is unnecessary (hence the asymmetry of Ended but no Begin) public void TransactionEnded(Transaction transaction, DbConnectionInternal transactedObject) { Debug.Assert(transaction != null, "null transaction?"); Debug.Assert(transactedObject != null, "null transactedObject?"); - // Note: connection may still be associated with transaction due to Explicit Unbinding requirement. + // Note: connection may still be associated with transaction due to Explicit Unbinding requirement. SqlClientEventSource.Log.TryPoolerTraceEvent(" {0}, Transaction {1}, Connection {2}, Transaction Completed", Id, transaction.GetHashCode(), transactedObject.ObjectID); // called by the internal connection when it get's told that the diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs index c7d562750a..15a856dcec 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ManagedSni/SniTcpHandle.netcore.cs @@ -171,84 +171,79 @@ public SniTcpHandle( _socket = Connect(serverName, port, timeout, ipPreference, cachedFQDN, ref pendingDNSInfo); } } + // Connection failed with a retryable exception and we have cached DNS info: retry using cached IPs. catch (Exception ex) + when (!timeout.IsExpired && + hasCachedDNSInfo && + ex is (SocketException or ArgumentException or AggregateException)) { - if (timeout.IsExpired) + // Retry with cached IP address + int portRetry = string.IsNullOrEmpty(cachedDNSInfo.Port) ? port : int.Parse(cachedDNSInfo.Port); + + string firstCachedIP; + string secondCachedIP; + + if (SqlConnectionIPAddressPreference.IPv6First == ipPreference) { - throw; + firstCachedIP = cachedDNSInfo.AddrIPv6; + secondCachedIP = cachedDNSInfo.AddrIPv4; } - // Retry with cached IP address - if (ex is SocketException || ex is ArgumentException || ex is AggregateException) + else + { + firstCachedIP = cachedDNSInfo.AddrIPv4; + secondCachedIP = cachedDNSInfo.AddrIPv6; + } + + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO, "Connection Id {0}, Retrying with cached DNS IP Address {1} and port {2}", args0: _connectionId, args1: firstCachedIP, args2: portRetry); + + try { - if (hasCachedDNSInfo == false) + if (parallel) { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.ERR, "Connection Id {0}, Cached DNS Info not found, exception occurred thrown: {1}", args0: _connectionId, args1: ex?.Message); - throw; + _socket = TryConnectParallel(firstCachedIP, portRetry, timeout, ref reportError, cachedFQDN, ref pendingDNSInfo); } else { - int portRetry = string.IsNullOrEmpty(cachedDNSInfo.Port) ? port : int.Parse(cachedDNSInfo.Port); - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO, "Connection Id {0}, Retrying with cached DNS IP Address {1} and port {2}", args0: _connectionId, args1: cachedDNSInfo.AddrIPv4, args2: cachedDNSInfo.Port); - - string firstCachedIP; - string secondCachedIP; - - if (SqlConnectionIPAddressPreference.IPv6First == ipPreference) - { - firstCachedIP = cachedDNSInfo.AddrIPv6; - secondCachedIP = cachedDNSInfo.AddrIPv4; - } - else - { - firstCachedIP = cachedDNSInfo.AddrIPv4; - secondCachedIP = cachedDNSInfo.AddrIPv6; - } - - try - { - if (parallel) - { - _socket = TryConnectParallel(firstCachedIP, portRetry, timeout, ref reportError, cachedFQDN, ref pendingDNSInfo); - } - else - { - _socket = Connect(firstCachedIP, portRetry, timeout, ipPreference, cachedFQDN, ref pendingDNSInfo); - } - } - catch (Exception exRetry) - { - if (timeout.IsExpired) - { - throw; - } - if (exRetry is - SocketException or - ArgumentException or - AggregateException) - { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO, "Connection Id {0}, Retrying exception {1}", args0: _connectionId, args1: exRetry?.Message); - if (parallel) - { - _socket = TryConnectParallel(secondCachedIP, portRetry, timeout, ref reportError, cachedFQDN, ref pendingDNSInfo); - } - else - { - _socket = Connect(secondCachedIP, portRetry, timeout, ipPreference, cachedFQDN, ref pendingDNSInfo); - } - } - else - { - SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.ERR, "Connection Id {0}, Retry failed, exception occurred: {1}", args0: _connectionId, args1: exRetry?.Message); - throw; - } - } + _socket = Connect(firstCachedIP, portRetry, timeout, ipPreference, cachedFQDN, ref pendingDNSInfo); } } - else + catch (Exception exRetry) + when (!timeout.IsExpired && + exRetry is + SocketException or + ArgumentNullException or + ArgumentException or + ArgumentOutOfRangeException or + AggregateException) + { + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.INFO, "Connection Id {0}, Retrying exception {1}", args0: _connectionId, args1: exRetry?.Message); + if (parallel) + { + _socket = TryConnectParallel(secondCachedIP, portRetry, timeout, ref reportError, cachedFQDN, ref pendingDNSInfo); + } + else + { + _socket = Connect(secondCachedIP, portRetry, timeout, ipPreference, cachedFQDN, ref pendingDNSInfo); + } + } + catch (Exception exRetry) when (!timeout.IsExpired) { + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.ERR, "Connection Id {0}, Retry failed, exception occurred: {1}", args0: _connectionId, args1: exRetry?.Message); throw; } } + // Connection failed with a retryable exception but no cached DNS info available: log and rethrow. + catch (Exception ex) + when (!timeout.IsExpired && + !hasCachedDNSInfo && + ex is + SocketException or + ArgumentException or + AggregateException) + { + SqlClientEventSource.Log.TrySNITraceEvent(nameof(SniTcpHandle), EventType.ERR, "Connection Id {0}, Cached DNS Info not found, exception occurred thrown: {1}", args0: _connectionId, args1: ex?.Message); + throw; + } if (_socket == null || !_socket.Connected) { @@ -424,7 +419,7 @@ private static Socket Connect(string serverName, int port, TimeoutTimer timeout, List checkErrorLst; // Repeating Socket.Select several times if our timeout is greater - // than int.MaxValue microseconds because of + // than int.MaxValue microseconds because of // https://github.com/dotnet/SqlClient/pull/1029#issuecomment-875364044 // which states that Socket.Select can't handle timeouts greater than int.MaxValue microseconds do @@ -583,7 +578,7 @@ private static Socket ParallelConnect(IPAddress[] serverAddresses, int port, Tim try { // Repeating Socket.Select several times if our timeout is greater - // than int.MaxValue microseconds because of + // than int.MaxValue microseconds because of // https://github.com/dotnet/SqlClient/pull/1029#issuecomment-875364044 // which states that Socket.Select can't handle timeouts greater than int.MaxValue microseconds do diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs index 0b75a6178d..94338c8dfb 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/Server/ValueUtilsSmi.cs @@ -2384,12 +2384,8 @@ private static void SetCharsOrString_FromReader(ITypedSettersV3 setters, int ord SetChars_FromReader(setters, ordinal, metaData, reader, offset); success = true; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } } if (!success) diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs index 414be57eb0..af2091d7f6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs @@ -1092,12 +1092,8 @@ private void Dispose(bool disposing) _internalTransaction = null; } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); } } @@ -1861,12 +1857,8 @@ private object ConvertValue(object value, _SqlMetaData metadata, bool isNull, re return value; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } throw SQL.BulkLoadCannotConvertValue(value.GetType(), type, metadata.ordinal, RowNumber, metadata.isEncrypted, metadata.column, value.ToString(), e); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs index 36d727575c..141416537f 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.NonQuery.cs @@ -236,21 +236,14 @@ private IAsyncResult BeginExecuteNonQueryInternal( BeginExecuteNonQueryInternalReadStage(localCompletion); } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableOrSecurityExceptionType(e)) { - // @TODO: Invert. - if (!ADP.IsCatchableOrSecurityExceptionType(e)) - { - // Exception is not catchable, the connection has already been caught and - // doomed in a lower level. - throw; - } - // For async, RunExecuteReader will never put the stateObj back into the pool, // so, do so now. ReliablePutStateObject(); throw; } + // Allow other exceptions to bubble up as-is. // When we use query caching for parameter encryption we need to retry on specific errors. // In these cases finalize the call internally and trigger a retry when needed. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs index d9f6c33233..a98d82118b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Reader.cs @@ -290,15 +290,8 @@ private IAsyncResult BeginExecuteReaderInternal( isRetry, nameof(BeginExecuteReader)); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableOrSecurityExceptionType(e)) { - if (!ADP.IsCatchableOrSecurityExceptionType(e)) - { - // If not catchable - the connection has already been caught and doomed in - // RunExecuteReader. - throw; - } - // For async, RunExecuteReader will never put the stateObj back into the pool, // so, do so now. ReliablePutStateObject(); @@ -307,6 +300,7 @@ private IAsyncResult BeginExecuteReaderInternal( throw; } } + // Allow other exceptions to bubble up as-is. if (writeTask is not null) { @@ -848,28 +842,25 @@ private void FinishExecuteReader( // @TODO: Why does the command set whether the reader is initialized?? ds.IsInitialized = true; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) + if (_inPrepare) { - if (_inPrepare) - { - // The flag is expected to be reset by OnReturnValue. We should receive - // the handle unless command execution failed. If it fails, move back - // to pending state. - _inPrepare = false; // reset the flag - IsDirty = true; // mark command as dirty so it will be prepared next time we're coming through - _execType = EXECTYPE.PREPAREPENDING; // reset execution type to pending - } + // The flag is expected to be reset by OnReturnValue. We should receive + // the handle unless command execution failed. If it fails, move back + // to pending state. + _inPrepare = false; // reset the flag + IsDirty = true; // mark command as dirty so it will be prepared next time we're coming through + _execType = EXECTYPE.PREPAREPENDING; // reset execution type to pending + } - try - { - ds.Close(); - } - catch (Exception eClose) - { - Debug.WriteLine($"Received this exception from SqlDataReader.Close() while in another catch block: {eClose}"); - } + try + { + ds.Close(); + } + catch (Exception eClose) + { + Debug.WriteLine($"Received this exception from SqlDataReader.Close() while in another catch block: {eClose}"); } throw; diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs index c025e1d3b7..2be6166e8b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.Xml.cs @@ -165,13 +165,9 @@ private static XmlReader CompleteXmlReader(SqlDataReader dataReader, bool isAsyn processAllRows: metaData[0].SqlDbType is not SqlDbType.Xml); xmlReader = sqlStream.ToXmlReader(isAsync); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - dataReader.Close(); - } - + dataReader.Close(); throw; } } @@ -253,21 +249,14 @@ private IAsyncResult BeginExecuteXmlReaderInternal( // @TODO: NonQuery pathway has the continueTaskWithState block inside this try. One or the other seems wrong } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableOrSecurityExceptionType(e)) { - // @TODO: Invert - if (!ADP.IsCatchableOrSecurityExceptionType(e)) - { - // If not catchable - the connection has already been caught and doomed in - // RunExecuteReader. - throw; - } - // For async, RunExecuteReader will never put the stateObj back into the pool, // so, do so now. ReliablePutStateObject(); throw; } + // Allow other exceptions to bubble up as-is. if (writeTask is not null) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs index 1073bb137c..ec26b955e1 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCommand.cs @@ -1829,12 +1829,8 @@ internal void OnStatementCompleted(int recordCount) handler(this, new StatementCompletedEventArgs(recordCount)); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableOrSecurityExceptionType(e)) { - if (!ADP.IsCatchableOrSecurityExceptionType(e)) - { - throw; - } } } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs index 9021d3e259..47a1be163a 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnection.cs @@ -2610,13 +2610,8 @@ internal void OnInfoMessage(SqlInfoMessageEventArgs imevent, out bool notified) { handler(this, imevent); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableOrSecurityExceptionType(e)) { - if (!ADP.IsCatchableOrSecurityExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs index e316bb3c08..ca44267eb6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDataReader.cs @@ -1875,12 +1875,8 @@ private TdsOperationStatus TryGetBytesInternal(int i, long dataIndex, byte[] buf Buffer.BlockCopy(data, ndataIndex, buffer, bufferIndex, cbytes); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } cbytes = data.Length; if (length < 0) @@ -2152,17 +2148,10 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn { CheckDataIsReady(columnIndex: i, allowPartiallyReadColumn: true); } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { // We need to wrap all exceptions inside a TargetInvocationException to simulate calling CreateSqlReader via MethodInfo.Invoke - if (ADP.IsCatchableExceptionType(ex)) - { - throw new TargetInvocationException(ex); - } - else - { - throw; - } + throw new TargetInvocationException(ex); } charsRead = GetStreamingXmlChars(i, dataIndex, buffer, bufferIndex, length); } @@ -2232,12 +2221,8 @@ override public long GetChars(int i, long dataIndex, char[] buffer, int bufferIn Array.Copy(_columnDataChars, ndataIndex, buffer, bufferIndex, cchars); _columnDataCharsRead += cchars; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } cchars = _columnDataChars.Length; if (length < 0) @@ -5044,12 +5029,8 @@ public override Task ReadAsync(CancellationToken cancellationToken) #endif } } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { - if (!ADP.IsCatchableExceptionType(ex)) - { - throw; - } return Task.FromException(ex); } @@ -5146,12 +5127,8 @@ override public Task IsDBNullAsync(int i, CancellationToken cancellationTo { CheckHeaderIsReady(columnIndex: i); } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { - if (!ADP.IsCatchableExceptionType(ex)) - { - throw; - } return Task.FromException(ex); } @@ -5204,12 +5181,8 @@ override public Task IsDBNullAsync(int i, CancellationToken cancellationTo #endif } } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { - if (!ADP.IsCatchableExceptionType(ex)) - { - throw; - } return Task.FromException(ex); } @@ -5305,12 +5278,8 @@ override public Task GetFieldValueAsync(int i, CancellationToken cancellat } } } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { - if (!ADP.IsCatchableExceptionType(ex)) - { - throw; - } return Task.FromException(ex); } @@ -5346,12 +5315,8 @@ override public Task GetFieldValueAsync(int i, CancellationToken cancellat #endif } } - catch (Exception ex) + catch (Exception ex) when (ADP.IsCatchableExceptionType(ex)) { - if (!ADP.IsCatchableExceptionType(ex)) - { - throw; - } return Task.FromException(ex); } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs index 836fbb7dd4..a101099dd8 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependency.cs @@ -638,13 +638,8 @@ internal static bool Start(string connectionString, string queue, bool useDefaul { Stop(connectionString, queue, useDefaults, true); } - catch (Exception e) - { // Discard stop failure! - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) + { ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. SqlClientEventSource.Log.TryNotificationTraceEvent(" Exception occurred from Stop() after duplicate was found on Start()."); } @@ -666,13 +661,8 @@ internal static bool Start(string connectionString, string queue, bool useDefaul // to provide options themselves. } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. SqlClientEventSource.Log.TryNotificationTraceEvent(" Exception occurred from _processDispatcher.Start(...), calling Invalidate(...)."); throw; @@ -785,13 +775,8 @@ internal static bool Stop(string connectionString, string queue, bool useDefault // to provide options themselves. } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs index 004ae0ed08..faf088b9b5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyListener.cs @@ -177,13 +177,8 @@ internal SqlConnectionContainer(SqlConnectionContainerHashHelper hashHelper, str _timeoutParam.Value = _defaultWaitforTimeout; // Sync successful, extend timeout to 60 seconds. AsynchronouslyQueryServiceBrokerQueue(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. if (setupCompleted) { @@ -400,12 +395,8 @@ private void CreateQueueAndService(bool restart) { com.ExecuteNonQuery(); // Cannot add 'IF OBJECT_ID' to create procedure query - wrap and discard failure. } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); try @@ -416,12 +407,8 @@ private void CreateQueueAndService(bool restart) trans = null; } } - catch (Exception f) + catch (Exception f) when (ADP.IsCatchableExceptionType(f)) { - if (!ADP.IsCatchableExceptionType(f)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(f); // Discard failure, but trace for now. } } @@ -479,12 +466,8 @@ private void CreateQueueAndService(bool restart) trans.Rollback(); trans = null; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. } } @@ -580,12 +563,8 @@ private void ProcessNotificationResults(SqlDataReader reader) { dispatcher.InvalidateCommandID(notification); // CROSS APP-DOMAIN CALL! } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure. User event could throw exception. } } @@ -664,12 +643,8 @@ private void Restart(object unused) { _con.Close(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard close failure, if it occurs. Only trace it. } } @@ -717,12 +692,8 @@ private void Restart(object unused) { CreateQueueAndService(true); // Ensure service, queue, etc is present, if we created it. } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure, but trace for now. failure = true; } @@ -765,12 +736,8 @@ private void Restart(object unused) TearDownAndDispose(); // Function will lock(this). } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); try @@ -786,12 +753,8 @@ private void Restart(object unused) SqlNotificationType.Change, null)); } - catch (Exception f) + catch (Exception f) when (ADP.IsCatchableExceptionType(f)) { - if (!ADP.IsCatchableExceptionType(f)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(f); // Discard exception from Invalidate. User events can throw. } @@ -799,12 +762,8 @@ private void Restart(object unused) { _con.Close(); } - catch (Exception f) + catch (Exception f) when (ADP.IsCatchableExceptionType(f)) { - if (!ADP.IsCatchableExceptionType(f)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(f); // Discard close failure, if it occurs. Only trace it. } @@ -884,12 +843,8 @@ internal bool Stop(string appDomainKey, out bool appDomainStop) // Rather than fighting the race condition, just call it and discard any potential failure. _com.Cancel(); // Cancel the pending command. No-op if connection closed. } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure, if it should occur. } _stop = true; @@ -992,12 +947,8 @@ private void TearDownAndDispose() _com.Parameters.Remove(_timeoutParam); _com.ExecuteNonQuery(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure. } } @@ -1016,12 +967,8 @@ private void TearDownAndDispose() { _com.ExecuteNonQuery(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure. } } @@ -1104,12 +1051,8 @@ internal static SqlNotification ProcessMessage(SqlXml xmlMessage) type = temp; } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure, if it should occur. } messageAttributes |= MessageAttributes.Type; @@ -1123,12 +1066,8 @@ internal static SqlNotification ProcessMessage(SqlXml xmlMessage) source = temp; } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); // Discard failure, if it should occur. } messageAttributes |= MessageAttributes.Source; @@ -1468,17 +1407,13 @@ private void Invalidate(string server, SqlNotification sqlNotification) { perAppDomainDispatcher.InvalidateServer(server, sqlNotification); } - catch (Exception f) + catch (Exception f) when (ADP.IsCatchableExceptionType(f)) { // Since we are looping over dependency dispatchers, do not allow one Invalidate // that results in a throw prevent us from invalidating all dependencies // related to this server. // NOTE - SqlDependencyPerAppDomainDispatcher already wraps individual dependency invalidates // with try/catch, but we should be careful and do the same here. - if (!ADP.IsCatchableExceptionType(f)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(f); // Discard failure, but trace. } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs index b990270924..b37e915464 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlDependencyUtils.cs @@ -272,15 +272,11 @@ internal void InvalidateCommandID(SqlNotification sqlNotification) { dependency.Invalidate(sqlNotification.Type, sqlNotification.Info, sqlNotification.Source); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { // Since we are looping over dependencies, do not allow one Invalidate // that results in a throw prevent us from invalidating all dependencies // related to this server. - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); } } @@ -327,15 +323,11 @@ internal void InvalidateServer(string server, SqlNotification sqlNotification) { dependency.Invalidate(sqlNotification.Type, sqlNotification.Info, sqlNotification.Source); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { // Since we are looping over dependencies, do not allow one Invalidate // that results in a throw prevent us from invalidating all dependencies // related to this server. - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } ADP.TraceExceptionWithoutRethrow(e); } } @@ -581,13 +573,8 @@ private static void TimeoutTimerCallback(object state) // to invoke user-code while holding an internal lock. dependencies[i].Invalidate(SqlNotificationType.Change, SqlNotificationInfo.Error, SqlNotificationSource.Timeout); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // This is an exception in user code, and we're in a thread-pool thread // without user's code up in the stack, no much we can do other than // eating the exception. diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs index bc5f5cbccb..ee6557e55b 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlInternalTransaction.cs @@ -181,12 +181,8 @@ private void CheckTransactionLevelAndZombie() Zombie(); } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } #if NETFRAMEWORK ADP.TraceExceptionWithoutRethrow(e); #endif @@ -243,13 +239,9 @@ internal void Commit() _innerConnection.ExecuteTransaction(TransactionRequest.Commit, null, IsolationLevel.Unspecified, null, false); ZombieParent(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - CheckTransactionLevelAndZombie(); - } - + CheckTransactionLevelAndZombie(); throw; } } @@ -350,18 +342,11 @@ internal void Rollback() // server transaction level. This transaction has been completed. Zombie(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - CheckTransactionLevelAndZombie(); + CheckTransactionLevelAndZombie(); - if (!_disposing) - { - throw; - } - } - else + if (!_disposing) { throw; } @@ -394,12 +379,9 @@ internal void Rollback(string transactionName) { _innerConnection.ExecuteTransaction(TransactionRequest.Rollback, transactionName, IsolationLevel.Unspecified, null, false); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - CheckTransactionLevelAndZombie(); - } + CheckTransactionLevelAndZombie(); throw; } } @@ -426,13 +408,9 @@ internal void Save(string savePointName) { _innerConnection.ExecuteTransaction(TransactionRequest.Save, savePointName, IsolationLevel.Unspecified, null, false); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (ADP.IsCatchableExceptionType(e)) - { - CheckTransactionLevelAndZombie(); - } - + CheckTransactionLevelAndZombie(); throw; } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlMetaDataFactory.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlMetaDataFactory.cs index 1d9ceebafd..841c71d410 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlMetaDataFactory.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlMetaDataFactory.cs @@ -422,12 +422,8 @@ private async ValueTask ExecuteCommandAsync(DataRow requestedCollecti { reader = isAsync ? await command.ExecuteReaderAsync(cancellationToken) : command.ExecuteReader(); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } throw ADP.QueryFailed(collectionName, e); } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs index 621413f9c1..e030fb11f5 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlParameter.cs @@ -2419,13 +2419,8 @@ value is IEnumerable value = Convert.ChangeType(value, destinationType.ClassType, null); } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - throw ADP.ParameterConversionFailed(value, destinationType.ClassType, e); } } diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs index 51f0abc7cf..3fb1ec1db6 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlQueryMetadataCache.cs @@ -123,29 +123,29 @@ internal bool GetQueryMetadataIfExists(SqlCommand sqlCommand) { SqlSecurityUtility.DecryptSymmetricKey(cipherMdCopy, sqlCommand.Connection, sqlCommand); } - catch (Exception ex) + catch (Exception ex) when (ex is SqlException or ArgumentException) { // Invalidate the cache entry. InvalidateCacheEntry(sqlCommand); - // If we get one of the expected exceptions, just fail the cache lookup, otherwise throw. - if (ex is SqlException or ArgumentException) + foreach (SqlParameter paramToCleanup in sqlCommand.Parameters) { - foreach (SqlParameter paramToCleanup in sqlCommand.Parameters) - { - paramToCleanup.CipherMetadata = null; - } - - IncrementCacheMisses(); - return false; + paramToCleanup.CipherMetadata = null; } + IncrementCacheMisses(); + return false; + } + catch (Exception) + { + // Invalidate the cache entry. + InvalidateCacheEntry(sqlCommand); throw; } } } - ConcurrentDictionary enclaveKeys = + ConcurrentDictionary enclaveKeys = _cache.Get>(enclaveLookupKey); if (enclaveKeys is not null) { diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs index 8bfedc3539..8a7304a611 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParser.cs @@ -208,7 +208,7 @@ static TdsParser() { // For CoreCLR, we need to register the ANSI Code Page encoding provider before attempting to get an Encoding from a CodePage // For a default installation of SqlServer the encoding exchanged during Login is 1252. This encoding is not loaded by default - // See Remarks at https://msdn.microsoft.com/en-us/library/system.text.encodingprovider(v=vs.110).aspx + // See Remarks at https://msdn.microsoft.com/en-us/library/system.text.encodingprovider(v=vs.110).aspx // SqlClient needs to register the encoding providers to make sure that even basic scenarios work with Sql Server. Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); } @@ -683,7 +683,7 @@ internal void RemoveEncryption() // create a new packet encryption changes the internal packet size Bug# 228403 _physicalStateObj.ClearAllWritePackets(); - } + } internal void EnableMars() { @@ -1376,11 +1376,11 @@ internal void TdsLogin( int feOffset = length; // calculate and reserve the required bytes for the featureEx length = ApplyFeatureExData( - requestedFeatures, - recoverySessionData, + requestedFeatures, + recoverySessionData, fedAuthFeatureExtensionData, UserAgent.Ucs2Bytes, - useFeatureExt, + useFeatureExt, length ); @@ -2792,7 +2792,7 @@ internal TdsOperationStatus TryRun(RunBehavior runBehavior, SqlCommand cmdHandle { _connHandler._federatedAuthenticationInfoReceived = true; SqlFedAuthInfo info; - + result = TryProcessFedAuthInfo(stateObj, tokenLength, out info); if (result != TdsOperationStatus.Done) { @@ -5711,7 +5711,7 @@ private TdsOperationStatus TryCommonProcessMetaData(TdsParserStateObject stateOb { return result; } - + // read flags and set appropriate flags in structure byte flags; result = stateObj.TryReadByte(out flags); @@ -7040,7 +7040,7 @@ internal TdsOperationStatus TryReadSqlValue(SqlBuffer value, return result; } - // Internally, we use Sqlbinary to deal with varbinary data and store it in + // Internally, we use Sqlbinary to deal with varbinary data and store it in // SqlBuffer as SqlBinary value. #if NET value.SqlBinary = SqlBinary.WrapBytes(b); @@ -8794,14 +8794,8 @@ private void ProcessAttention(TdsParserStateObject stateObj) // Call run loop to process looking for attention ack. Run(RunBehavior.Attention, null, null, null, stateObj); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - // UNDONE - should not be catching all exceptions!!! - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - // If an exception occurs - break the connection. // Attention error will not be thrown in this case by Run(), but other failures may. ADP.TraceExceptionWithoutRethrow(e); @@ -9169,7 +9163,7 @@ internal int WriteFedAuthFeatureRequest(FederatedAuthenticationFeatureExtensionD /// internal int WriteVectorSupportFeatureRequest(bool write) { - const int len = 6; + const int len = 6; if (write) { @@ -9527,15 +9521,11 @@ private void WriteLoginData(SqlLogin rec, true ); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - // UNDONE - should not be catching all exceptions!!! - if (ADP.IsCatchableExceptionType(e)) - { - // be sure to wipe out our buffer if we started sending stuff - _physicalStateObj.ResetPacketCounters(); - _physicalStateObj.ResetBuffer(); - } + // be sure to wipe out our buffer if we started sending stuff + _physicalStateObj.ResetPacketCounters(); + _physicalStateObj.ResetBuffer(); throw; } @@ -9890,13 +9880,8 @@ internal SqlDataReader TdsExecuteTransactionManagerRequest( return dtcReader; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - FailureCleanup(stateObj, e); throw; @@ -10067,14 +10052,8 @@ internal Task TdsExecuteSQLBatch(string text, int timeout, SqlNotificationReques // Finished sync return null; } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - // UNDONE - should not be catching all exceptions!!! - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - FailureCleanup(stateObj, e); throw; @@ -10367,7 +10346,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet { isSqlVal = param.ParameterIsSqlType; // We have to forward the TYPE info, we need to know what type we are returning. Once we null the parameter we will no longer be able to distinguish what type were seeing. - // Output parameter of SqlDbType vector are defined through SqlParameter.Value. + // Output parameter of SqlDbType vector are defined through SqlParameter.Value. // This check ensures that we do not discard the parameter value when SqlDbType is vector. if (mt.SqlDbType != SqlDbTypeExtensions.Vector) { @@ -10652,7 +10631,7 @@ private Task TDSExecuteRPCAddParameter(TdsParserStateObject stateObj, SqlParamet Debug.Assert(udtVal != null, "GetBytes returned null instance. Make sure that it always returns non-null value"); size = udtVal.Length; - + if (size >= maxSupportedSize && maxsize != -1) { throw SQL.UDTInvalidSize(maxsize, maxSupportedSize); @@ -13154,7 +13133,7 @@ private Task WriteUnterminatedValue(object value, MetaType type, byte scale, int { if (type.NullableType == TdsEnums.SQLJSON) { - // TODO : Performance and BOM check. Saurabh + // TODO : Performance and BOM check. Saurabh byte[] jsonAsBytes = Encoding.UTF8.GetBytes((string)value); WriteInt(jsonAsBytes.Length, stateObj); return stateObj.WriteByteArray(jsonAsBytes, jsonAsBytes.Length, 0, canAccumulate: false); @@ -13812,13 +13791,13 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj } TdsOperationStatus result = TryReadPlpUnicodeChars( - ref temp, - 0, - length >> 1, - stateObj, - out length, + ref temp, + 0, + length >> 1, + stateObj, + out length, supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot - rentedBuff: ref buffIsRented, + rentedBuff: ref buffIsRented, startOffset, canContinue ); @@ -14028,7 +14007,7 @@ bool writeDataSizeToSnapshot stateObj._longlenleft--; if (writeDataSizeToSnapshot) { - // we need to write the single b1 byte to the array because we may run out of data + // we need to write the single b1 byte to the array because we may run out of data // and need to wait for another packet buff[offst] = (char)((b1 & 0xff)); currentPacketId = IncrementSnapshotDataSize(stateObj, restartingDataSizeCount, currentPacketId, 1); diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs index ad8651cebd..45ed29c2ce 100644 --- a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs +++ b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs @@ -1083,13 +1083,8 @@ internal bool Deactivate() goodForReuse = true; } } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } - ADP.TraceExceptionWithoutRethrow(e); } return goodForReuse; @@ -3835,12 +3830,8 @@ private bool OnTimeoutCore(int expectedState, int targetState, bool asyncClose = { SendAttention(mustTakeWriteLock: true, asyncClose); } - catch (Exception e) + catch (Exception e) when (ADP.IsCatchableExceptionType(e)) { - if (!ADP.IsCatchableExceptionType(e)) - { - throw; - } // if unable to send attention, cancel the _networkPacketTaskSource to // request the parser be broken. SNIWritePacket errors will already // be in the _errors collection.