Merge pull request #81 from pkg/withMessage-withStack

Refactor withMessage/withStack

Author: davecheney

errors.go

@@ -91,68 +91,60 @@ import (
 	"io"
 )
 
-// _error is an error implementation returned by New and Errorf
-// that implements its own fmt.Formatter.
-type _error struct {
-	msg string
-	*stack
-}
-
-func (e _error) Error() string { return e.msg }
-
-func (e _error) Format(s fmt.State, verb rune) {
-	switch verb {
-	case 'v':
-		if s.Flag('+') {
-			io.WriteString(s, e.msg)
-			fmt.Fprintf(s, "%+v", e.StackTrace())
-			return
-		}
-		fallthrough
-	case 's':
-		io.WriteString(s, e.msg)
-	}
-}
-
 // New returns an error with the supplied message.
+// New also records the stack trace at the point it was called.
 func New(message string) error {
-	return _error{
-		message,
-		callers(),
+	return &fundamental{
+		msg:   message,
+		stack: callers(),
 	}
 }
 
 // Errorf formats according to a format specifier and returns the string
 // as a value that satisfies error.
+// Errorf also records the stack trace at the point it was called.
 func Errorf(format string, args ...interface{}) error {
-	return _error{
-		fmt.Sprintf(format, args...),
-		callers(),
+	return &fundamental{
+		msg:   fmt.Sprintf(format, args...),
+		stack: callers(),
 	}
 }
 
-type cause struct {
-	cause error
-	msg   string
+// fundamental is an error that has a message and a stack, but no caller.
+type fundamental struct {
+	msg string
+	*stack
 }
 
-func (c cause) Error() string { return fmt.Sprintf("%s: %v", c.msg, c.Cause()) }
-func (c cause) Cause() error  { return c.cause }
+func (f *fundamental) Error() string { return f.msg }
+
+func (f *fundamental) Format(s fmt.State, verb rune) {
+	switch verb {
+	case 'v':
+		if s.Flag('+') {
+			io.WriteString(s, f.msg)
+			f.stack.Format(s, verb)
+			return
+		}
+		fallthrough
+	case 's', 'q':
+		io.WriteString(s, f.msg)
+	}
+}
 
-// wrapper is an error implementation returned by Wrap and Wrapf
-// that implements its own fmt.Formatter.
-type wrapper struct {
-	cause
+type withStack struct {
+	error
 	*stack
 }
 
-func (w wrapper) Format(s fmt.State, verb rune) {
+func (w *withStack) Cause() error { return w.error }
+
+func (w *withStack) Format(s fmt.State, verb rune) {
 	switch verb {
 	case 'v':
 		if s.Flag('+') {
-			fmt.Fprintf(s, "%+v\n", w.Cause())
-			io.WriteString(s, w.msg)
-			fmt.Fprintf(s, "%+v", w.StackTrace())
+			fmt.Fprintf(s, "%+v", w.Cause())
+			w.stack.Format(s, verb)
 			return
 		}
 		fallthrough
@@ -165,31 +157,61 @@ func (w wrapper) Format(s fmt.State, verb rune) {
 
 // Wrap returns an error annotating err with message.
 // If err is nil, Wrap returns nil.
+// Wrap is conceptually the same as calling
+//
+//     errors.WithStack(errors.WithMessage(err, msg))
 func Wrap(err error, message string) error {
 	if err == nil {
 		return nil
 	}
-	return wrapper{
-		cause: cause{
-			cause: err,
-			msg:   message,
-		},
-		stack: callers(),
+	err = &withMessage{
+		cause: err,
+		msg:   message,
+	}
+	return &withStack{
+		err,
+		callers(),
 	}
 }
 
 // Wrapf returns an error annotating err with the format specifier.
 // If err is nil, Wrapf returns nil.
+// Wrapf is conceptually the same as calling
+//
+//     errors.WithStack(errors.WithMessage(err, format, args...))
 func Wrapf(err error, format string, args ...interface{}) error {
 	if err == nil {
 		return nil
 	}
-	return wrapper{
-		cause: cause{
-			cause: err,
-			msg:   fmt.Sprintf(format, args...),
-		},
-		stack: callers(),
+	err = &withMessage{
+		cause: err,
+		msg:   fmt.Sprintf(format, args...),
+	}
+	return &withStack{
+		err,
+		callers(),
+	}
+}
+
+type withMessage struct {
+	cause error
+	msg   string
+}
+
+func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() }
+func (w *withMessage) Cause() error  { return w.cause }
+
+func (w *withMessage) Format(s fmt.State, verb rune) {
+	switch verb {
+	case 'v':
+		if s.Flag('+') {
+			fmt.Fprintf(s, "%+v\n", w.Cause())
+			io.WriteString(s, w.msg)
+			return
+		}
+		fallthrough
+	case 's', 'q':
+		io.WriteString(s, w.Error())
 	}
 }
 

format_test.go

@@ -29,8 +29,8 @@ func TestFormatNew(t *testing.T) {
 			"\t.+/github.com/pkg/errors/format_test.go:25",
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.error, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.error, tt.format, tt.want)
 	}
 }
 
@@ -55,8 +55,8 @@ func TestFormatErrorf(t *testing.T) {
 			"\t.+/github.com/pkg/errors/format_test.go:51",
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.error, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.error, tt.format, tt.want)
 	}
 }
 
@@ -83,14 +83,32 @@ func TestFormatWrap(t *testing.T) {
 		Wrap(io.EOF, "error"),
 		"%s",
 		"error: EOF",
+	}, {
+		Wrap(io.EOF, "error"),
+		"%v",
+		"error: EOF",
+	}, {
+		Wrap(io.EOF, "error"),
+		"%+v",
+		"EOF\n" +
+			"error\n" +
+			"github.com/pkg/errors.TestFormatWrap\n" +
+			"\t.+/github.com/pkg/errors/format_test.go:91",
+	}, {
+		Wrap(Wrap(io.EOF, "error1"), "error2"),
+		"%+v",
+		"EOF\n" +
+			"error1\n" +
+			"github.com/pkg/errors.TestFormatWrap\n" +
+			"\t.+/github.com/pkg/errors/format_test.go:98\n",
 	}, {
 		Wrap(New("error with space"), "context"),
 		"%q",
 		`"context: error with space"`,
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.error, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.error, tt.format, tt.want)
 	}
 }
 
@@ -100,20 +118,24 @@ func TestFormatWrapf(t *testing.T) {
 		format string
 		want   string
 	}{{
-		Wrapf(New("error"), "error%d", 2),
+		Wrapf(io.EOF, "error%d", 2),
 		"%s",
-		"error2: error",
+		"error2: EOF",
 	}, {
-		Wrap(io.EOF, "error"),
+		Wrapf(io.EOF, "error%d", 2),
 		"%v",
-		"error: EOF",
+		"error2: EOF",
 	}, {
-		Wrap(io.EOF, "error"),
+		Wrapf(io.EOF, "error%d", 2),
 		"%+v",
 		"EOF\n" +
-			"error\n" +
+			"error2\n" +
 			"github.com/pkg/errors.TestFormatWrapf\n" +
-			"\t.+/github.com/pkg/errors/format_test.go:111",
+			"\t.+/github.com/pkg/errors/format_test.go:129",
+	}, {
+		Wrapf(New("error"), "error%d", 2),
+		"%s",
+		"error2: error",
 	}, {
 		Wrapf(New("error"), "error%d", 2),
 		"%v",
@@ -123,22 +145,15 @@ func TestFormatWrapf(t *testing.T) {
 		"%+v",
 		"error\n" +
 			"github.com/pkg/errors.TestFormatWrapf\n" +
-			"\t.+/github.com/pkg/errors/format_test.go:122",
-	}, {
-		Wrap(Wrap(io.EOF, "error1"), "error2"),
-		"%+v",
-		"EOF\n" +
-			"error1\n" +
-			"github.com/pkg/errors.TestFormatWrapf\n" +
-			"\t.+/github.com/pkg/errors/format_test.go:128\n",
+			"\t.+/github.com/pkg/errors/format_test.go:144",
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.error, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.error, tt.format, tt.want)
 	}
 }
 
-func testFormatRegexp(t *testing.T, arg interface{}, format, want string) {
+func testFormatRegexp(t *testing.T, n int, arg interface{}, format, want string) {
 	got := fmt.Sprintf(format, arg)
 	lines := strings.SplitN(got, "\n", -1)
 	for i, w := range strings.SplitN(want, "\n", -1) {
@@ -147,7 +162,7 @@ func testFormatRegexp(t *testing.T, arg interface{}, format, want string) {
 			t.Fatal(err)
 		}
 		if !match {
-			t.Errorf("fmt.Sprintf(%q, err): got: %q, want: %q", format, got, want)
+			t.Errorf("test %d: line %d: fmt.Sprintf(%q, err): got: %q, want: %q", n+1, i+1, format, got, want)
 		}
 	}
 }

stack.go

@@ -100,6 +100,19 @@ func (st StackTrace) Format(s fmt.State, verb rune) {
 // stack represents a stack of program counters.
 type stack []uintptr
 
+func (s *stack) Format(st fmt.State, verb rune) {
+	switch verb {
+	case 'v':
+		switch {
+		case st.Flag('+'):
+			for _, pc := range *s {
+				f := Frame(pc)
+				fmt.Fprintf(st, "\n%+v", f)
+			}
+		}
+	}
+}
+
 func (s *stack) StackTrace() StackTrace {
 	f := make([]Frame, len(*s))
 	for i := 0; i < len(f); i++ {

stack_test.go

@@ -120,8 +120,8 @@ func TestFrameFormat(t *testing.T) {
 		"unknown:0",
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.Frame, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.Frame, tt.format, tt.want)
 	}
 }
 
@@ -155,12 +155,12 @@ func TestTrimGOPATH(t *testing.T) {
 		"github.com/pkg/errors/stack_test.go",
 	}}
 
-	for _, tt := range tests {
+	for i, tt := range tests {
 		pc := tt.Frame.pc()
 		fn := runtime.FuncForPC(pc)
 		file, _ := fn.FileLine(pc)
 		got := trimGOPATH(fn.Name(), file)
-		testFormatRegexp(t, got, "%s", tt.want)
+		testFormatRegexp(t, i, got, "%s", tt.want)
 	}
 }
 
@@ -204,7 +204,7 @@ func TestStackTrace(t *testing.T) {
 				"\t.+/github.com/pkg/errors/stack_test.go:198", // this is the stack of Errorf's caller's caller
 		},
 	}}
-	for _, tt := range tests {
+	for i, tt := range tests {
 		x, ok := tt.err.(interface {
 			StackTrace() StackTrace
 		})
@@ -214,7 +214,7 @@ func TestStackTrace(t *testing.T) {
 		}
 		st := x.StackTrace()
 		for j, want := range tt.want {
-			testFormatRegexp(t, st[j], "%+v", want)
+			testFormatRegexp(t, i, st[j], "%+v", want)
 		}
 	}
 }
@@ -286,7 +286,7 @@ func TestStackTraceFormat(t *testing.T) {
 		`\[\]errors.Frame{stack_test.go:225, stack_test.go:284}`,
 	}}
 
-	for _, tt := range tests {
-		testFormatRegexp(t, tt.StackTrace, tt.format, tt.want)
+	for i, tt := range tests {
+		testFormatRegexp(t, i, tt.StackTrace, tt.format, tt.want)
 	}
 }