Repository navigation
Conversation
… in severe errors For severity 20 and above, go-mssqldb returns mssql.ServerError, whose Error() is the fixed "SQL Server had internal error". The errors the server sent are kept in the mssql.Error it wraps: its own fields describe the last one, and its All field lists every error from first to last. RunSQL in both sources now appends every entry of All, formatted from its fields, after the existing text, and keeps the original error wrapped. Other errors are unchanged. Fixes googleapis#4444
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a common error-handling utility for MSSQL sources to extract and format detailed SQL Server errors from severe database errors, and integrates it into both the Cloud SQL MSSQL and standard MSSQL sources. Feedback on the changes suggests checking the return value of errors.As when extracting the underlying mssql.Error to prevent passing an uninitialized zero-value struct to the formatting function.
…lt for the wrapped mssql.Error Look up mssql.ServerError and the mssql.Error it wraps in one condition and return the error unchanged if either is missing, instead of relying on an empty mssql.Error formatting to "".
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
For SQL Server errors of severity 20 and above, the
mssqlandcloud-sql-mssqlsources answer with onlySQL Server had internal error: no error number, severity or message (#4444).go-mssqldb returns
mssql.ServerErrorfor these errors. ItsError()is that fixed text; the errors the server sent are kept in themssql.Errorit wraps. That error's own fields describe the last error, which is usually a consequence ("the session is in the kill state"); itsAllfield lists every error from first to last, and the first is the cause.This PR adds
mssqlcommon.WithServerErrorDetails, whichRunSQLin both sources now applies at its two error returns (the query call andresults.Err()). For anmssql.ServerErrorit appends every entry ofAll, in order, formatted from its fields, after the existing text, and keeps the original error wrapped, soerrors.Asstill finds it. Every other error is returned unchanged.RAISERROR('severe error text', 20, 1) WITH LOGthroughmssql-execute-sql, before and after (SQL Server 2025 CU10):Design notes:
mssql.Error.Error(), whose format changes in go-mssqldb v1.11 (Collect all error messages for Error.Error() microsoft/go-mssqldb#250, which chore(deps): update go #4246 brings in). The output is the same on v1.10.0 and v1.11.2.erroris lower case on purpose:util.ProcessGeneralErrorclassifies byError 401,Error 403andError 429, and SQL Server has errors with those numbers. A unit test checks that the category staysAGENT_ERROR.SQL Server had internal errorprefix stays, for clients that match on it.internal/tools/mssql/mssqlcommon, followingmysqlcommon. fix(source/mssql,source/cloudsqlmssql): return decimal, money and GUID values as text instead of base64 #4284 addsmssqlcommon.goto the same package, so the two merge in either order.Testing:
internal/tools/mssql/mssqlcommon/errors_test.go.mssql.ServerErrorkeeps the wrapped error in an unexported field, so the tests stand in for it witherrors.Join(mssql.Error{...}, mssql.ServerError{}).tests.yaml(build, bothgo test -raceruns, coverage gate), thego mod tidycheck, and golangci-lint v2.15.0 on the changed packages (0 issues), all in agolang:1.27.3container.mssqlcommoncode is at 100%.internal/sources/mssql(8.2%) andinternal/sources/cloudsqlmssql(7.9%) are the same as onmain;RunSQL, where the two call sites are, is reached only by the integration tests.Dockerfile, against SQL Server 2025 CU10: both error returns (the query call above, andresults.Err()viaSELECT 1 AS a; RAISERROR(...)) now carry the chain. Severity 16 errors, divide by zero, result sets and multiple result sets are unchanged. The chain the helper formats is the same on SQL Server 2017 CU31, 2019 CU32, 2022 CU27 and 2025 CU10, with go-mssqldb v1.10.0 and v1.11.2 (details in mssql: severe errors (severity 20 and above) lose the server's message: "SQL Server had internal error" #4444).WITH LOGneeds a login in the sysadmin role or withALTER TRACE(a plain login gets error 2778), and I don't know whether the CI logins have either. If they do, I can add one totests/mssql.cloud-sql-mssqlgets the same change at the same two returns; I couldn't run it against Cloud SQL.Security:
PR Checklist
CONTRIBUTING.md
bug/issue
before writing your code! That way we can discuss the change, evaluate
designs, and agree on the general idea
review
!if this involve a breaking change: not breaking; the existing text stays as the prefix🛠️ Fixes #4444