Update core store iterator API to return errors

[aaronc]

In #14622, we added error return values to the cosmossdk.io/core/store.Store methods. In discussions in that PR, it was noted that the Iterator interface doesn't have error return values but rather an Error() method which isn't used correctly. As a follow-up, we would like to adapt the Iterator interface to return errors in appropriate places.

[tac0turtle]

This should also be opened on cosmos-db. The interface is there. Should we do this as part of v1 on cosmos-db or v2?

[tac0turtle]

this is something we should keep in mind for store v2 but no need to change the current design. With things like orm and collections teams wont really need to deal with this api directly

[mhofman]

This issue has bitten me hard, I don't understand why it was closed.

Cosmos iterators borrow the tendermint iterator interface, which according to the documentation are supposed to be used with the following pattern:

var itr Iterator = ...
defer itr.Close()
for ; itr.Valid(); itr.Next() {
  k, v := itr.Key(); itr.Value()
  ...
}
if err := itr.Error(); err != nil {
  ...
}

However when you do so with most iterators returned by cosmos, err will not be nil for successful iterations that ran to completion. Error() is not supposed to be an "alias" for Valid().

With cosmos iterators there is currently no way to know if an iteration stopped because of an error or because it ran to completion.

[alexanderbez]

Yikes, that seems like a pretty bad design flaw :/