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

SqlErrorCode Enumeration #518

Closed
rgarrison12345 opened this issue Apr 9, 2020 · 16 comments
Closed

SqlErrorCode Enumeration #518

rgarrison12345 opened this issue Apr 9, 2020 · 16 comments

Comments

@rgarrison12345
Copy link

Describe the solution you'd like

I use Polly framework to implement transient errors and fault tolerance when making sql queries/connections. I would like to see an enumeration of at least the most common error codes thrown by SQL Server. Instead of having to go to system.messages and inspect which may not be available if SQL connection is down.

I'd like to build my polly policies off of an enumerated list of error code values vs values that are hard coded in my client code. I think this will make the discovery and resolution of most common sql issues using the driver go much quicker.

MySqlConnector is already implementing this feature

https://github.com/mysql-net/MySqlConnector/blob/master/src/MySqlConnector/MySql.Data.MySqlClient/MySqlErrorCode.g.cs

@cheenamalhotra
Copy link
Member

Hi @rgarrison12345

The SqlClient driver supports Transient Fault Handling by default and s_transientErrors is the list of errors that we handle internally. Have you tried the same and are there any other error codes you think are "transient" and not handled by the driver?

@rgarrison12345
Copy link
Author

rgarrison12345 commented Apr 9, 2020

Hi @cheenamalhotra I am familiar with the Connect Retry Count and Connect Retry Interval properties in the connection string. I have used them successfully in projects in the past.

I'm looking for a way for my client code to have an understanding of what error was thrown from SQL Server from a new property on the SqlException object. I am aware there is a message property on the SqlException object.

A good example of this is a deadlock. I can inspect the message property and know it's a deadlock but I don't want to introduce any code that has any reliance on parsing a string property. I'd like an enumerated list of common error codes. Similar to what the MySqlConnector provides today.

@cheenamalhotra
Copy link
Member

I'd like an enumerated list of common error codes. Similar to what the MySqlConnector provides today.

The thing is the driver does not maintain error codes for exceptions SQL Server throws. The exceptions are directly thrown to end user. That's why there's no list on SqlClient side.

A good example of this is a deadlock. I can inspect the message property and know it's a deadlock but I don't want to introduce any code that has any reliance on parsing a string property.

I understand your point, generally drivers would provide "ErrorCode" with Exception that apps can use for filtering exceptions. And I do see there's an available "ErrorCode" property that the driver does not populate from internal exception. So if we populate ErrorCode in SqlException, would that solve your requirement?

@rgarrison12345
Copy link
Author

That would solve my requirement thank you.

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Apr 10, 2020

I looked more into it and tried to solve it but looks like System.Data.Common.DbException does not provide a way to construct an Exception that can contain both InnerException and ErrorCode. Also this InnerException is of System.Exception type, which has no placeholder for Number or ErrorCode.

And we do want to use constructor with InnerException to provide chain of inner exceptions for end users. So seems like we're stuck till we get some support from System.Data end.

I've also opened an issue dotnet/runtime#34798 for the same, where we can follow-up further.

/cc @saurabh500 @David-Engel

@saurabh500
Copy link
Contributor

SqlException has Errors property which is a collection System.Data.SqlClient.SqlErrorCollection

@rgarrison12345 is this not sufficient ?

@saurabh500
Copy link
Contributor

Say for the example of deadlocks, the server may send back a 1205 which will be reflected as one of the SqlException.Errors[0].Number and there will usually be a SqlException.Errors[0].State associated as well, which is sent back by SQL Server. I seems to me that this information should be sufficient to do what you are doing, without relying on string parsing.

About SqlClient providing an enumeration, I think that is a maintenance nightmare from the client side and the client will have be updated with new error code being introduced by the server side.
That is not desirable.

Every consumer of SqlClient would have some "interesting errors" that they can work with. I think that the consumer of SqlClient should maintain that enumeration and work off of that.

@rgarrison12345
Copy link
Author

SqlError works for me yes.

I am sorry, I did not fully explain my use case. I am working with I'm working with MariaDb and SqlServer and would like a database agnostic error object that is contained in the providers exception object. Be it SqlException or MySqlException or otherwise.

I was thinking SqlError could inherit from a new DbError which implements IDbError class in System.Data.Common, and that could be implemented for each provider. Each provider could have their own error collection class on the exception object.

@saurabh500
Copy link
Contributor

saurabh500 commented Apr 10, 2020

Ah ok. You are looking for a way to consume these errors based on the ADO.Net base class programming model and trying to avoid provide specific dependency.

This should definitely be followed up on dotnet/runtime repo where System.Data.Common resides.

cc @roji @ajcvickers thoughts?

@roji
Copy link
Member

roji commented Apr 10, 2020

I'm a bit skeptical of the idea of a database-agnostic error representation; ideally we'd have this, but databases (and drivers) unfortunately vary quite significantly in how they represent and map errors, and an abstraction over that isn't likely to work. Here are some points:

  • The SQL standard does contain the concept of SQLSTATE which standardizes database errors. However, SQL Server (or at least SqlClient) doesn't seem to support this: instead there's a SQLServer-specific error code scheme. There's also a State property, but that's a byte that doesn't seem to be related to SQLSTATE. Note also that SQLSTATE is a string (with the first two chars representing a category, and the latter three a sub-category), while the SQLServer code is a number (@saurabh500 @cheenamalhotra can you confirm that SQL Server doesn't support SQLSTATE?).
    • In the face of diverging error code schemes, a database-agnostic enum doesn't seem possible. Even if we were to surface the error in some database-agnostic way, if the actual values are database-specific, there's nothing really that an application could do with them.
  • An exception raised by SQL Server can contain multiple errors, while other database (e.g. PostgreSQL) can only return one.
  • We currently don't have a standard distinction between database-generated errors and driver-generated errors. For example, the SQL Server error list contains some errors which are about driver connectivity to the server (e.g. 53), and others which are clearly server-originated (e.g. 106). For comparison, in Npgsql there's an NpgsqlException as the general Npgsql exception, and a subclass of that called PostgresException for database-originated errors (the SQLSTATE is accessible on the latter only).

This is only a partial list of issues off the top of my head - I'm sure there are others.

However, if I interpret @rgarrison12345 correctly, what is really being asked for is a way to know whether an error is transient or not, for the purposes of implementing a database-agnostic retry policy. This has been a recurring theme, and I do think it makes sense to allow ADO.NET drivers to expose whether a given error is transient - opened dotnet/runtime#34817 with an API proposal.

@rgarrison12345
Copy link
Author

Thanks for working with me on this @roji. I reviewed the API proposal I think this would be a great help to developers to develop retry policies.

@cheenamalhotra
Copy link
Member

Hi @roji

Thanks for summary.

We currently don't have a standard distinction between database-generated errors and driver-generated errors. For example, the SQL Server error list contains some errors which are about driver connectivity to the server (e.g. 53), and others which are clearly server-originated (e.g. 106).

The problem is SqlClient behaves server way by attaching Client Exception as "Inner Exception" if you see here, this is done always and we never really let user know the "ErrorCode" in SqlException directly.

if (innerException == null && errorCollection[0].Win32ErrorCode != 0 && errorCollection[0].Win32ErrorCode != -1)
{
innerException = new Win32Exception(errorCollection[0].Win32ErrorCode);
}
SqlException exception = new SqlException(message.ToString(), errorCollection, innerException, conId);

Changing this is possible only for this scenario but not for other cases where InnerException is coming from server and we have to attach InnerException here.

SqlErrors is the only way someone capturing exception can get to know Error Number or by parsing Message, which seems weird to me. That's why I wanted to know if we can allow creating DbException object with ErrorCode and InnerException both, that would help us keep the existing behavior and also provide ErrorCode from DbException class.

For multiple error scenarios, yes we have SqlErrors object supported if someone is willing to drill down the stack. But that should NOT be the default way to find error number In my opinion.

@cheenamalhotra
Copy link
Member

cheenamalhotra commented Apr 13, 2020

can you confirm that SQL Server doesn't support SQLSTATE?

SQLSTATE is not directly supported by SQL Server, but is supported by ODBC and JDBC Drivers.

However in SqlClient, we don't provide the 5 character State Code, we return the data for 'State' as byte in SqlError as received from Server.

@cheenamalhotra cheenamalhotra added this to Needs triage in SqlClient Triage Board via automation Apr 13, 2020
@cheenamalhotra cheenamalhotra moved this from Needs triage to Under Investigation in SqlClient Triage Board Apr 13, 2020
@cheenamalhotra
Copy link
Member

Hi @rgarrison12345

We will close this issue as new proposed APIs with SqlException will be supported for .NET 5.0 in future.

SqlClient Triage Board automation moved this from Under Investigation to Closed Jul 23, 2020
@rgarrison12345
Copy link
Author

Hello @cheenamalhotra, is this featuring being implemented with the new "SqlState" property?

@cheenamalhotra
Copy link
Member

Yes, we have 2 new tracking issues: #648 and #649

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

4 participants