Skip to content

Commit

Permalink
Add new error handling function for non defer context. Apply changes …
Browse files Browse the repository at this point in the history
…requested in review
  • Loading branch information
facuMH committed Dec 18, 2024
1 parent 1b0fa6e commit 61af49f
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 20 deletions.
6 changes: 3 additions & 3 deletions cmd/sonicd/app/launcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,10 +202,10 @@ func initApp() {
return nil
}

app.After = func(ctx *cli.Context) (err error) {
app.After = func(ctx *cli.Context) error {
debug.Exit()
caution.CloseAndReportError(&err, prompt.Stdin, "failed to reset terminal input") // Resets terminal mode.
return
// Close will resets terminal mode.
return caution.IfErrorAddContext(prompt.Stdin.Close(), "failed to reset terminal input")
}
}

Expand Down
3 changes: 2 additions & 1 deletion cmd/sonictool/app/genesis.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package app

import (
"errors"
"fmt"
"os"
"strconv"
Expand Down Expand Up @@ -57,7 +58,7 @@ func gfileGenesisImport(ctx *cli.Context) (err error) {

genesisStore, genesisHashes, err := genesisstore.OpenGenesisStore(genesisReader)
if err != nil {
return fmt.Errorf("failed to read genesis file: %w", err)
return errors.Join(fmt.Errorf("failed to read genesis file: %w", err), genesisReader.Close())
}
defer caution.CloseAndReportError(&err, genesisStore, "failed to close the genesis store")
if err := genesis.IsGenesisTrusted(genesisStore, genesisHashes); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion cmd/sonictool/app/heal.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func healLiveFromArchive(ctx context.Context, carmenLiveDir, carmenArchiveDir st
wg.Add(1)
go func() {
defer wg.Done()
defer caution.CloseAndReportError(&err, writer, "failed to close writer")
defer caution.CloseAndReportError(&exportErr, writer, "failed to close writer")
exportErr = mptio.ExportBlockFromArchive(ctx, mptio.NewLog(), carmenArchiveDir, bufWriter, uint64(recoveredBlock))
if exportErr == nil {
exportErr = bufWriter.Flush()
Expand Down
13 changes: 8 additions & 5 deletions cmd/sonictool/app/sign_genesis.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package app

import (
"errors"
"fmt"
"os"

Expand Down Expand Up @@ -59,18 +60,20 @@ func signGenesis(ctx *cli.Context) error {
return nil
}

func getGenesisHeaderHashes(genesisFile string) (ogenesis.Header, ogenesis.Hashes, error) {
func getGenesisHeaderHashes(genesisFile string) (header ogenesis.Header, genesisHashes ogenesis.Hashes, err error) {
genesisReader, err := os.Open(genesisFile)
// note, genesisStore closes the reader, no need to defer close it here
if err != nil {
return ogenesis.Header{}, nil, fmt.Errorf("failed to open the genesis file: %w", err)
err = fmt.Errorf("failed to open the genesis file: %w", err)
return
}

genesisStore, genesisHashes, err := genesisstore.OpenGenesisStore(genesisReader)
if err != nil {
return ogenesis.Header{}, nil, fmt.Errorf("failed to read genesis file: %w", err)
err = errors.Join(fmt.Errorf("failed to read genesis file: %w", err), genesisReader.Close())
return
}
defer caution.CloseAndReportError(&err, genesisStore, "failed to close the genesis store")

return genesisStore.Header(), genesisHashes, nil
header = genesisStore.Header()
return
}
4 changes: 3 additions & 1 deletion cmd/sonictool/genesis/signature.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,9 @@ func WriteSignatureIntoGenesisFile(header genesis.Header, signature []byte, file
return
}
_, err = writer.Flush()
err = fmt.Errorf("failed to flush genesis file: %w", err)
if err != nil {
err = fmt.Errorf("failed to flush genesis file: %w", err)
}
return
}

Expand Down
5 changes: 3 additions & 2 deletions debug/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,9 @@ func (h *HandlerT) StartCPUProfile(file string) error {
return err
}
if err := pprof.StartCPUProfile(f); err != nil {
caution.CloseAndReportError(&err, f, "failed to close profile file")
return err
return errors.Join(
fmt.Errorf("failed to start CPU: %w", err),
caution.IfErrorAddContext(f.Close(), "failed to close CPU profile file"))
}
h.cpuW = f
h.cpuFile = file
Expand Down
6 changes: 4 additions & 2 deletions debug/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package debug

import (
"errors"
"fmt"
"os"
"runtime/trace"

Expand All @@ -40,8 +41,9 @@ func (h *HandlerT) StartGoTrace(file string) error {
return err
}
if err := trace.Start(f); err != nil {
caution.CloseAndReportError(&err, f, "failed to close trace file")
return err
return errors.Join(
fmt.Errorf("failed to start Go trace: %w", err),
caution.IfErrorAddContext(f.Close(), "failed to close trace file"))
}
h.traceW = f
h.traceFile = file
Expand Down
5 changes: 4 additions & 1 deletion topicsdb/search_parallel.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"sync"

"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/log"
)

type logHandler func(rec *logrec) (gonext bool, err error)
Expand Down Expand Up @@ -117,5 +118,7 @@ func (tt *index) scanPatternVariant(pos uint8, variant common.Hash, start uint64
break
}
}
onMatched(nil)
if _, err := onMatched(nil); err != nil {
log.Warn("searchParallel", "err", err)
}
}
8 changes: 8 additions & 0 deletions utils/caution/caution.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,11 @@ func CloseAndReportError(err *error, closer io.Closer, message string) {
return closer.Close()
}, message)
}

// IfErrorAddContext adds a message to an error, if the error is not nil.
func IfErrorAddContext(err error, message string) error {
if err == nil {
return nil
}
return fmt.Errorf("%s: %w", message, err)
}
12 changes: 12 additions & 0 deletions utils/caution/caution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,15 @@ func TestCloseAndReportError_UsagePatternPropagatesError(t *testing.T) {
gotError := testFun()
require.ErrorIs(t, gotError, expectedError)
}

func TestIfErrorAddContext_PropagatesNil(t *testing.T) {
if IfErrorAddContext(nil, "message") != nil {
t.Error("IfErrorAddContext should return nil when err is nil")
}
}

func TestIfErrorAddContext_AddsContextToError(t *testing.T) {
err := fmt.Errorf("someError")
errWithContext := IfErrorAddContext(err, "message")
require.ErrorContains(t, errWithContext, "message: someError")
}
10 changes: 6 additions & 4 deletions valkeystore/encryption/io.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package encryption

import (
"errors"
"fmt"
"os"
"path/filepath"
Expand All @@ -23,10 +24,11 @@ func writeTemporaryKeyFile(file string, content []byte) (string, error) {
}

if _, err = f.Write(content); err != nil {
caution.CloseAndReportError(&err, f, "failed to close key file")
caution.ExecuteAndReportError(&err, func() error { return os.Remove(f.Name()) },
"failed to remove temporary key file")
return "", err
return "", errors.Join(
fmt.Errorf("failed to write key file: %w", err),
caution.IfErrorAddContext(f.Close(), "failed to close key file"),
caution.IfErrorAddContext(os.Remove(f.Name()), "failed to remove temporary key file"),
)
}

return f.Name(), f.Close()
Expand Down

0 comments on commit 61af49f

Please sign in to comment.