Skip to content

Fixed TDS message severity handling - #2741

Merged
Divang Sharma (divang) merged 16 commits into
mainfrom
dev/divang/handle_fatal_severity
Oct 17, 2025
Merged

Fixed TDS message severity handling#2741
Divang Sharma (divang) merged 16 commits into
mainfrom
dev/divang/handle_fatal_severity

Conversation

@divang

@divang Divang Sharma (divang) commented Aug 19, 2025

Copy link
Copy Markdown
Contributor

Problem:
TDS token handler was not properly detecting and handling fatal severity errors (severity 25+) in DONE tokens. When SQL Server encounters fatal errors, it sets error status flags in DONE tokens but the driver wasn't capturing these. This could lead to silent failures or missed error conditions where applications wouldn't be notified of critical database errors. TDS stream corruption could occur in scenarios involving severity 25 rollback situations where environment changes weren't properly handled.

Root Cause:
The base TDSTokenHandler.onDone() method only processed DONE tokens for completion tracking but ignored error status flags. Error status flags TDS.DONE_ERROR and TDS.DONE_SRVERROR in DONE tokens weren't being checked to generate appropriate SQLServerError. Fatal errors (severity 25) were not being propagated as database errors, causing them to be silently ignored. Environment change handling onEnvChange() was processing changes even during fatal error scenarios.

Solution:
Enhanced TDSTokenHandler.onDone() method to check for error status flags in DONE tokens.
Added check for [TDS.DONE_ERROR] and [TDS.DONE_SRVERROR] status flags using tdsReader.peekStatusFlag()
When error status is detected, create a synthetic SQLServerError with the standard "R_serverError" message
Add the synthetic error to the token handler's database error collection for proper propagation. Ensured proper TDS stream position management by peeking status flags before consuming DONE token data. Maintained backward compatibility with existing error handling mechanisms.

Testing:
Added comprehensive test case to simulate severity 25 fatal errors.
Test validates that is properly generated when DONE tokens contain error status flags. Verifies correct error message assignment using "R_severeyError" template for fatal error conditions. Ensures the error detection mechanism works correctly with mocked TDS reader scenarios. Covers the specific fatal error handling scenario to prevent regression.

@codecov

codecov Bot commented Aug 19, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 52.28%. Comparing base (86cae8c) to head (beccf1c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2741      +/-   ##
============================================
+ Coverage     52.20%   52.28%   +0.07%     
- Complexity     4151     4163      +12     
============================================
  Files           149      149              
  Lines         34358    34366       +8     
  Branches       5732     5733       +1     
============================================
+ Hits          17937    17967      +30     
+ Misses        13928    13912      -16     
+ Partials       2493     2487       -6     

☔ 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.

@machavan Mahendra Chavan (machavan) added this to the 13.2.1 milestone Aug 19, 2025
@Ananya2 Ananya Garg (Ananya2) changed the title Fixed TDS meesage severity handling Fixed TDS message severity handling Aug 20, 2025
tdsReader.getConnection().getSessionRecovery().decrementUnprocessedResponseCount();
}

if ((status & TDS.DONE_ERROR) != 0 || (status & TDS.DONE_SRVERROR) != 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

SqlClient treats DONE_SRVERROR as a fatal error:
https://github.com/dotnet/SqlClient/blob/v6.1.1/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs#L3167

Fatal errors ultimately close the connection and signal up to connection pools about the error. We need to plumb through something similar. See makeFromDatabaseError in SQLServerException.java. We don't have the database error, so we will need to figure something out there.

@divang Divang Sharma (divang) Aug 29, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think, adding Error Severity 20 will make sure the notifyPooledConnection() call.
SQLServerError syntheticError = new SQLServerError();
syntheticError.setErrorMessage(SQLServerException.getErrString("R_severeError"));
syntheticError.setErrorSeverity((byte) 20); // A severity of 20 indicates a fatal error in the current
// process.

Ref: makeFromDatabaseError in SQLServerException.java:
// Close the connection if we get a severity 20 or higher error class (nClass is severity of error).
if ((sqlServerError.getErrorSeverity() >= 20) && (null != con)) {
con.notifyPooledConnection(theException);
con.close();
}

@divang
Divang Sharma (divang) merged commit 3c17a1a into main Oct 17, 2025
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants