Skip to content

Commit

Permalink
fix: sentinel errors should not have stack traces (#2042)
Browse files Browse the repository at this point in the history
## Problem

While debugging a program which is using badger I noticed that stack
traces returned from my code (which uses errors with stack traces) are
useless. I found the reason: badger uses sentinel errors (great!) but it
creates them with stack traces at the "global init time" (e.g., `var
ErrFoo = errors.New(...)`) and not at "return from function time".
Because of that I was seeing stack traces like:

```
github.com/dgraph-io/badger/v4.init
	/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/compaction.go:41
runtime.doInit1
	/usr/local/go/src/runtime/proc.go:6740
runtime.doInit
	/usr/local/go/src/runtime/proc.go:6707
runtime.main
	/usr/local/go/src/runtime/proc.go:249
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1650
```

Instead of a stack trace which would show me where the call which is
returning sentinel error is made.

## Solution

Ideally, one would create all sentinel errors without stack traces
(standard errors package works great for that) and then when the error
happens in a function, you return `errors.WithStack(ErrFoobar)` instead
of just `ErrFoobar`. This means that sentinel error is then annotated
with a stack trace at the place where the error happens. But if you do
that, then you have to check for sentinel errors with `errors.Is`
instead of just comparing errors with equality (`==`). That might be a
breaking change for people who are not using `errors.Is` as they should.
Because badger's own codebase uses equality instead of `errors.Is` I
decided to not do this in this PR, but instead:

Just make sentinel errors without stack traces. This allows me to
annotate with a stack trace inside my program (otherwise my program does
not do so because if finds an existing stack trace and assumes it is a
deeper one) and I get at least that part of the stack trace. Equality
still works as usual.

I suggest this is merged this way and in v5 of badger `errors.WithStack`
and required `errors.Is` are introduced.

## Side note

Some time ago [I made this errors package for
Go](https://gitlab.com/tozd/go/errors) which fixes various issues in
`github.com/pkg/errors` and also has direct support for sentinel (in
that package called "base") errors. It also has some features badger has
in its `y` package for dealing with errors. You could consider adopting
that package to have a comprehensive errors handling in badger.
  • Loading branch information
mitar authored Oct 14, 2024
1 parent 16b63df commit de3932e
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 45 deletions.
3 changes: 2 additions & 1 deletion badger/cmd/bank.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"context"
"errors"
stderrors "errors"
"fmt"
"log"
"math"
Expand Down Expand Up @@ -148,7 +149,7 @@ func min(a, b uint64) uint64 {
return b
}

var errAbandoned = errors.New("Transaction abandonded due to insufficient balance")
var errAbandoned = stderrors.New("Transaction abandonded due to insufficient balance")

func moveMoney(db *badger.DB, from, to int) error {
return db.Update(func(txn *badger.Txn) error {
Expand Down
3 changes: 2 additions & 1 deletion db.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"context"
"encoding/binary"
stderrors "errors"
"expvar"
"fmt"
"math"
Expand Down Expand Up @@ -1015,7 +1016,7 @@ func (db *DB) batchSetAsync(entries []*Entry, f func(error)) error {
return nil
}

var errNoRoom = errors.New("No room for write")
var errNoRoom = stderrors.New("No room for write")

// ensureRoomForWrite is always called serially.
func (db *DB) ensureRoomForWrite() error {
Expand Down
57 changes: 28 additions & 29 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
package badger

import (
stderrors "errors"
"math"

"github.com/pkg/errors"
)

const (
Expand All @@ -30,97 +29,97 @@ const (
var (
// ErrValueLogSize is returned when opt.ValueLogFileSize option is not within the valid
// range.
ErrValueLogSize = errors.New("Invalid ValueLogFileSize, must be in range [1MB, 2GB)")
ErrValueLogSize = stderrors.New("Invalid ValueLogFileSize, must be in range [1MB, 2GB)")

// ErrKeyNotFound is returned when key isn't found on a txn.Get.
ErrKeyNotFound = errors.New("Key not found")
ErrKeyNotFound = stderrors.New("Key not found")

// ErrTxnTooBig is returned if too many writes are fit into a single transaction.
ErrTxnTooBig = errors.New("Txn is too big to fit into one request")
ErrTxnTooBig = stderrors.New("Txn is too big to fit into one request")

// ErrConflict is returned when a transaction conflicts with another transaction. This can
// happen if the read rows had been updated concurrently by another transaction.
ErrConflict = errors.New("Transaction Conflict. Please retry")
ErrConflict = stderrors.New("Transaction Conflict. Please retry")

// ErrReadOnlyTxn is returned if an update function is called on a read-only transaction.
ErrReadOnlyTxn = errors.New("No sets or deletes are allowed in a read-only transaction")
ErrReadOnlyTxn = stderrors.New("No sets or deletes are allowed in a read-only transaction")

// ErrDiscardedTxn is returned if a previously discarded transaction is re-used.
ErrDiscardedTxn = errors.New("This transaction has been discarded. Create a new one")
ErrDiscardedTxn = stderrors.New("This transaction has been discarded. Create a new one")

// ErrEmptyKey is returned if an empty key is passed on an update function.
ErrEmptyKey = errors.New("Key cannot be empty")
ErrEmptyKey = stderrors.New("Key cannot be empty")

// ErrInvalidKey is returned if the key has a special !badger! prefix,
// reserved for internal usage.
ErrInvalidKey = errors.New("Key is using a reserved !badger! prefix")
ErrInvalidKey = stderrors.New("Key is using a reserved !badger! prefix")

// ErrBannedKey is returned if the read/write key belongs to any banned namespace.
ErrBannedKey = errors.New("Key is using the banned prefix")
ErrBannedKey = stderrors.New("Key is using the banned prefix")

// ErrThresholdZero is returned if threshold is set to zero, and value log GC is called.
// In such a case, GC can't be run.
ErrThresholdZero = errors.New(
ErrThresholdZero = stderrors.New(
"Value log GC can't run because threshold is set to zero")

// ErrNoRewrite is returned if a call for value log GC doesn't result in a log file rewrite.
ErrNoRewrite = errors.New(
ErrNoRewrite = stderrors.New(
"Value log GC attempt didn't result in any cleanup")

// ErrRejected is returned if a value log GC is called either while another GC is running, or
// after DB::Close has been called.
ErrRejected = errors.New("Value log GC request rejected")
ErrRejected = stderrors.New("Value log GC request rejected")

// ErrInvalidRequest is returned if the user request is invalid.
ErrInvalidRequest = errors.New("Invalid request")
ErrInvalidRequest = stderrors.New("Invalid request")

// ErrManagedTxn is returned if the user tries to use an API which isn't
// allowed due to external management of transactions, when using ManagedDB.
ErrManagedTxn = errors.New(
ErrManagedTxn = stderrors.New(
"Invalid API request. Not allowed to perform this action using ManagedDB")

// ErrNamespaceMode is returned if the user tries to use an API which is allowed only when
// NamespaceOffset is non-negative.
ErrNamespaceMode = errors.New(
ErrNamespaceMode = stderrors.New(
"Invalid API request. Not allowed to perform this action when NamespaceMode is not set.")

// ErrInvalidDump if a data dump made previously cannot be loaded into the database.
ErrInvalidDump = errors.New("Data dump cannot be read")
ErrInvalidDump = stderrors.New("Data dump cannot be read")

// ErrZeroBandwidth is returned if the user passes in zero bandwidth for sequence.
ErrZeroBandwidth = errors.New("Bandwidth must be greater than zero")
ErrZeroBandwidth = stderrors.New("Bandwidth must be greater than zero")

// ErrWindowsNotSupported is returned when opt.ReadOnly is used on Windows
ErrWindowsNotSupported = errors.New("Read-only mode is not supported on Windows")
ErrWindowsNotSupported = stderrors.New("Read-only mode is not supported on Windows")

// ErrPlan9NotSupported is returned when opt.ReadOnly is used on Plan 9
ErrPlan9NotSupported = errors.New("Read-only mode is not supported on Plan 9")
ErrPlan9NotSupported = stderrors.New("Read-only mode is not supported on Plan 9")

// ErrTruncateNeeded is returned when the value log gets corrupt, and requires truncation of
// corrupt data to allow Badger to run properly.
ErrTruncateNeeded = errors.New(
ErrTruncateNeeded = stderrors.New(
"Log truncate required to run DB. This might result in data loss")

// ErrBlockedWrites is returned if the user called DropAll. During the process of dropping all
// data from Badger, we stop accepting new writes, by returning this error.
ErrBlockedWrites = errors.New("Writes are blocked, possibly due to DropAll or Close")
ErrBlockedWrites = stderrors.New("Writes are blocked, possibly due to DropAll or Close")

// ErrNilCallback is returned when subscriber's callback is nil.
ErrNilCallback = errors.New("Callback cannot be nil")
ErrNilCallback = stderrors.New("Callback cannot be nil")

// ErrEncryptionKeyMismatch is returned when the storage key is not
// matched with the key previously given.
ErrEncryptionKeyMismatch = errors.New("Encryption key mismatch")
ErrEncryptionKeyMismatch = stderrors.New("Encryption key mismatch")

// ErrInvalidDataKeyID is returned if the datakey id is invalid.
ErrInvalidDataKeyID = errors.New("Invalid datakey id")
ErrInvalidDataKeyID = stderrors.New("Invalid datakey id")

// ErrInvalidEncryptionKey is returned if length of encryption keys is invalid.
ErrInvalidEncryptionKey = errors.New("Encryption key's length should be" +
ErrInvalidEncryptionKey = stderrors.New("Encryption key's length should be" +
"either 16, 24, or 32 bytes")
// ErrGCInMemoryMode is returned when db.RunValueLogGC is called in in-memory mode.
ErrGCInMemoryMode = errors.New("Cannot run value log GC when DB is opened in InMemory mode")
ErrGCInMemoryMode = stderrors.New("Cannot run value log GC when DB is opened in InMemory mode")

// ErrDBClosed is returned when a get operation is performed after closing the DB.
ErrDBClosed = errors.New("DB Closed")
ErrDBClosed = stderrors.New("DB Closed")
)
3 changes: 2 additions & 1 deletion levels.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"context"
"encoding/hex"
stderrors "errors"
"fmt"
"math"
"math/rand"
Expand Down Expand Up @@ -1512,7 +1513,7 @@ func tablesToString(tables []*table.Table) []string {
return res
}

var errFillTables = errors.New("Unable to fill tables")
var errFillTables = stderrors.New("Unable to fill tables")

// doCompact picks some table on level l and compacts it away to the next level.
func (s *levelsController) doCompact(id int, p compactionPriority) error {
Expand Down
5 changes: 3 additions & 2 deletions manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bufio"
"bytes"
"encoding/binary"
stderrors "errors"
"fmt"
"hash/crc32"
"io"
Expand Down Expand Up @@ -352,8 +353,8 @@ func (r *countingReader) ReadByte() (b byte, err error) {
}

var (
errBadMagic = errors.New("manifest has bad magic")
errBadChecksum = errors.New("manifest has checksum mismatch")
errBadMagic = stderrors.New("manifest has bad magic")
errBadChecksum = stderrors.New("manifest has checksum mismatch")
)

// ReplayManifestFile reads the manifest file and constructs two manifest objects. (We need one
Expand Down
5 changes: 2 additions & 3 deletions merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,10 @@
package badger

import (
stderrors "errors"
"sync"
"time"

"github.com/pkg/errors"

"github.com/dgraph-io/badger/v4/y"
"github.com/dgraph-io/ristretto/z"
)
Expand Down Expand Up @@ -58,7 +57,7 @@ func (db *DB) GetMergeOperator(key []byte,
return op
}

var errNoMerge = errors.New("No need for merge")
var errNoMerge = stderrors.New("No need for merge")

func (op *MergeOperator) iterateAndMerge() (newVal []byte, latest uint64, err error) {
txn := op.db.NewTransaction(false)
Expand Down
5 changes: 3 additions & 2 deletions value.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package badger
import (
"bytes"
"context"
stderrors "errors"
"fmt"
"hash"
"hash/crc32"
Expand Down Expand Up @@ -63,8 +64,8 @@ const (
vlogHeaderSize = 20
)

var errStop = errors.New("Stop iteration")
var errTruncate = errors.New("Do truncate")
var errStop = stderrors.New("Stop iteration")
var errTruncate = stderrors.New("Do truncate")

type logEntry func(e Entry, vp valuePointer) error

Expand Down
4 changes: 2 additions & 2 deletions y/checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
package y

import (
stderrors "errors"
"hash/crc32"

"github.com/cespare/xxhash/v2"
"github.com/pkg/errors"

"github.com/dgraph-io/badger/v4/pb"
)

// ErrChecksumMismatch is returned at checksum mismatch.
var ErrChecksumMismatch = errors.New("checksum mismatch")
var ErrChecksumMismatch = stderrors.New("checksum mismatch")

// CalculateChecksum calculates checksum for data using ct checksum type.
func CalculateChecksum(data []byte, ct pb.Checksum_Algorithm) uint64 {
Expand Down
7 changes: 3 additions & 4 deletions y/y.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package y
import (
"bytes"
"encoding/binary"
stderrors "errors"
"fmt"
"hash/crc32"
"io"
Expand All @@ -30,20 +31,18 @@ import (
"time"
"unsafe"

"github.com/pkg/errors"

"github.com/dgraph-io/badger/v4/pb"
"github.com/dgraph-io/ristretto/z"
)

var (
// ErrEOF indicates an end of file when trying to read from a memory mapped file
// and encountering the end of slice.
ErrEOF = errors.New("ErrEOF: End of file")
ErrEOF = stderrors.New("ErrEOF: End of file")

// ErrCommitAfterFinish indicates that write batch commit was called after
// finish
ErrCommitAfterFinish = errors.New("Batch commit not permitted after finish")
ErrCommitAfterFinish = stderrors.New("Batch commit not permitted after finish")
)

type Flags int
Expand Down

0 comments on commit de3932e

Please sign in to comment.