-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Fix error handling of TxIndex.Search #4095
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4095 +/- ##
=========================================
+ Coverage 66.98% 67% +0.02%
=========================================
Files 247 247
Lines 20993 20993
=========================================
+ Hits 14062 14067 +5
+ Misses 5886 5883 -3
+ Partials 1045 1043 -2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! See my comment about using a switch below...
state/txindex/kv/kv.go
Outdated
@@ -180,9 +180,9 @@ func (txi *TxIndex) Search(q *query.Query) ([]*types.TxResult, error) { | |||
} else if ok { | |||
res, err := txi.Get(hash) | |||
if res == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what do you think about using a switch here?
switch {
case err != nil:
return []*types.TxResult{}, errors.Wrapf(err, "can't get tx %X", hash)
case res == nil:
return []*types.TxResult{}, nil
default:
return []*types.TxResult{res}, nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good comment :) I think it's nice to have explicit error handling for each state.
I'll change to use the switch statement.
Could you add an entry to CHANGELOG_PENDING.md?
|
This PR fixes error handling for performing a txindex search.
TxIndex.Get
returns(txresult, nil)
if the transaction is found.(nil, nil)
if the transaction is not found.(nil, error)
if error is occurred.Therefore, if
res
is notnil
, I thinkTxIndex.Search
should return(txresult, nil)
.Previously, however, this was not a problem because
errors.Wrap
returnsnil
if its first argumenterr
isnil
.