Skip to content
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

refactor(store): add missing error checks in store #17817

Merged
merged 1 commit into from
Sep 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions store/iavl/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@
// Implements types.KVStore.
func (st *Store) Delete(key []byte) {
defer telemetry.MeasureSince(time.Now(), "store", "iavl", "delete")
st.tree.Remove(key)
if _, _, err := st.tree.Remove(key); err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
}

// DeleteVersions deletes a series of versions from the MutableTree. An error
Expand Down Expand Up @@ -370,7 +372,8 @@
for ; iterator.Valid(); iterator.Next() {
pairs.Pairs = append(pairs.Pairs, kv.Pair{Key: iterator.Key(), Value: iterator.Value()})
}
iterator.Close()

_ = iterator.Close()

bz, err := pairs.Marshal()
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion store/listenkv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@
// onWrite writes a KVStore operation to all of the WriteListeners
func (s *Store) onWrite(delete bool, key, value []byte) {
for _, l := range s.listeners {
l.OnWrite(s.parentStoreKey, key, value, delete)
if err := l.OnWrite(s.parentStoreKey, key, value, delete); err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
}
}
}
17 changes: 12 additions & 5 deletions store/rootmulti/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func deleteKVStore(kv types.KVStore) error {
keys = append(keys, itr.Key())
itr.Next()
}
itr.Close()
_ = itr.Close()

for _, k := range keys {
kv.Delete(k)
Expand All @@ -328,7 +328,7 @@ func moveKVStoreData(oldDB types.KVStore, newDB types.KVStore) error {
newDB.Set(itr.Key(), itr.Value())
itr.Next()
}
itr.Close()
_ = itr.Close()

// then delete the old store
return deleteKVStore(oldDB)
Expand Down Expand Up @@ -1068,7 +1068,9 @@ func (rs *Store) GetCommitInfo(ver int64) (*types.CommitInfo, error) {
func (rs *Store) flushMetadata(db dbm.DB, version int64, cInfo *types.CommitInfo) {
rs.logger.Debug("flushing metadata", "height", version)
batch := db.NewBatch()
defer batch.Close()
defer func() {
_ = batch.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why have this instead of only batch.Close()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because otherwise the error isn't checked. (here too, but now it's explicitly not checked)

}()

if cInfo != nil {
flushCommitInfo(batch, version, cInfo)
Expand Down Expand Up @@ -1168,7 +1170,10 @@ func flushCommitInfo(batch dbm.Batch, version int64, cInfo *types.CommitInfo) {
}

cInfoKey := fmt.Sprintf(commitInfoKeyFmt, version)
batch.Set([]byte(cInfoKey), bz)

if err := batch.Set([]byte(cInfoKey), bz); err != nil {
panic(err)
}
}

func flushLatestVersion(batch dbm.Batch, version int64) {
Expand All @@ -1177,5 +1182,7 @@ func flushLatestVersion(batch dbm.Batch, version int64) {
panic(err)
}

batch.Set([]byte(latestVersionKey), bz)
if err := batch.Set([]byte(latestVersionKey), bz); err != nil {
panic(err)
}
}
8 changes: 5 additions & 3 deletions store/streaming/constructor.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func LoadStreamingServices(
// Close any services we may have already spun up before hitting the error
// on this one.
for _, activeStreamer := range activeStreamers {
activeStreamer.Close()
_ = activeStreamer.Close()
}

return nil, nil, err
Expand All @@ -176,7 +176,7 @@ func LoadStreamingServices(
// Close any services we may have already spun up before hitting the error
// on this one.
for _, activeStreamer := range activeStreamers {
activeStreamer.Close()
_ = activeStreamer.Close()
}

return nil, nil, err
Expand All @@ -186,7 +186,9 @@ func LoadStreamingServices(
bApp.SetStreamingService(streamingService)

// kick off the background streaming service loop
streamingService.Stream(wg)
if err := streamingService.Stream(wg); err != nil {
return nil, nil, err
}

// add to the list of active streamers
activeStreamers = append(activeStreamers, streamingService)
Expand Down
4 changes: 3 additions & 1 deletion store/tracekv/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,5 +195,7 @@
panic(errors.Wrap(err, "failed to write trace operation"))
}

io.WriteString(w, "\n")
if _, err = io.WriteString(w, "\n"); err != nil {
panic(errors.Wrap(err, "failed to write newline"))
Dismissed Show dismissed Hide dismissed
}
}
Loading