Skip to content
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.

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

SqlConnection.State value is not updated when connection is broken #1874

Closed
Charles-Gagnon opened this issue Dec 21, 2022 · 7 comments
Closed
Assignees

Comments

@Charles-Gagnon
Copy link

Describe the bug

Note, this may be happening in other scenarios as well. But this is a clear example of behavior I wasn't expected so figured we'd start from there.

The root issue is that the State property doesn't match what I'd expect when the connection to the server is broken.

To reproduce

  1. Create sample app that continuously executes a query against a server, e.g.
using SqlConnection conn = new SqlConnection("SERVER INFO HERE");
conn.Open();
while(true)
{
    SqlCommand cmd = conn.CreateCommand();
    cmd.CommandText = "SELECT 1";
    try
    {
        var result = cmd.ExecuteNonQuery();
        Console.WriteLine("Executed successfully");
    } catch (Exception e)
    {
        Console.WriteLine($"State={conn.State} {e.Message}");
    }
    Thread.Sleep(1000);
}
  1. Start the program, it should start printing out "Executed successfully"
  2. Shut off the server

Expected behavior

I expect an exception to be thrown (this happens) and the connection state to be Closed/Broken (this does not happen).

Instead I get the failures but the connection State stays as open

image

Further technical details

Microsoft.Data.SqlClient version: 5.1.0-preview2.22314.2
.NET target: .NET 6.0
SQL Server version: SQL 2022
Operating system: Docker container

@lcheunglci
Copy link
Contributor

Hi @Charles-Gagnon , thanks for bringing this to our attention and thanks for providing the repro sample. We'll take a look after the release.

@lcheunglci lcheunglci added By Design By design and removed untriaged labels Jan 10, 2023
@lcheunglci
Copy link
Contributor

This behavior is by design. Connection State does not change outside of client server communication. The only things that change State are the application closing the connection or some kind of failure of an active call between the client and the server. There are no performant opportunities for the driver to change the connection state outside of those events.

SqlClient Triage Board automation moved this from Needs triage to Closed Jan 10, 2023
@Charles-Gagnon
Copy link
Author

Why is the above example not considered a "failure of an active call"? I try to execute a query and it fails with a connection error.

@Charles-Gagnon
Copy link
Author

The documentation for State specifically says :

Indicates the state of the SqlConnection during the most recent network operation performed on the connection.

which in the above example is false - I tried to execute a network operation but it was broken and so failed.

@lcheunglci lcheunglci reopened this Jan 11, 2023
SqlClient Triage Board automation moved this from Closed to Needs triage Jan 11, 2023
@lcheunglci lcheunglci removed the By Design By design label Jan 11, 2023
@lcheunglci lcheunglci moved this from Needs triage to Needs Investigation in SqlClient Triage Board Jan 11, 2023
@lcheunglci
Copy link
Contributor

Thanks for your response. I'll do some investigation after our release.

@lcheunglci lcheunglci self-assigned this Jan 11, 2023
@cheenamalhotra
Copy link
Member

cheenamalhotra commented Jan 20, 2023

I've talked to @Charles-Gagnon offline on this, and I had few points to add:

Error with Class 20 is not retriable, as in this case, so application must not retry (as per guidelines), for now I would recommend that to be handled.

For SqlClient, since the error class is 20 and this is infact a login failure, the driver should be able to update the connection state to 'Closed' and make it ready to be re-opened. The parser is also disconnected here so it doesn't change the fact that this connection will never be reusable in this state. It has to be re-opened by applications if this error occurs.

Doing so is presumably safe for driver since closing an already closed connection doesn't lead to exception (for applications capturing this error with class 20 and closing connection).

@JRahnama
Copy link
Member

JRahnama commented Apr 3, 2023

Closing the issue. PR #1953 is merged with the fix for this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

5 participants