Destructure Wrap{,f} into WithStack(WithMessage(err, msg))

Introduces WithMessage as well as errors.fundamental, errors.withMessage
and errors.withStack internal types.

Adjust tests for the new wrapped format when combining fundamental and
wrapped errors.

Author: davecheney

errors.go

@@ -87,71 +87,76 @@
 package errors
 
 import (
-	"errors"
 	"fmt"
 	"io"
 )
 
 // 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 {
-	err := errors.New(message)
-	return &withStack{
-		err,
-		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 {
-	err := fmt.Errorf(format, args...)
-	return &withStack{
-		err,
-		callers(),
+	return &fundamental{
+		msg:   fmt.Sprintf(format, args...),
+		stack: callers(),
 	}
 }
 
-type withStack struct {
-	error
+// fundamental is an error that has a message and a stack, but no caller.
+type fundamental struct {
+	msg string
 	*stack
 }
 
-func (w *withStack) Format(s fmt.State, verb rune) {
+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, w.Error())
-			w.stack.Format(s, verb)
+			io.WriteString(s, f.msg)
+			f.stack.Format(s, verb)
 			return
 		}
 		fallthrough
-	case 's':
-		io.WriteString(s, w.Error())
+	case 's', 'q':
+		io.WriteString(s, f.msg)
 	}
 }
 
-type cause struct {
-	cause error
-	msg   string
+// WithStack annotates err with a stack trace at the point WithStack was called.
+// If err is nil, WithStack returns nil.
+func WithStack(err error) error {
+	if err == nil {
+		return nil
+	}
+	return &withStack{
+		err,
+		callers(),
+	}
 }
 
-func (c cause) Error() string { return fmt.Sprintf("%s: %v", c.msg, c.Cause()) }
-func (c cause) Cause() error  { return c.cause }
-
-// 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
@@ -164,31 +169,73 @@ 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(),
+	}
+}
+
+// WithMessage annotates err with a new message.
+// If err is nil, WithStack returns nil.
+func WithMessage(err error, message string) error {
+	if err == nil {
+		return nil
+	}
+	return &withMessage{
+		cause: err,
+		msg:   message,
+	}
+}
+
+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_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)
 	}
 }
 
@@ -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)
 	}
 }