SqlDataAdapter.Fill(DataTable) does not detect re-raised error from SQL Server
- Dominant language
- C#
- Stars
- 989
- Forks
- 340
- Avg merge
- 4d 19h
- Merged PRs (30d)
- 72
Description
### Describe the bug
Imagine this: you are calling a stored procedure. This stored procedure has a shape like this:
```#sql
CREATE PROCEDURE some_as @par ... AS
BEGIN TRY
-- Lots of stuff here and then there is a final SELECT that returns data
SELECT ...
END TRY
BEGIN CATCH
IF @@trancount > 0 ROLLBACK TRANSACTION
; THROW
END CATCH
```
That is, it has standard exception handling according to best practice.
You want the result set in a DataTable, and you use SqlDataAdapter.Fill(DataTable).
Now, imagine that during that final SELECT, there is an error during execution. That could be a conversion error or an arithmetic error like overflow. It is quite natural to expect that this error propagates to the .NET code. However, that does not happen. Instead your code will jog along with the data that was returned before the error occurred in SQL. This can have grave consequences if the user who sees the data thinks it is correct and make business decisions accordingly.
The reason this happen is that under the covers, I assume, .Fill is calling ExecuteReader and consumes the first result set. However, because SQL Server reports the error in a separate statement, the client will not see the error until it moves on to the next result set. And apparently Fill(DataTable) never calles SqlDataReader.NextResult() which it must do.
See https://www.sommarskog.se/error_handling/Part3.html#AllResultSets for more details.
### To reproduce
Here is some repro code:
```c#
using Microsoft.Data.SqlClient;
using System.Data;
public static class SqlErrorRepro
{
private static string sqlBatchText = @"
BEGIN TRY
SELECT name, log(max_length) FROM sys.columns
END TRY
BEGIN CATCH
; THROW
END CATCH";
public static string connString =
@"Data Source=.;Integrated Security=SSPI;Database=tempdb;Encrypt=Optional";
public static void DtLoad()
{
using (SqlConnection cn = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand(sqlBatchText, cn))
{
try
{
cn.Open();
DataTable dt = new DataTable();
dt.Load(cmd.ExecuteReader());
System.Console.WriteLine("{0} rows read", dt.Rows.Count);
}
catch (System.Exception ex)
{
System.Console.WriteLine("ERROR: " + ex.Message);
}
}
}
public static void DaFillDt()
{
using (SqlConnection cn = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand(sqlBatchText, cn))
{
try
{
cn.Open();
SqlDataAdapter da = new SqlDataAdapter(cmd);
DataTable dt = new DataTable();
da.Fill(dt);
System.Console.WriteLine("{0} rows read", dt.Rows.Count);
}
catch (System.Exception ex)
{
System.Console.WriteLine("ERROR: " + ex.Message);
}
}
}
public static void DaFillDs()
{
using (SqlConnection cn = new SqlConnection(connString))
using (SqlCommand cmd = new SqlCommand(sqlBatchText, cn))
{
try
{
cn.Open();
SqlDataAdapter da = new SqlDataAdapter(cmd);
DataSet ds = new DataSet();
da.Fill(ds);
System.Console.WriteLine("{0} rows read", ds.Tables[0].Rows.Count);
}
catch (System.Exception ex)
{
System.Console.WriteLine("ERROR: " + ex.Message);
}
}
}
public static void Main()
{
System.Console.WriteLine(System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription);
System.Console.WriteLine("DataTable.load");
DtLoad();
System.Console.WriteLine("DataAdataper.Fill(DataTable)");
DaFillDt();
System.Console.WriteLine("DataAdataper.Fill(DataSet)");
DaFillDs();
}
}
```
### Expected behavior
This is the actual output from the program when I run it:
> .NET 6.0.12
> DataTable.load
> ERROR: An invalid floating point operation occurred.
> DataAdataper.Fill(DataTable)
> 60 rows read
> DataAdataper.Fill(DataSet)
> ERROR: An invalid floating point operation occurred.
The expected behaviour is that all three executions produce "ERROR: An invalid floating point operation occurred.". That is, "60 rows read" is incorrect.
Note that in the program I have also included Fill(DataSet) which finds the error - quite naturally, since it has to read all result sets. But also DataTable.Load(SqlDataReader) gives the correct result - and logically it performs the same operation as SqlDataAdapter.Fill(DataTable). Thus, this is a perfectly acceptable workaround. But the problem is that this method is not very well known, and the innocent and ignorant programmer may use Fill(DataTable) out of habut.
### Further technical details
This has been in about every version of SqlClient that I have tested this on. Including Microsoft.Data.SqlClient 5.01. I've also tested this with various .NET versions, including .NET 6. (but not .NET 7.) SQL Server version 2012 or later. I ran the test on Windows 11 21H2.
Contributor guide
Assessment
This issue has not been assessed yet.