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

How to include "ErrorCode" as well as "Inner Exception" in "DbException" derived classes #34798

Closed
cheenamalhotra opened this issue Apr 10, 2020 · 8 comments

Comments

@cheenamalhotra
Copy link
Member

In SqlClient, we construct derived class object of System.Data.Common.DbException with an instance of InnerException & message. Whereas we also have ErrorCode internally in Inner Exception thrown by SQL Server which we cannot expose to users who catch this exception.

So basically, there is no way to provide InnerException as well as ErrorCode when constructing derived DbException object instance.

E.g.

try
{
    App();
}
catch (SqlException e)
{
    e.InnerException(); // Does not contain error code property
    e.ErrorCode; // Contains default int value, not helpful for end users. 
                 // No way to override this error code either.
}

Would like to know how can derived classes throw an exception that contains "Error Code" as well as "Inner Exception" with inner message as in this scenario and if that's something that can be considered in future?

@roji
Copy link
Member

roji commented Apr 10, 2020

@cheenamalhotra is this about somehow surfacing SqlError.Number on SqlException (although which one, given that SqlException has a collection of SqlErrors)? If so, is this a SqlClient-specific questions, given that other database provides represent error codes in a different way (e.g. strings)? If that's the case maybe it's better to move this to https://github.com/dotnet/SqlClient...

(note that I've commented with some ideas on dotnet/SqlClient#518)

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented Apr 13, 2020

Is this about somehow surfacing SqlError.Number on SqlException (although which one, given that SqlException has a collection of SqlErrors)?

Yes, the first parent exception number will be provided to user, as it is provided by SQL Server and also JDBC driver does the same: SqlServerException.java#L197

If someone is willing to capture SqlClient specific exception, only then they should reach to SqlErrors collection.

Is this a SqlClient-specific questions, given that other database provides represent error codes in a different way (e.g. strings)?

No, my point was related to DbException's "ErrorCode" property which is unused and contains garbage info everytime a SqlException is created. And if any data provider (SqlClient in this case) wants to use that property, there should be an option for us to do so. We just want an ability to provide Error Code when generating exception along with Inner Exception.

On the same note, how do other providers give error code/number with exceptions? Do users always have to parse the "Message" to get the same? I have always seen Error Numbers available with Exceptions at-least in Java, if not here, and it seems very weird that user applications have to parse "Message" everytime!

@roji
Copy link
Member

roji commented Apr 13, 2020

Oh, I missed that you were referring to the already-existing DbException.ErrorCode.

This property has a very specific use - for surfacing Windows HRESULT error codes coming up from native interop. I don't think it would be correct to use that property to expose other kinds of error codes (e.g. SQL Server codes) - if that's what you're asking. We can reach out and ask the .NET runtime folk about that to confirm, if you want.

On the same note, how do other providers give error code/number with exceptions? Do users always have to parse the "Message" to get the same? I have always seen Error Numbers available with Exceptions at-least in Java, if not here, and it seems very weird that user applications have to parse "Message" everytime!

No, parsing the "Message" is indeed a big anti-pattern as you've written. Every provider typically has their own subclass of DbException which exposes error information in a way that's specific to that provider and database. For example, Npgsql has PostgresException for PostgreSQL server errors; it exposes an SqlState string property which follows the SQL standard, as well as other contextual information on the error. Here's MySqlConnector's exception type, apparently exposing both a number and an SqlState string (no idea how exactly it works :)).

To summarize, error codes do seem to be exposed in good, structured ways across providers (outside of Message) - but unfortunately not in the same way.

@cheenamalhotra
Copy link
Member Author

cheenamalhotra commented Apr 13, 2020

Every provider typically has their own subclass of DbException which exposes error information in a way that's specific to that provider and database.

I wonder why there's no error code standard in .NET data providers.

If you see JDBC Specs, SqlException(Java) (equivalent of DbException) also exposes SQLState, which seems closest to me that even mssql-jdbc driver takes care of in SqlServerException implementation. If SqlClient could also provide SQLState, would it make sense to introduce that in DbException?

In that case we won't have to worry about ErrorCode (int) here, as long as we can provide 1 Identifier for Exception thrown that is receivable from base class.

@roji
Copy link
Member

roji commented Apr 13, 2020

I think you're absolutely right. I was thinking about this along with IsTransient, but looking around the various providers showed spotty support:

  1. SqlClient currently doesn't expose it. Thanks for the JDBC link - that's valuable information! It does seem like there's translation only for a precious few codes, so I wonder how useful that actually is (compare with PostgreSQL's error code page).
  2. Microsoft.Data.Sqlite also doesn't expose SQLState, but instead has one numeric error code and an additional "extended" numeric code.

To be honest, at this point I was discouraged with the divergent behavior and gave up. However, if you think adding a meaningful SQLState translation to SqlClient (similar to what the JDBC driver does, ideally more complete) is feasible, we could start a conversation about this on dotnet/runtime with the other provider maintainers. Let me know what you think.

I also think this question is orthogonal to:

  1. Adding something SqlErrorCode to SqlException (to expose errors more easily to users without going into the error list), and
  2. Adding IsTransient

@cheenamalhotra
Copy link
Member Author

Sure thing!

I will investigate on SqlClient side if there are any limitations, then we can create an API Proposal for SQLState support.

@roji
Copy link
Member

roji commented Apr 13, 2020

Ok! Do you prefer to wait until you check it out for SqlClient or should I open an issue for this tomorrow?

@cheenamalhotra
Copy link
Member Author

I'd like to discuss this with @saurabh500 once before we finalize in case there is a specific reason why this not supported till now in SqlClient, otherwise when comparing with Java Standards, this seems an appropriate API proposal for .NET Data providers.

You can open new issue to start discussions and I'll close this one, we can discuss more there.

@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jun 23, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants