Skip to content

Commit 0cd16f0

Browse files
*: use errors.AssertionFailedf for panic format strings across the codebase
Previously, panic messages containing runtime values were being redacted when surfaced in CockroachDB logs, making it impossible to diagnose issues from crash reports. Replace all panic(fmt.Sprintf(...)) calls with panic(errors.AssertionFailedf(...)) so the panic value is a proper structured error with stack traces. errors.AssertionFailedf treats arguments as unsafe by default, so user key data is still redacted in CockroachDB logs. Additionally, wrap safe diagnostic arguments (counts, sizes, indices, reference counts, levels, etc.) with errors.Safe() so they appear in redacted crash reports. Implement SafeFormatter on several types (CompactionState, OpType, OpKind, interleavePos, bitmapEncoding, compactionKind, objID) so their string representations are visible in redacted output. User key data arguments are intentionally left unwrapped to remain redacted. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
1 parent 85c3ab5 commit 0cd16f0

File tree

127 files changed

+468
-359
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

127 files changed

+468
-359
lines changed

batch.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"bytes"
99
"context"
1010
"encoding/binary"
11-
"fmt"
1211
"io"
1312
"math"
1413
"sort"
@@ -2080,7 +2079,7 @@ func newFlushableBatch(batch *Batch, comparer *Comparer) (*flushableBatch, error
20802079

20812080
func (b *flushableBatch) setSeqNum(seqNum base.SeqNum) {
20822081
if b.seqNum != 0 {
2083-
panic(fmt.Sprintf("pebble: flushableBatch.seqNum already set: %d", b.seqNum))
2082+
panic(errors.AssertionFailedf("pebble: flushableBatch.seqNum already set: %d", b.seqNum))
20842083
}
20852084
b.seqNum = seqNum
20862085
for i := range b.tombstones {

checkpoint_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
"github.com/cockroachdb/crlib/testutils/leaktest"
2020
"github.com/cockroachdb/datadriven"
21+
"github.com/cockroachdb/errors"
2122
"github.com/cockroachdb/pebble/internal/base"
2223
"github.com/cockroachdb/pebble/internal/testutils"
2324
"github.com/cockroachdb/pebble/objstorage/remote"
@@ -271,7 +272,7 @@ func TestCopyCheckpointOptions(t *testing.T) {
271272
require.NoError(t, f.Close())
272273
return string(newFile)
273274
default:
274-
panic(fmt.Sprintf("unrecognized command %q", td.Cmd))
275+
panic(errors.AssertionFailedf("unrecognized command %q", td.Cmd))
275276
}
276277
})
277278
}

cmd/pebble/test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"time"
1919

2020
"github.com/HdrHistogram/hdrhistogram-go"
21+
"github.com/cockroachdb/errors"
2122
"github.com/cockroachdb/pebble"
2223
)
2324

@@ -141,7 +142,7 @@ func (w *namedHistogram) Record(elapsed time.Duration) {
141142
// Note that a histogram only drops recorded values that are out of range,
142143
// but we clamp the latency value to the configured range to prevent such
143144
// drops. This code path should never happen.
144-
panic(fmt.Sprintf(`%s: recording value: %s`, w.name, err))
145+
panic(errors.AssertionFailedf(`%s: recording value: %s`, errors.Safe(w.name), err))
145146
}
146147
}
147148

cockroachkvs/cockroachkvs.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ func normalizeVersionForCompare(a []byte) []byte {
476476
func normalizeEngineSuffixForCompare(a []byte) []byte {
477477
// Check sentinel byte.
478478
if invariants.Enabled && len(a) != int(a[len(a)-1]) {
479-
panic(errors.AssertionFailedf("malformed suffix: %x (length byte is %d; but suffix is %d bytes)", a, a[len(a)-1], len(a)))
479+
panic(errors.AssertionFailedf("malformed suffix: %x (length byte is %d; but suffix is %d bytes)", errors.Safe(a), errors.Safe(a[len(a)-1]), errors.Safe(len(a))))
480480
}
481481
// Strip off sentinel byte.
482482
a = a[:len(a)-1]
@@ -659,7 +659,7 @@ func (kw *cockroachKeyWriter) WriteKey(
659659
versionLen := int(key[len(key)-1])
660660
if (len(key)-versionLen) != int(keyPrefixLen) || key[keyPrefixLen-1] != 0x00 {
661661
panic(errors.AssertionFailedf("invalid %d-byte key with %d-byte prefix (%q)",
662-
len(key), keyPrefixLen, key))
662+
errors.Safe(len(key)), errors.Safe(keyPrefixLen), key))
663663
}
664664
// TODO(jackson): Avoid copying the previous suffix.
665665
kw.prevSuffix = append(kw.prevSuffix[:0], key[keyPrefixLen:]...)
@@ -754,7 +754,7 @@ func (kw *cockroachKeyWriter) Finish(
754754
case cockroachColUntypedVersion:
755755
return kw.untypedVersions.Finish(0, rows, offset, buf)
756756
default:
757-
panic(fmt.Sprintf("unknown default key column: %d", col))
757+
panic(errors.AssertionFailedf("unknown default key column: %d", errors.Safe(col)))
758758
}
759759
}
760760

@@ -793,7 +793,7 @@ func (ks *cockroachKeySeeker) init(
793793
header := bd.Header()
794794
header = header[:len(header)-colblk.DataBlockCustomHeaderSize]
795795
if len(header) != 1 {
796-
panic(errors.AssertionFailedf("invalid key schema-specific header %x", header))
796+
panic(errors.AssertionFailedf("invalid key schema-specific header %x", errors.Safe(header)))
797797
}
798798
ks.suffixTypes = suffixTypes(header[0])
799799
}
@@ -1003,7 +1003,7 @@ func (ks *cockroachKeySeeker) MaterializeUserKey(
10031003
ki *colblk.PrefixBytesIter, prevRow, row int,
10041004
) []byte {
10051005
if invariants.Enabled && (row < 0 || row >= ks.roachKeys.Rows()) {
1006-
panic(errors.AssertionFailedf("invalid row number %d", row))
1006+
panic(errors.AssertionFailedf("invalid row number %d", errors.Safe(row)))
10071007
}
10081008
if prevRow+1 == row && prevRow >= 0 {
10091009
ks.roachKeys.SetNext(ki)
@@ -1148,7 +1148,7 @@ func (fk formatKeySuffix) Format(f fmt.State, c rune) {
11481148
fmt.Fprintf(f, "%d.%09d,%d", wallTime/1e9, wallTime%1e9, logical)
11491149
}
11501150
default:
1151-
panic(errors.AssertionFailedf("invalid suffix length: %d", len(fk)))
1151+
panic(errors.AssertionFailedf("invalid suffix length: %d", errors.Safe(len(fk))))
11521152
}
11531153
}
11541154

cockroachkvs/cockroachkvs_test.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/cockroachdb/crlib/testutils/leaktest"
2121
"github.com/cockroachdb/crlib/testutils/require"
2222
"github.com/cockroachdb/datadriven"
23+
"github.com/cockroachdb/errors"
2324
"github.com/cockroachdb/pebble"
2425
"github.com/cockroachdb/pebble/internal/base"
2526
"github.com/cockroachdb/pebble/internal/testutils"
@@ -148,7 +149,7 @@ func TestComparerFuncs(t *testing.T) {
148149
}
149150
return buf.String()
150151
default:
151-
panic(fmt.Sprintf("unknown command: %s", td.Cmd))
152+
panic(errors.AssertionFailedf("unknown command: %s", td.Cmd))
152153
}
153154
})
154155
}
@@ -226,7 +227,7 @@ func TestKeySchema_KeyWriter(t *testing.T) {
226227
tbl.Render()
227228
return buf.String()
228229
default:
229-
panic(fmt.Sprintf("unrecognized command %q", td.Cmd))
230+
panic(errors.AssertionFailedf("unrecognized command %q", td.Cmd))
230231
}
231232
})
232233
}
@@ -322,7 +323,7 @@ func TestKeySchema_KeySeeker(t *testing.T) {
322323
}
323324
return buf.String()
324325
default:
325-
panic(fmt.Sprintf("unrecognized command %q", td.Cmd))
326+
panic(errors.AssertionFailedf("unrecognized command %q", td.Cmd))
326327
}
327328
})
328329
}
@@ -638,7 +639,7 @@ func parseUserKey(userKeyStr string) []byte {
638639
var err error
639640
roachKey, err = strconv.Unquote(roachKey)
640641
if err != nil {
641-
panic(fmt.Sprintf("invalid user key string %s: %v", userKeyStr, err))
642+
panic(errors.AssertionFailedf("invalid user key string %s: %v", userKeyStr, err))
642643
}
643644
}
644645
k := append(append([]byte(roachKey), 0), parseVersion(versionStr)...)
@@ -664,7 +665,7 @@ func parseVersion(versionStr string) []byte {
664665
}
665666
ret := AppendTimestamp([]byte{0x00}, wallTime, uint32(logicalTime))
666667
if len(ret) != 14 && len(ret) != 10 {
667-
panic(fmt.Sprintf("expected 10 or 14-length ret got %d", len(ret)))
668+
panic(errors.AssertionFailedf("expected 10 or 14-length ret got %d", len(ret)))
668669
}
669670
validateEngineKey.MustValidate(ret)
670671
// TODO(jackson): Refactor to allow us to generate a suffix without a
@@ -675,7 +676,7 @@ func parseVersion(versionStr string) []byte {
675676
// Parse as a hex-encoded version.
676677
var version []byte
677678
if _, err := fmt.Sscanf(versionStr, "%X", &version); err != nil {
678-
panic(fmt.Sprintf("invalid version string %q", versionStr))
679+
panic(errors.AssertionFailedf("invalid version string %q", versionStr))
679680
}
680681
return append(version, byte(len(version)+1))
681682
}

compaction.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@ func (k compactionKind) String() string {
188188
return "?"
189189
}
190190

191+
func (k compactionKind) SafeFormat(w redact.SafePrinter, _ rune) {
192+
w.Print(redact.SafeString(k.String()))
193+
}
194+
191195
// compactingOrFlushing returns "flushing" if the compaction kind is a flush,
192196
// otherwise it returns "compacting".
193197
func (k compactionKind) compactingOrFlushing() string {
@@ -1690,9 +1694,9 @@ func (d *DB) flush1() (bytesFlushed uint64, err error) {
16901694
for i := 0; i < n; i++ {
16911695
if logNum := d.mu.mem.queue[i].logNum; logNum >= minUnflushedLogNum {
16921696
panic(errors.AssertionFailedf("logNum invariant violated: flushing %d items; %d:type=%T,logNum=%d; %d:type=%T,logNum=%d",
1693-
n,
1694-
i, d.mu.mem.queue[i].flushable, logNum,
1695-
n, d.mu.mem.queue[n].flushable, minUnflushedLogNum))
1697+
errors.Safe(n),
1698+
errors.Safe(i), errors.Safe(d.mu.mem.queue[i].flushable), logNum,
1699+
errors.Safe(n), errors.Safe(d.mu.mem.queue[n].flushable), minUnflushedLogNum))
16961700
}
16971701
}
16981702
}
@@ -1837,7 +1841,7 @@ func (d *DB) flush1() (bytesFlushed uint64, err error) {
18371841
d.clearCompactingState(c, err != nil)
18381842
if c.UsesBurstConcurrency() {
18391843
if v := d.mu.compact.burstConcurrency.Add(-1); v < 0 {
1840-
panic(errors.AssertionFailedf("burst concurrency underflow: %d", v))
1844+
panic(errors.AssertionFailedf("burst concurrency underflow: %d", errors.Safe(v)))
18411845
}
18421846
}
18431847
delete(d.mu.compact.inProgress, c)
@@ -2101,7 +2105,7 @@ func (d *DB) runPickedCompaction(pc pickedCompaction, grantHandle CompactionGran
21012105
}
21022106
}
21032107
if doneChannel == nil {
2104-
panic(errors.AssertionFailedf("did not find manual compaction with id %d", pc.ManualID()))
2108+
panic(errors.AssertionFailedf("did not find manual compaction with id %d", errors.Safe(pc.ManualID())))
21052109
}
21062110
}
21072111

@@ -2267,7 +2271,7 @@ func (d *DB) compact(c compaction, errChannel chan error) {
22672271
}
22682272
if c.UsesBurstConcurrency() {
22692273
if v := d.mu.compact.burstConcurrency.Add(-1); v < 0 {
2270-
panic(errors.AssertionFailedf("burst concurrency underflow: %d", v))
2274+
panic(errors.AssertionFailedf("burst concurrency underflow: %d", errors.Safe(v)))
22712275
}
22722276
}
22732277
delete(d.mu.compact.inProgress, c)

compaction_picker.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,8 @@ func newPickedTableCompaction(
281281
panic("base level cannot be 0")
282282
}
283283
if startLevel > 0 && startLevel < baseLevel {
284-
panic(fmt.Sprintf("invalid compaction: start level %d should not be empty (base level %d)",
285-
startLevel, baseLevel))
284+
panic(errors.AssertionFailedf("invalid compaction: start level %d should not be empty (base level %d)",
285+
errors.Safe(startLevel), errors.Safe(baseLevel)))
286286
}
287287

288288
pc := &pickedTableCompaction{
@@ -514,7 +514,7 @@ func (pc *pickedTableCompaction) maybeGrow(
514514
func (pc *pickedTableCompaction) maybeGrowL0ForBase(cmp base.Compare, maxExpandedBytes uint64) {
515515
if invariants.Enabled {
516516
if pc.startLevel.level != 0 {
517-
panic(fmt.Sprintf("pc.startLevel.level is %d, expected 0", pc.startLevel.level))
517+
panic(errors.AssertionFailedf("pc.startLevel.level is %d, expected 0", errors.Safe(pc.startLevel.level)))
518518
}
519519
}
520520

@@ -1664,7 +1664,7 @@ func (p *compactionPickerByScore) pickedCompactionFromCandidateFile(
16641664
}
16651665
}
16661666
if !found {
1667-
panic(fmt.Sprintf("file %s not found in level %d as expected", candidate.TableNum, startLevel))
1667+
panic(errors.AssertionFailedf("file %s not found in level %d as expected", candidate.TableNum, errors.Safe(startLevel)))
16681668
}
16691669
}
16701670

compaction_picker_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1638,7 +1638,7 @@ func TestCompactionPickerScores(t *testing.T) {
16381638
}
16391639
}()
16401640
default:
1641-
panic(fmt.Sprintf("unrecognized `wait-for-compaction` value: %q", waitFor))
1641+
panic(errors.AssertionFailedf("unrecognized `wait-for-compaction` value: %q", waitFor))
16421642
}
16431643

16441644
buf.Reset()

data_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ func printIterState(
371371
if iter.Valid() {
372372
hasPoint, hasRange := iter.HasPointAndRange()
373373
if hasPoint || !hasRange {
374-
panic(fmt.Sprintf("pebble: unexpected HasPointAndRange (%t, %t)", hasPoint, hasRange))
374+
panic(errors.AssertionFailedf("pebble: unexpected HasPointAndRange (%t, %t)", hasPoint, hasRange))
375375
}
376376
start, end := iter.RangeBounds()
377377
fmt.Fprintf(b, "%s [%s-%s)", iter.Key(), formatASCIIKey(start), formatASCIIKey(end))

db.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package pebble // import "github.com/cockroachdb/pebble"
77

88
import (
99
"context"
10-
"fmt"
1110
"io"
1211
"reflect"
1312
"slices"
@@ -796,7 +795,7 @@ func (d *DB) applyInternal(batch *Batch, opts *WriteOptions, noSyncWait bool) er
796795
return ErrReadOnly
797796
}
798797
if batch.db != nil && batch.db != d {
799-
panic(fmt.Sprintf("pebble: batch db mismatch: %p != %p", batch.db, d))
798+
panic(errors.AssertionFailedf("pebble: batch db mismatch: %p != %p", errors.Safe(batch.db), errors.Safe(d)))
800799
}
801800

802801
sync := opts.GetSync()
@@ -805,7 +804,7 @@ func (d *DB) applyInternal(batch *Batch, opts *WriteOptions, noSyncWait bool) er
805804
}
806805

807806
if fmv := d.FormatMajorVersion(); fmv < batch.minimumFormatMajorVersion {
808-
panic(fmt.Sprintf(
807+
panic(errors.AssertionFailedf(
809808
"pebble: batch requires at least format major version %d (current: %d)",
810809
batch.minimumFormatMajorVersion, fmv,
811810
))
@@ -995,7 +994,7 @@ func assertZeroed(v reflect.Value) error {
995994
return assertZeroed(v.Elem())
996995
case reflect.Slice:
997996
if v.Len() > 0 {
998-
return errors.AssertionFailedf("%s is not zeroed (%d len): %#v", typ.Name(), v.Len(), v)
997+
return errors.AssertionFailedf("%s is not zeroed (%d len): %#v", errors.Safe(typ.Name()), errors.Safe(v.Len()), v)
999998
}
1000999
resliced := v.Slice(0, v.Cap())
10011000
for i := 0; i < resliced.Len(); i++ {
@@ -1015,7 +1014,7 @@ func assertZeroed(v reflect.Value) error {
10151014
}
10161015
return nil
10171016
}
1018-
return errors.AssertionFailedf("%s (%s) is not zeroed: %#v", typ.Name(), typ.Kind(), v)
1017+
return errors.AssertionFailedf("%s (%s) is not zeroed: %#v", errors.Safe(typ.Name()), typ.Kind(), errors.Safe(v))
10191018
}
10201019

10211020
func newIterAlloc() *iterAlloc {

0 commit comments

Comments
 (0)