Skip to content

Commit

Permalink
replace a panic with an error in merge-logs
Browse files Browse the repository at this point in the history
Release justification: bug fix

Release note (bug fix): Users would receive a panic message when the log parser
fails to extract log file formats. This has been replaced with a helpful
error message.
  • Loading branch information
cameronnunez committed Sep 2, 2021
1 parent a46e3a2 commit a5caea6
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
5 changes: 2 additions & 3 deletions pkg/cli/debug_merge_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,8 @@ func (s *fileLogStream) open() bool {
if s.err = seekToFirstAfterFrom(s.f, s.from, s.editMode, s.format); s.err != nil {
return false
}
var err error
if s.d, err = log.NewEntryDecoderWithFormat(bufio.NewReaderSize(s.f, readBufSize), s.editMode, s.format); err != nil {
panic(err)
if s.d, s.err = log.NewEntryDecoderWithFormat(bufio.NewReaderSize(s.f, readBufSize), s.editMode, s.format); s.err != nil {
return false
}
return true
}
Expand Down
28 changes: 25 additions & 3 deletions pkg/cli/debug_merge_logs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ package cli

import (
"bytes"
"fmt"
"io/ioutil"
"path/filepath"
"testing"
Expand Down Expand Up @@ -184,14 +185,22 @@ func getCases(format string) []testCase {
}
}

func (c testCase) run(t *testing.T) {
outBuf := bytes.Buffer{}
func resetDebugMergeLogFlags(errorFn func(s string)) {
debugMergeLogsCmd.Flags().Visit(func(f *pflag.Flag) {
if err := f.Value.Set(f.DefValue); err != nil {
t.Fatalf("Failed to set flag to default: %v", err)
errorFn(fmt.Sprintf("Failed to set flag to default: %v", err))
}
})
}

func (c testCase) run(t *testing.T) {
resetDebugMergeLogFlags(func(s string) { t.Fatal(s) })
outBuf := bytes.Buffer{}
debugMergeLogsCmd.SetOut(&outBuf)
// Ensure that the original writer is restored when the test
// completes. Otherwise, subsequent tests may not see their output.
defer debugMergeLogsCmd.SetOut(nil)

if err := debugMergeLogsCmd.ParseFlags(c.flags); err != nil {
t.Fatalf("Failed to set flags: %v", err)
}
Expand Down Expand Up @@ -235,3 +244,16 @@ func TestCrdbV1DebugMergeLogs(t *testing.T) {
t.Run(c.name, c.run)
}
}

func Example_format_error() {
c := NewCLITest(TestCLIParams{NoServer: true})
defer c.Cleanup()

resetDebugMergeLogFlags(func(s string) { fmt.Fprintf(stderr, "ERROR: %v", s) })

c.RunWithArgs([]string{"debug", "merge-logs", "testdata/merge_logs_v1/missing_format/*"})

// Output:
// debug merge-logs testdata/merge_logs_v1/missing_format/*
// ERROR: decoding format: failed to extract log file format from the log
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70 [config] binary: CockroachDB CCL v20.1.17 (x86_64-apple-darwin14, built 2021/05/17 16:30:22,
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70 [config] arguments: [./cockroach start]
I210801 21:05:59.364923 1 util/log/sync_buffer.go:70 line format: [IWEF]yymmdd hh:mm:ss.uuuuuu goid file:line msg utf8=✓

0 comments on commit a5caea6

Please sign in to comment.