refactor(x/upgrade)!: use KVStoreService and context.Context #16227
Conversation
| return nil, err | ||
| } | ||
| defer it.Close() |
There was a problem hiding this comment.
if it errors above is this still run?
There was a problem hiding this comment.
No, it returns the error immediately, skipping the defer statement.
There was a problem hiding this comment.
Static check complains about it with should check returned error before deferring it.Close() (SA5001). I suppose it makes sense because if it's returning an error then the iterator never got opened
There was a problem hiding this comment.
that makes sense.
Its completely optional, but in this case maybe we need to handle err returned from it.Close() since there is a possibility of errors coming up while closing like resource availability, connection errors etc.
Maybe something like this:
defer func() {
if err := it.Close(); err != nil {
// handle the err/log
}
}()
There was a problem hiding this comment.
We've been ignoring these errors all over the SDK, would like to fix them in a follow up PR instead of adding that to this one
| prefix := []byte{types.VersionMapByte} | ||
| it, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
I think we need to close the iterator before returning the error
| return nil, err | |
| _ = it.Close() | |
| return nil, err |
| prefix := []byte{types.VersionMapByte} | ||
| it, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) | ||
| if err != nil { | ||
| return nil, err |
| return nil, err | ||
| } | ||
| defer it.Close() |
There was a problem hiding this comment.
No, it returns the error immediately, skipping the defer statement.
| prefix := []byte{types.VersionMapByte} | ||
| it, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) | ||
| if err != nil { | ||
| return 0, err |
| prefix := []byte{types.DoneByte} | ||
| it, err := store.Iterator(prefix, storetypes.PrefixEndBytes(prefix)) | ||
| if err != nil { | ||
| return 0, err |
Description
TO-DO:
context.Contextintype MigrationHandler func(sdk.Context) error. It touches pretty much all modules, so it's going to be a big PR (doing it in a follow up pr)Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change