Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make the logging of stacks in errors an opt-in behavior #4826

Merged
merged 2 commits into from
Apr 19, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions go/vt/vterrors/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -256,23 +257,34 @@ 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)
}
}

func assertEquals(t *testing.T, a, b interface{}) {
if a != b {
t.Fatalf("expected [%s] to be equal to [%s]", a, b)
}
}
}
17 changes: 15 additions & 2 deletions go/vt/vterrors/vterrors.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,22 @@
package vterrors

import (
"flag"
"fmt"
"io"

"golang.org/x/net/context"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"
)

// 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")
}

// 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 {
Expand Down Expand Up @@ -122,7 +131,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))
Expand Down Expand Up @@ -198,7 +209,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
}

Expand Down