Merge | SqlTransaction

[benrr101]

Description: In the latest installment of "Ben Tries To Combine Two Projects Into One", we are merging the SqlTransaction class. This is actually a pretty easy merge - there are ~4 methods that need to be merged into a single file in the common project.

The big headache with these are that they use the constrained execution region pattern that only applies to netfx - this means each of these methods have an entirely different set of exception handling for special exceptions that can only happen in netfx constrained execution regions. However, the netcore implementation has a diagnostic listener that expects an exception to be set on it if one occurs. This presents a challenge of how to retain these differences while also making it readable. There are two patterns we could adopt:

1. All exception handles have a conditional for netcore to set the diagnostic listener eg:

//...
catch (OutOfMemoryException e) {
  #if NET
  diagnosticListener.SetException(e)
  #endif

  _connection.Abort();
 throw;
} catch (ThreadAbortException e ) {
  // .. repeat from above
} catch (Exception e) {
  #if NET
  diagnosticListener.SetException(e);
  #endif
  throw;
}

2. Conditional exception handles for netfx/netcore

#if NETFRAMEWORK
catch (OutOfMemoryException e) {
  _connection.Abort();
  throw;
} catch (ThreadAbortException e) {
  // .. repeat from above
}
#else
catch (Exception e) {
  diagnosticListener.SetException(e);
  throw;
}
#endif

In my humble opinion, option 2 is somewhat better, and it is the pattern I have adopted in this PR.

The other merge as part of this PR is moving SqlTransaction.Common back into the SqlTransaction file. Thus, no more partial for SqlTransaction. The Common file looks to be an early attempt to merge what could be merged, so since we have reconciled the differences, we can bring them all together again.

Testing: Once again, no functional differences, just code moving around.

@edwardneal Please don't be upset if I sniped your next merge target 😄

[paulmedynski]

Some comments/questions about preprocessor directives that are likely relevant to this PR. Other comments about existing code/patterns that we could maybe improve upon since we're here already.

[codecov]

Codecov Report

Attention: Patch coverage is 65.65657% with 68 lines in your changes missing coverage. Please review.

Project coverage is 72.78%. Comparing base (33083f3) to head (a693933).
Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
...ent/src/Microsoft/Data/SqlClient/SqlTransaction.cs	65.48%	68 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3231      +/-   ##
==========================================
+ Coverage   72.72%   72.78%   +0.06%     
==========================================
  Files         303      301       -2     
  Lines       59712    59614      -98     
==========================================
- Hits        43426    43393      -33     
+ Misses      16286    16221      -65     

Flag	Coverage Δ
addons	92.58% <ø> (ø)
netcore	75.18% <80.71%> (+0.09%)	⬆️
netfx	71.46% <61.27%> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.