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

Retain error information in SQLServerPreparedStatement.getMetaData exceptions #1430

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

danburkert
Copy link
Contributor

Fixes a bug in SQLServerPreparedStatement.buildExecuteMetaData which
caused the SqlState as returned from the server error to be discarded in
thrown exceptions. The SqlState information is useful for application
level error handling.

No tests are provided, as I can't get the test harness to run in my
local environment. However, here are steps to reproduce the 'bad'
exception with no included SqlState:

Connection conn = null;     // Fill in with SqlServer connection.
conn.prepareStatement("s");
conn.getMetaData();         // Throws exception without SqlState.

…ceptions

Fixes a bug in SQLServerPreparedStatement.buildExecuteMetaData which
caused the SqlState as returned from the server error to be discarded in
thrown exceptions. The SqlState information is useful for application
level error handling.

No tests are provided, as I can't get the test harness to run in my
local environment. However, here are steps to reproduce the 'bad' exception
with no included SqlState:

```java
Connection conn = null;     // Fill in with SqlServer connection.
conn.prepareStatement("s");
conn.getMetaData();         // Throws exception without SqlState.
```
@ghost
Copy link

ghost commented Sep 17, 2020

CLA assistant check
All CLA requirements met.

@ulvii ulvii added this to the 9.1.0 milestone Sep 28, 2020
@@ -1050,7 +1050,7 @@ else if (needsPrepare)
}

@Override
public final java.sql.ResultSetMetaData getMetaData() throws SQLServerException {
public final java.sql.ResultSetMetaData getMetaData() throws SQLServerException, SQLTimeoutException {
Copy link
Contributor

@ulvii ulvii Sep 28, 2020

Choose a reason for hiding this comment

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

Is adding SQLTimeoutException necessary here? Could you explain this a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. buildExecuteMetaData() previously caught and wrapped the SQLTimeoutException in another exception with less info. That's been changed so that it's no longer caught, and it gets thrown directly. The JDBC API allows for throwing this exception type, so it shouldn't be rethrown with less info.

@ulvii
Copy link
Contributor

ulvii commented Oct 9, 2020

Hi @danburkert ,
Would you sign the Contributor License Agreement ?

@danburkert
Copy link
Contributor Author

Hi @danburkert ,
Would you sign the Contributor License Agreement ?

done!

@ulvii ulvii merged commit a497714 into microsoft:dev Oct 13, 2020
@ulvii
Copy link
Contributor

ulvii commented Oct 13, 2020

Thanks for the contribution @danburkert .

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