Skip to content

Commit

Permalink
internal/ctlog: use fmt.Errorf("%w: ...") instead of errors.Join
Browse files Browse the repository at this point in the history
Suggested by @cristaloleg at #12 (comment)
  • Loading branch information
FiloSottile committed Mar 17, 2024
1 parent 4a6a5bf commit 35a389f
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 10 deletions.
4 changes: 2 additions & 2 deletions internal/ctlog/ctlog.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ func (l *Log) sequencePool(ctx context.Context, p *pool) (err error) {

timestamp := timeNowUnixMilli()
if timestamp <= l.tree.Time {
return errors.Join(errFatal, fmtErrorf("time did not progress! %d -> %d", l.tree.Time, timestamp))
return fmt.Errorf("%w: time did not progress! %d -> %d", errFatal, l.tree.Time, timestamp)
}

edgeTiles := maps.Clone(l.edgeTiles)
Expand Down Expand Up @@ -798,7 +798,7 @@ func (l *Log) sequencePool(ctx context.Context, p *pool) (err error) {
// This is a critical error, since we don't know the state of the
// checkpoint in the database at this point. Bail and let LoadLog get us
// to a good state after restart.
return errors.Join(errFatal, fmtErrorf("couldn't upload checkpoint to database: %w", err))
return fmt.Errorf("%w: couldn't upload checkpoint to database: %w", errFatal, err)
}

// At this point the pool is fully serialized: the new tree was uploaded to
Expand Down
31 changes: 31 additions & 0 deletions internal/ctlog/ctlog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"crypto/sha256"
"crypto/x509"
"encoding/base64"
"errors"
"flag"
mathrand "math/rand"
"sync/atomic"
Expand Down Expand Up @@ -353,6 +354,36 @@ func TestReloadWrongKey(t *testing.T) {
}
}

func TestFatalError(t *testing.T) {
tl := NewEmptyTestLog(t)
addCertificate(t, tl)
addCertificate(t, tl)
fatalIfErr(t, tl.Log.Sequence())

lockErr := errors.New("lock replace error")
tl.Config.Lock.(*MemoryLockBackend).ReplaceCallback = func(
old ctlog.LockedCheckpoint, new []byte) error {
return lockErr
}

addCertificateExpectFailure(t, tl)
addCertificateExpectFailure(t, tl)
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()
if err := tl.Log.RunSequencer(ctx, 50*time.Millisecond); !errors.Is(err, lockErr) {
t.Errorf("expected fatal error, got %v", err)
}
tl.CheckLog()

tl.Config.Lock.(*MemoryLockBackend).ReplaceCallback = nil

tl = ReloadLog(t, tl)
addCertificate(t, tl)
fatalIfErr(t, tl.Log.Sequence())
tl.CheckLog()
// TODO: expect log size 3.
}

func BenchmarkSequencer(b *testing.B) {
tl := NewEmptyTestLog(b)
b.ResetTimer()
Expand Down
4 changes: 2 additions & 2 deletions internal/ctlog/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,8 @@ func fmtErrorf(format string, args ...interface{}) error {
category = strings.TrimSpace(category)
category = strings.TrimSuffix(category, ":")
err := fmt.Errorf(format, args...)
if err, ok := errors.Unwrap(err).(categoryError); ok {
category = category + ": " + err.category
if subCatErr := new(categoryError); errors.As(err, subCatErr) {
category = category + ": " + subCatErr.category
}
return categoryError{category: category, err: err}
}
Expand Down
33 changes: 27 additions & 6 deletions internal/ctlog/testlog_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,20 @@ func (tl *TestLog) StartSequencer() {
}()
}

func waitFuncWrapper(t testing.TB, le *ctlog.LogEntry, f func(ctx context.Context) (*ctlog.SequencedLogEntry, error)) func(ctx context.Context) (*ctlog.SequencedLogEntry, error) {
func waitFuncWrapper(t testing.TB, le *ctlog.LogEntry, expectSuccess bool,
f func(ctx context.Context) (*ctlog.SequencedLogEntry, error),
) func(ctx context.Context) (*ctlog.SequencedLogEntry, error) {
var called bool
fw := func(ctx context.Context) (*ctlog.SequencedLogEntry, error) {
called = true
se, err := f(ctx)
if err != nil {
if !expectSuccess {
if err == nil {
t.Error("expected an error")
}
} else if err != nil {
t.Error(err)
}
if !reflect.DeepEqual(*le, se.LogEntry) {
} else if !reflect.DeepEqual(*le, se.LogEntry) {
t.Error("LogEntry is different")
}
return se, err
Expand All @@ -256,7 +262,7 @@ func addCertificateWithSeed(t *testing.T, tl *TestLog, seed int64) func(ctx cont
e.Certificate = make([]byte, r.Intn(4)+8)
r.Read(e.Certificate)
f, _ := tl.Log.AddLeafToPool(e)
return waitFuncWrapper(t, e, f)
return waitFuncWrapper(t, e, true, f)
}

func addCertificateFast(t *testing.T, tl *TestLog) {
Expand All @@ -266,6 +272,14 @@ func addCertificateFast(t *testing.T, tl *TestLog) {
tl.Log.AddLeafToPool(e)
}

func addCertificateExpectFailure(t *testing.T, tl *TestLog) {
e := &ctlog.LogEntry{}
e.Certificate = make([]byte, mathrand.Intn(4)+8)
rand.Read(e.Certificate)
f, _ := tl.Log.AddLeafToPool(e)
waitFuncWrapper(t, e, false, f)
}

func addPreCertificate(t *testing.T, tl *TestLog) func(ctx context.Context) (*ctlog.SequencedLogEntry, error) {
return addPreCertificateWithSeed(t, tl, mathrand.Int63())
}
Expand All @@ -283,7 +297,7 @@ func addPreCertificateWithSeed(t *testing.T, tl *TestLog, seed int64) func(ctx c
r.Read(e.PrecertSigningCert)
}
f, _ := tl.Log.AddLeafToPool(e)
return waitFuncWrapper(t, e, f)
return waitFuncWrapper(t, e, true, f)
}

const tileWidth = 1 << ctlog.TileHeight
Expand Down Expand Up @@ -365,6 +379,8 @@ type MemoryLockBackend struct {
t testing.TB
mu sync.Mutex
m map[[sha256.Size]byte][]byte

ReplaceCallback func(old ctlog.LockedCheckpoint, new []byte) error
}

type memoryLockCheckpoint struct {
Expand Down Expand Up @@ -412,6 +428,11 @@ func (b *MemoryLockBackend) Replace(ctx context.Context, old ctlog.LockedCheckpo
if err := ctx.Err(); err != nil {
return nil, err
}
if b.ReplaceCallback != nil {
if err := b.ReplaceCallback(old, new); err != nil {
return nil, err
}
}
b.mu.Lock()
defer b.mu.Unlock()
if old == nil {
Expand Down

0 comments on commit 35a389f

Please sign in to comment.