Collect all error messages for Error.Error()

[jegorbunov]

Sometimes mssql server reports a chain of errors with the last error having a message like: Could not drop constraint. See previous errors.. Since users code will generally print an error by simply using .Error() method to get the details about an underlying error then the method should provide as much detail as possible in my opinion.

CHANGELOG

BREAKING CHANGE:

Before this patch an Error() method would return a message like:

mssql: message

After this commit it returns:

mssql: message (error number)

And even:

mssql: message 1 (error number 1)
mssql: message 2 (error number 2)
...

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.60%. Comparing base (abddba8) to head (9602fa0).

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #250       +/-   ##
===========================================
+ Coverage   80.56%   96.60%   +16.04%     
===========================================
  Files          35       92       +57     
  Lines        6910    74361    +67451     
===========================================
+ Hits         5567    71840    +66273     
- Misses       1070     2186     +1116     
- Partials      273      335       +62     

Flag	Coverage Δ
unittests	96.54% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
error.go	100.00% <100.00%> (ø)

... and 59 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[shueybubbles]

🕐

[jegorbunov]

@shueybubbles, can you have another look?

I've fixed the comments: now tests do check how error messages are formed in a more detailed way.

[dlevy-msft-sql]

@jegorbunov friendly nudge — two items from the last review still open:

1. Nil-All regression (blocking): Error() returns "" when All is empty. token.go:165 constructs Error{Message: "Request failed but didn't provide reason"} without populating All, and external callers can do the same. Simplest fix:

   func (e Error) Error() string {
       if len(e.All) == 0 {
           return "mssql: " + e.Message
       }
       // ... existing All loop ...
   }

   That also lets you drop the err.All = []Error{err} workaround in TestError_Methods.

2. Single-error format change (design): For a single error the output goes from "mssql: msg" to "mssql: msg (num)". That is a breaking change for anyone matching error strings. Two reasonable resolutions:

   • Keep the old format for the 1-error case and only multi-line when len(All) > 1, or
   • Keep the new format but call it out in the PR body / CHANGELOG so the breaking change is explicit.

Happy to push the fix to your branch (maintainer_can_modify is enabled) if you would rather not iterate further — just let me know. Otherwise, take your time.

[jegorbunov]

@jegorbunov friendly nudge — two items from the last review still open:

1. Nil-All regression (blocking): Error() returns "" when All is empty. token.go:165 constructs Error{Message: "Request failed but didn't provide reason"} without populating All, and external callers can do the same. Simplest fix:

   func (e Error) Error() string {
       if len(e.All) == 0 {
           return "mssql: " + e.Message
       }
       // ... existing All loop ...
   }

   That also lets you drop the err.All = []Error{err} workaround in TestError_Methods.

2. Single-error format change (design): For a single error the output goes from "mssql: msg" to "mssql: msg (num)". That is a breaking change for anyone matching error strings. Two reasonable resolutions:

   • Keep the old format for the 1-error case and only multi-line when len(All) > 1, or
   • Keep the new format but call it out in the PR body / CHANGELOG so the breaking change is explicit.

Happy to push the fix to your branch (maintainer_can_modify is enabled) if you would rather not iterate further — just let me know. Otherwise, take your time.

Thanks for the wait.

I've fixed the first problem and added a CHANGELOG section to the PR/commit message as for the second problem. I believe that having an error number in the message is helpful enough for the change.