Collect all error messages for Error.Error()#250
Collect all error messages for Error.Error()#250jegorbunov wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #250 +/- ##
==========================================
- Coverage 74.91% 74.88% -0.03%
==========================================
Files 32 32
Lines 6457 6463 +6
==========================================
+ Hits 4837 4840 +3
- Misses 1332 1334 +2
- Partials 288 289 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| msg.WriteString(fmt.Sprintf("mssql: %s (%d)", err.Message, err.Number)) | ||
| } | ||
| return msg.String() | ||
| } |
There was a problem hiding this comment.
how is this change not affecting any tests? Is there really no coverage of this call?
There was a problem hiding this comment.
Yep. There is no tests which would check how the string returned by Error() string is formed. Do you think we need them?
There was a problem hiding this comment.
without a test, any behavior is considered correct. I think having a test now is worthwhile because it documents the contract that callers can expect.
| func (e Error) Error() string { | ||
| return "mssql: " + e.Message | ||
| var msg strings.Builder | ||
| for i, err := range e.All { |
There was a problem hiding this comment.
It seems to me that the natural order is the correct one, eg:
mssql: .... actual reason ...
mssql: Failed to drop constraint. See previous error.
So, it is like error log printed when you run an sql query in your terminal.
But I am not sure and open for any suggestions. The main problem I am trying to solve as stated in the PR description is to make Error() return enough details to take action.
There was a problem hiding this comment.
your example implies there are Error objects without a Number. Where will See previous error show up?
3 participants
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.