search

denisenkom / go-mssqldb

Microsoft SQL server driver written in go language
BSD 3-Clause "New" or "Revised" License
1.82k stars 495 forks source link

Add support for multiple errors #655

Closed tc-hib closed 3 years ago

tc-hib commented 3 years ago

This is a proposal to fix #39 (returning multiple errors)

In Error, add a Previous field that's a slice of other Error, starting from the most recent.

It might not be fully backward compatible as it makes the struct non-comparable.

codecov[bot] commented 3 years ago

Codecov Report

Merging #655 (09006fe) into master (74bac08) will increase coverage by 0.13%. The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #655      +/-   ##
==========================================
+ Coverage   70.26%   70.40%   +0.13%     
==========================================
  Files          23       23              
  Lines        5146     5149       +3     
==========================================
+ Hits         3616     3625       +9     
+ Misses       1304     1300       -4     
+ Partials      226      224       -2
Impacted Files Coverage Δ
error.go 75.00% <ø> (ø)
token.go 61.11% <83.33%> (+0.18%) :arrow_up:
tds.go 66.87% <0.00%> (+0.73%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 74bac08...09006fe. Read the comment docs.

kardianos commented 3 years ago

@tc-hib The code looks fine. I would argue for a slightly different Error struct.

  1. I agree the current Error struct should continue to be the most recent error to preserve compatibility.
  2. Rather then a Previous struct, that hold ones in desc order and excluding the current error, that it should be named All or similar. If you are dealing with a simple error setup, the most recent is appropriate. However, if you are dealing with a more robust query system, you will want to copy over All errors and display them the top error first, as errors often cascade.
tc-hib commented 3 years ago

Hello,

Thanks for having a look. I'll fix it as soon as I'm back home. If there's only one error, would you still store it in the All member ? I would, for consistency. (and simplicity)

kardianos commented 3 years ago

I would too. This would ensure any way you access it, it is simple and prevents foot guns.