From 07f7fab5356ee33c11aeba40c48bef8aae220df1 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Fri, 19 Apr 2019 10:00:14 -0700 Subject: [PATCH 1/2] make the logging of stacks in errors an opt-in behavior Signed-off-by: Michael Demmer --- go/vt/vterrors/vterrors.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/go/vt/vterrors/vterrors.go b/go/vt/vterrors/vterrors.go index c798fa8ffb7..8a7000006c5 100644 --- a/go/vt/vterrors/vterrors.go +++ b/go/vt/vterrors/vterrors.go @@ -70,6 +70,7 @@ package vterrors import ( + "flag" "fmt" "io" @@ -77,6 +78,12 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) +var logErrStacks bool + +func init() { + flag.BoolVar(&logErrStacks, "logerrstacks", false, "log stack traces in errors") +} + // New returns an error with the supplied message. // New also records the stack trace at the point it was called. func New(code vtrpcpb.Code, message string) error { @@ -122,7 +129,9 @@ func (f *fundamental) Format(s fmt.State, verb rune) { case 'v': panicIfError(io.WriteString(s, "Code: "+f.code.String()+"\n")) panicIfError(io.WriteString(s, f.msg+"\n")) - f.stack.Format(s, verb) + if logErrStacks { + f.stack.Format(s, verb) + } return case 's': panicIfError(io.WriteString(s, f.msg)) @@ -198,7 +207,9 @@ func (w *wrapping) Format(s fmt.State, verb rune) { if rune('v') == verb { panicIfError(fmt.Fprintf(s, "%v\n", w.Cause())) panicIfError(io.WriteString(s, w.msg)) - w.stack.Format(s, verb) + if logErrStacks { + w.stack.Format(s, verb) + } return } From 547293004087f9aa74ee4611b66395643a0c90e9 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Fri, 19 Apr 2019 13:58:03 -0700 Subject: [PATCH 2/2] fix tests to handle the opt-in stack logging behavior Signed-off-by: Michael Demmer --- go/vt/vterrors/errors_test.go | 44 ++++++++++++++++++++++------------- go/vt/vterrors/vterrors.go | 10 ++++---- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/go/vt/vterrors/errors_test.go b/go/vt/vterrors/errors_test.go index 772b7d90251..f0498ee3306 100644 --- a/go/vt/vterrors/errors_test.go +++ b/go/vt/vterrors/errors_test.go @@ -172,15 +172,16 @@ func TestStackFormat(t *testing.T) { err := outer() got := fmt.Sprintf("%v", err) - assertStringContains(t, got, "innerMost") - assertStringContains(t, got, "middle") - assertStringContains(t, got, "outer") -} + assertContains(t, got, "innerMost", false) + assertContains(t, got, "middle", false) + assertContains(t, got, "outer", false) -func assertStringContains(t *testing.T, s, substring string) { - if !strings.Contains(s, substring) { - t.Errorf("string did not contain `%v`: \n %v", substring, s) - } + LogErrStacks = true + defer func() { LogErrStacks = false }() + got = fmt.Sprintf("%v", err) + assertContains(t, got, "innerMost", true) + assertContains(t, got, "middle", true) + assertContains(t, got, "outer", true) } // errors.New, etc values are not expected to be compared by value @@ -256,18 +257,29 @@ func TestWrapping(t *testing.T) { err1 := Errorf(vtrpcpb.Code_UNAVAILABLE, "foo") err2 := Wrapf(err1, "bar") err3 := Wrapf(err2, "baz") + errorWithoutStack := fmt.Sprintf("%v", err3) + + LogErrStacks = true errorWithStack := fmt.Sprintf("%v", err3) + LogErrStacks = false assertEquals(t, err3.Error(), "baz: bar: foo") - assertContains(t, errorWithStack, "foo") - assertContains(t, errorWithStack, "bar") - assertContains(t, errorWithStack, "baz") - assertContains(t, errorWithStack, "TestWrapping") + assertContains(t, errorWithoutStack, "foo", true) + assertContains(t, errorWithoutStack, "bar", true) + assertContains(t, errorWithoutStack, "baz", true) + assertContains(t, errorWithoutStack, "TestWrapping", false) + + assertContains(t, errorWithStack, "foo", true) + assertContains(t, errorWithStack, "bar", true) + assertContains(t, errorWithStack, "baz", true) + assertContains(t, errorWithStack, "TestWrapping", true) + } -func assertContains(t *testing.T, s, substring string) { - if !strings.Contains(s, substring) { - t.Fatalf("expected string that contains [%s] but got [%s]", substring, s) +func assertContains(t *testing.T, s, substring string, contains bool) { + t.Helper() + if doesContain := strings.Contains(s, substring); doesContain != contains { + t.Errorf("string `%v` contains `%v`: %v, want %v", s, substring, doesContain, contains) } } @@ -275,4 +287,4 @@ func assertEquals(t *testing.T, a, b interface{}) { if a != b { t.Fatalf("expected [%s] to be equal to [%s]", a, b) } -} \ No newline at end of file +} diff --git a/go/vt/vterrors/vterrors.go b/go/vt/vterrors/vterrors.go index 8a7000006c5..95af7967544 100644 --- a/go/vt/vterrors/vterrors.go +++ b/go/vt/vterrors/vterrors.go @@ -78,10 +78,12 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) -var logErrStacks bool +// LogErrStacks controls whether or not printing errors includes the +// embedded stack trace in the output. +var LogErrStacks bool func init() { - flag.BoolVar(&logErrStacks, "logerrstacks", false, "log stack traces in errors") + flag.BoolVar(&LogErrStacks, "LogErrStacks", false, "log stack traces in errors") } // New returns an error with the supplied message. @@ -129,7 +131,7 @@ func (f *fundamental) Format(s fmt.State, verb rune) { case 'v': panicIfError(io.WriteString(s, "Code: "+f.code.String()+"\n")) panicIfError(io.WriteString(s, f.msg+"\n")) - if logErrStacks { + if LogErrStacks { f.stack.Format(s, verb) } return @@ -207,7 +209,7 @@ func (w *wrapping) Format(s fmt.State, verb rune) { if rune('v') == verb { panicIfError(fmt.Fprintf(s, "%v\n", w.Cause())) panicIfError(io.WriteString(s, w.msg)) - if logErrStacks { + if LogErrStacks { w.stack.Format(s, verb) } return