cmd/stcrashreceiver, lib/db: Improve panic message handling (#7029)

cmd/stcrashreceiver/main.go

@@ -88,6 +88,7 @@ func handleFailureFn(dsn string) func(w http.ResponseWriter, req *http.Request)
 			pkt.Extra = raven.Extra{
 				"count": r.Count,
 			}
+			pkt.Fingerprint = []string{r.Description}
 
 			if err := sendReport(dsn, pkt, userIDFor(req)); err != nil {
 				log.Println("Failed to send  crash report:", err)

cmd/stcrashreceiver/sentry.go

@@ -128,10 +128,44 @@ func parseCrashReport(path string, report []byte) (*raven.Packet, error) {
 		"url": reportServer + path,
 	}
 	pkt.Interfaces = []raven.Interface{&trace}
+	pkt.Fingerprint = crashReportFingerprint(pkt.Message)
 
 	return pkt, nil
 }
 
+var (
+	indexRe          = regexp.MustCompile(`\[[-:0-9]+\]`)
+	sizeRe           = regexp.MustCompile(`(length|capacity) [0-9]+`)
+	ldbPosRe         = regexp.MustCompile(`(\(pos=)([0-9]+)\)`)
+	ldbChecksumRe    = regexp.MustCompile(`(want=0x)([a-z0-9]+)( got=0x)([a-z0-9]+)`)
+	ldbFileRe        = regexp.MustCompile(`(\[file=)([0-9]+)(\.ldb\])`)
+	ldbInternalKeyRe = regexp.MustCompile(`(internal key ")[^"]+(", len=)[0-9]+`)
+	ldbPathRe        = regexp.MustCompile(`(open|write|read) .+[\\/].+[\\/]index[^\\/]+[\\/][^\\/]+: `)
+)
+
+func crashReportFingerprint(message string) []string {
+	// Do not fingerprint on the stack in case of db corruption or fatal
+	// db io error - where it occurs doesn't matter.
+	orig := message
+	message = ldbPosRe.ReplaceAllString(message, "${1}x)")
+	message = ldbFileRe.ReplaceAllString(message, "${1}x${3}")
+	message = ldbChecksumRe.ReplaceAllString(message, "${1}X${3}X")
+	message = ldbInternalKeyRe.ReplaceAllString(message, "${1}x${2}x")
+	message = ldbPathRe.ReplaceAllString(message, "$1 x: ")
+	if message != orig {
+		return []string{message}
+	}
+
+	message = indexRe.ReplaceAllString(message, "[x]")
+	message = sizeRe.ReplaceAllString(message, "$1 x")
+
+	// {{ default }} is what sentry uses as a fingerprint by default. While
+	// never specified, the docs point at this being some hash derived from the
+	// stack trace. Here we include the filtered panic message on top of that.
+	// https://docs.sentry.io/platforms/go/data-management/event-grouping/sdk-fingerprinting/#basic-example
+	return []string{"{{ default }}", message}
+}
+
 // syncthing v1.1.4-rc.1+30-g6aaae618-dirty-crashrep "Erbium Earthworm" (go1.12.5 darwin-amd64) [email protected] 2019-05-23 16:08:14 UTC [foo, bar]
 var longVersionRE = regexp.MustCompile(`syncthing\s+(v[^\s]+)\s+"([^"]+)"\s\(([^\s]+)\s+([^-]+)-([^)]+)\)\s+([^\s]+)[^\[]*(?:\[(.+)\])?$`)
 

cmd/stcrashreceiver/sentry_test.go

@@ -76,3 +76,66 @@ func TestParseReport(t *testing.T) {
 
 	fmt.Printf("%s\n", bs)
 }
+
+func TestCrashReportFingerprint(t *testing.T) {
+	cases := []struct {
+		message, exp string
+		ldb          bool
+	}{
+		{
+			message: "panic: leveldb/table: corruption on data-block (pos=51308946): checksum mismatch, want=0xa89f9aa0 got=0xd27cc4c7 [file=004003.ldb]",
+			exp:     "panic: leveldb/table: corruption on data-block (pos=x): checksum mismatch, want=0xX got=0xX [file=x.ldb]",
+			ldb:     true,
+		},
+		{
+			message: "panic: leveldb/table: corruption on table-footer (pos=248): bad magic number [file=001370.ldb]",
+			exp:     "panic: leveldb/table: corruption on table-footer (pos=x): bad magic number [file=x.ldb]",
+			ldb:     true,
+		},
+		{
+			message: "panic: runtime error: slice bounds out of range [4294967283:4194304]",
+			exp:     "panic: runtime error: slice bounds out of range [x]",
+		},
+		{
+			message: "panic: runtime error: slice bounds out of range [-2:]",
+			exp:     "panic: runtime error: slice bounds out of range [x]",
+		},
+		{
+			message: "panic: runtime error: slice bounds out of range [:4294967283] with capacity 32768",
+			exp:     "panic: runtime error: slice bounds out of range [x] with capacity x",
+		},
+		{
+			message: "panic: runtime error: index out of range [0] with length 0",
+			exp:     "panic: runtime error: index out of range [x] with length x",
+		},
+		{
+			message: `panic: leveldb: internal key "\x01", len=1: invalid length`,
+			exp:     `panic: leveldb: internal key "x", len=x: invalid length`,
+			ldb:     true,
+		},
+		{
+			message: `panic: write /var/syncthing/config/index-v0.14.0.db/2732813.log: cannot allocate memory`,
+			exp:     `panic: write x: cannot allocate memory`,
+			ldb:     true,
+		},
+		{
+			message: `panic: filling Blocks: read C:\Users\Serv-Resp-Tizayuca\AppData\Local\Syncthing\index-v0.14.0.db\006561.ldb: Error de datos (comprobación de redundancia cíclica).`,
+			exp:     `panic: filling Blocks: read x: Error de datos (comprobación de redundancia cíclica).`,
+			ldb:     true,
+		},
+	}
+
+	for i, tc := range cases {
+		fingerprint := crashReportFingerprint(tc.message)
+
+		expLen := 2
+		if tc.ldb {
+			expLen = 1
+		}
+		if l := len(fingerprint); l != expLen {
+			t.Errorf("tc %v: Unexpected fingerprint length: %v != %v", i, l, expLen)
+		} else if msg := fingerprint[expLen-1]; msg != tc.exp {
+			t.Errorf("tc %v:\n\"%v\" !=\n\"%v\"", i, msg, tc.exp)
+		}
+	}
+}

lib/db/lowlevel.go

@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"time"
 
 	"github.com/dchest/siphash"
@@ -816,7 +817,7 @@ func (db *Lowlevel) getMetaAndCheck(folder string) *metadataTracker {
 	var err error
 	defer func() {
 		if err != nil && !backend.IsClosed(err) {
-			panic(err)
+			warnAndPanic(err)
 		}
 	}()
 
@@ -944,14 +945,14 @@ func (db *Lowlevel) verifyLocalSequence(curSeq int64, folder string) bool {
 
 	t, err := db.newReadOnlyTransaction()
 	if err != nil {
-		panic(err)
+		warnAndPanic(err)
 	}
 	ok := true
 	if err := t.withHaveSequence([]byte(folder), curSeq+1, func(fi protocol.FileIntf) bool {
 		ok = false // we got something, which we should not have
 		return false
 	}); err != nil && !backend.IsClosed(err) {
-		panic(err)
+		warnAndPanic(err)
 	}
 	t.close()
 
@@ -1161,3 +1162,11 @@ func (db *Lowlevel) needsRepairPath() string {
 func unchanged(nf, ef protocol.FileIntf) bool {
 	return ef.FileVersion().Equal(nf.FileVersion()) && ef.IsInvalid() == nf.IsInvalid() && ef.FileLocalFlags() == nf.FileLocalFlags()
 }
+
+var ldbPathRe = regexp.MustCompile(`(open|write|read) .+[\\/].+[\\/]index[^\\/]+[\\/][^\\/]+: `)
+
+func warnAndPanic(err error) {
+	l.Warnf("Fatal error: %v", err)
+	msg := ldbPathRe.ReplaceAllString(err.Error(), "$1 x: ")
+	panic(msg)
+}

lib/db/set.go

@@ -531,6 +531,5 @@ func fatalError(err error, opStr string, db *Lowlevel) {
 			}
 		}
 	}
-	l.Warnf("Fatal error: %v: %v", opStr, err)
-	panic(err)
+	warnAndPanic(fmt.Errorf("%v: %w:", opStr, err))
 }