Skip to content

Commit 9cadab9

Browse files
davecheneydavecheney
authored
Merge pull request #81 from pkg/withMessage-withStack
Refactor withMessage/withStack
2 parents 2a9be18 + 1b876e0 commit 9cadab9

File tree

4 files changed

+137
-87
lines changed

4 files changed

+137
-87
lines changed

‎errors.go‎

Lines changed: 76 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -91,68 +91,60 @@ import (
9191
"io"
9292
)
9393

94-
// _error is an error implementation returned by New and Errorf
95-
// that implements its own fmt.Formatter.
96-
type _error struct {
97-
msg string
98-
*stack
99-
}
100-
101-
func (e _error) Error() string { return e.msg }
102-
103-
func (e _error) Format(s fmt.State, verb rune) {
104-
switch verb {
105-
case 'v':
106-
if s.Flag('+') {
107-
io.WriteString(s, e.msg)
108-
fmt.Fprintf(s, "%+v", e.StackTrace())
109-
return
110-
}
111-
fallthrough
112-
case 's':
113-
io.WriteString(s, e.msg)
114-
}
115-
}
116-
11794
// New returns an error with the supplied message.
95+
// New also records the stack trace at the point it was called.
11896
func New(message string) error {
119-
return _error{
120-
message,
121-
callers(),
97+
return &fundamental{
98+
msg: message,
99+
stack: callers(),
122100
}
123101
}
124102

125103
// Errorf formats according to a format specifier and returns the string
126104
// as a value that satisfies error.
105+
// Errorf also records the stack trace at the point it was called.
127106
func Errorf(format string, args ...interface{}) error {
128-
return _error{
129-
fmt.Sprintf(format, args...),
130-
callers(),
107+
return &fundamental{
108+
msg: fmt.Sprintf(format, args...),
109+
stack: callers(),
131110
}
132111
}
133112

134-
type cause struct {
135-
cause error
136-
msg string
113+
// fundamental is an error that has a message and a stack, but no caller.
114+
type fundamental struct {
115+
msg string
116+
*stack
137117
}
138118

139-
func (c cause) Error() string { return fmt.Sprintf("%s: %v", c.msg, c.Cause()) }
140-
func (c cause) Cause() error { return c.cause }
119+
func (f *fundamental) Error() string { return f.msg }
120+
121+
func (f *fundamental) Format(s fmt.State, verb rune) {
122+
switch verb {
123+
case 'v':
124+
if s.Flag('+') {
125+
io.WriteString(s, f.msg)
126+
f.stack.Format(s, verb)
127+
return
128+
}
129+
fallthrough
130+
case 's', 'q':
131+
io.WriteString(s, f.msg)
132+
}
133+
}
141134

142-
// wrapper is an error implementation returned by Wrap and Wrapf
143-
// that implements its own fmt.Formatter.
144-
type wrapper struct {
145-
cause
135+
type withStack struct {
136+
error
146137
*stack
147138
}
148139

149-
func (w wrapper) Format(s fmt.State, verb rune) {
140+
func (w *withStack) Cause() error { return w.error }
141+
142+
func (w *withStack) Format(s fmt.State, verb rune) {
150143
switch verb {
151144
case 'v':
152145
if s.Flag('+') {
153-
fmt.Fprintf(s, "%+v\n", w.Cause())
154-
io.WriteString(s, w.msg)
155-
fmt.Fprintf(s, "%+v", w.StackTrace())
146+
fmt.Fprintf(s, "%+v", w.Cause())
147+
w.stack.Format(s, verb)
156148
return
157149
}
158150
fallthrough
@@ -165,31 +157,61 @@ func (w wrapper) Format(s fmt.State, verb rune) {
165157

166158
// Wrap returns an error annotating err with message.
167159
// If err is nil, Wrap returns nil.
160+
// Wrap is conceptually the same as calling
161+
//
162+
// errors.WithStack(errors.WithMessage(err, msg))
168163
func Wrap(err error, message string) error {
169164
if err == nil {
170165
return nil
171166
}
172-
return wrapper{
173-
cause: cause{
174-
cause: err,
175-
msg: message,
176-
},
177-
stack: callers(),
167+
err = &withMessage{
168+
cause: err,
169+
msg: message,
170+
}
171+
return &withStack{
172+
err,
173+
callers(),
178174
}
179175
}
180176

181177
// Wrapf returns an error annotating err with the format specifier.
182178
// If err is nil, Wrapf returns nil.
179+
// Wrapf is conceptually the same as calling
180+
//
181+
// errors.WithStack(errors.WithMessage(err, format, args...))
183182
func Wrapf(err error, format string, args ...interface{}) error {
184183
if err == nil {
185184
return nil
186185
}
187-
return wrapper{
188-
cause: cause{
189-
cause: err,
190-
msg: fmt.Sprintf(format, args...),
191-
},
192-
stack: callers(),
186+
err = &withMessage{
187+
cause: err,
188+
msg: fmt.Sprintf(format, args...),
189+
}
190+
return &withStack{
191+
err,
192+
callers(),
193+
}
194+
}
195+
196+
type withMessage struct {
197+
cause error
198+
msg string
199+
}
200+
201+
func (w *withMessage) Error() string { return w.msg + ": " + w.cause.Error() }
202+
func (w *withMessage) Cause() error { return w.cause }
203+
204+
func (w *withMessage) Format(s fmt.State, verb rune) {
205+
switch verb {
206+
case 'v':
207+
if s.Flag('+') {
208+
fmt.Fprintf(s, "%+v\n", w.Cause())
209+
io.WriteString(s, w.msg)
210+
return
211+
}
212+
fallthrough
213+
case 's', 'q':
214+
io.WriteString(s, w.Error())
193215
}
194216
}
195217

‎format_test.go‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ func TestFormatNew(t *testing.T) {
2929
"\t.+/github.com/pkg/errors/format_test.go:25",
3030
}}
3131

32-
for _, tt := range tests {
33-
testFormatRegexp(t, tt.error, tt.format, tt.want)
32+
for i, tt := range tests {
33+
testFormatRegexp(t, i, tt.error, tt.format, tt.want)
3434
}
3535
}
3636

@@ -55,8 +55,8 @@ func TestFormatErrorf(t *testing.T) {
5555
"\t.+/github.com/pkg/errors/format_test.go:51",
5656
}}
5757

58-
for _, tt := range tests {
59-
testFormatRegexp(t, tt.error, tt.format, tt.want)
58+
for i, tt := range tests {
59+
testFormatRegexp(t, i, tt.error, tt.format, tt.want)
6060
}
6161
}
6262

@@ -83,14 +83,32 @@ func TestFormatWrap(t *testing.T) {
8383
Wrap(io.EOF, "error"),
8484
"%s",
8585
"error: EOF",
86+
}, {
87+
Wrap(io.EOF, "error"),
88+
"%v",
89+
"error: EOF",
90+
}, {
91+
Wrap(io.EOF, "error"),
92+
"%+v",
93+
"EOF\n" +
94+
"error\n" +
95+
"github.com/pkg/errors.TestFormatWrap\n" +
96+
"\t.+/github.com/pkg/errors/format_test.go:91",
97+
}, {
98+
Wrap(Wrap(io.EOF, "error1"), "error2"),
99+
"%+v",
100+
"EOF\n" +
101+
"error1\n" +
102+
"github.com/pkg/errors.TestFormatWrap\n" +
103+
"\t.+/github.com/pkg/errors/format_test.go:98\n",
86104
}, {
87105
Wrap(New("error with space"), "context"),
88106
"%q",
89107
`"context: error with space"`,
90108
}}
91109

92-
for _, tt := range tests {
93-
testFormatRegexp(t, tt.error, tt.format, tt.want)
110+
for i, tt := range tests {
111+
testFormatRegexp(t, i, tt.error, tt.format, tt.want)
94112
}
95113
}
96114

@@ -100,20 +118,24 @@ func TestFormatWrapf(t *testing.T) {
100118
format string
101119
want string
102120
}{{
103-
Wrapf(New("error"), "error%d", 2),
121+
Wrapf(io.EOF, "error%d", 2),
104122
"%s",
105-
"error2: error",
123+
"error2: EOF",
106124
}, {
107-
Wrap(io.EOF, "error"),
125+
Wrapf(io.EOF, "error%d", 2),
108126
"%v",
109-
"error: EOF",
127+
"error2: EOF",
110128
}, {
111-
Wrap(io.EOF, "error"),
129+
Wrapf(io.EOF, "error%d", 2),
112130
"%+v",
113131
"EOF\n" +
114-
"error\n" +
132+
"error2\n" +
115133
"github.com/pkg/errors.TestFormatWrapf\n" +
116-
"\t.+/github.com/pkg/errors/format_test.go:111",
134+
"\t.+/github.com/pkg/errors/format_test.go:129",
135+
}, {
136+
Wrapf(New("error"), "error%d", 2),
137+
"%s",
138+
"error2: error",
117139
}, {
118140
Wrapf(New("error"), "error%d", 2),
119141
"%v",
@@ -123,22 +145,15 @@ func TestFormatWrapf(t *testing.T) {
123145
"%+v",
124146
"error\n" +
125147
"github.com/pkg/errors.TestFormatWrapf\n" +
126-
"\t.+/github.com/pkg/errors/format_test.go:122",
127-
}, {
128-
Wrap(Wrap(io.EOF, "error1"), "error2"),
129-
"%+v",
130-
"EOF\n" +
131-
"error1\n" +
132-
"github.com/pkg/errors.TestFormatWrapf\n" +
133-
"\t.+/github.com/pkg/errors/format_test.go:128\n",
148+
"\t.+/github.com/pkg/errors/format_test.go:144",
134149
}}
135150

136-
for _, tt := range tests {
137-
testFormatRegexp(t, tt.error, tt.format, tt.want)
151+
for i, tt := range tests {
152+
testFormatRegexp(t, i, tt.error, tt.format, tt.want)
138153
}
139154
}
140155

141-
func testFormatRegexp(t *testing.T, arg interface{}, format, want string) {
156+
func testFormatRegexp(t *testing.T, n int, arg interface{}, format, want string) {
142157
got := fmt.Sprintf(format, arg)
143158
lines := strings.SplitN(got, "\n", -1)
144159
for i, w := range strings.SplitN(want, "\n", -1) {
@@ -147,7 +162,7 @@ func testFormatRegexp(t *testing.T, arg interface{}, format, want string) {
147162
t.Fatal(err)
148163
}
149164
if !match {
150-
t.Errorf("fmt.Sprintf(%q, err): got: %q, want: %q", format, got, want)
165+
t.Errorf("test %d: line %d: fmt.Sprintf(%q, err): got: %q, want: %q", n+1, i+1, format, got, want)
151166
}
152167
}
153168
}

‎stack.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,19 @@ func (st StackTrace) Format(s fmt.State, verb rune) {
100100
// stack represents a stack of program counters.
101101
type stack []uintptr
102102

103+
func (s *stack) Format(st fmt.State, verb rune) {
104+
switch verb {
105+
case 'v':
106+
switch {
107+
case st.Flag('+'):
108+
for _, pc := range *s {
109+
f := Frame(pc)
110+
fmt.Fprintf(st, "\n%+v", f)
111+
}
112+
}
113+
}
114+
}
115+
103116
func (s *stack) StackTrace() StackTrace {
104117
f := make([]Frame, len(*s))
105118
for i := 0; i < len(f); i++ {

‎stack_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ func TestFrameFormat(t *testing.T) {
120120
"unknown:0",
121121
}}
122122

123-
for _, tt := range tests {
124-
testFormatRegexp(t, tt.Frame, tt.format, tt.want)
123+
for i, tt := range tests {
124+
testFormatRegexp(t, i, tt.Frame, tt.format, tt.want)
125125
}
126126
}
127127

@@ -155,12 +155,12 @@ func TestTrimGOPATH(t *testing.T) {
155155
"github.com/pkg/errors/stack_test.go",
156156
}}
157157

158-
for _, tt := range tests {
158+
for i, tt := range tests {
159159
pc := tt.Frame.pc()
160160
fn := runtime.FuncForPC(pc)
161161
file, _ := fn.FileLine(pc)
162162
got := trimGOPATH(fn.Name(), file)
163-
testFormatRegexp(t, got, "%s", tt.want)
163+
testFormatRegexp(t, i, got, "%s", tt.want)
164164
}
165165
}
166166

@@ -204,7 +204,7 @@ func TestStackTrace(t *testing.T) {
204204
"\t.+/github.com/pkg/errors/stack_test.go:198", // this is the stack of Errorf's caller's caller
205205
},
206206
}}
207-
for _, tt := range tests {
207+
for i, tt := range tests {
208208
x, ok := tt.err.(interface {
209209
StackTrace() StackTrace
210210
})
@@ -214,7 +214,7 @@ func TestStackTrace(t *testing.T) {
214214
}
215215
st := x.StackTrace()
216216
for j, want := range tt.want {
217-
testFormatRegexp(t, st[j], "%+v", want)
217+
testFormatRegexp(t, i, st[j], "%+v", want)
218218
}
219219
}
220220
}
@@ -286,7 +286,7 @@ func TestStackTraceFormat(t *testing.T) {
286286
`\[\]errors.Frame{stack_test.go:225, stack_test.go:284}`,
287287
}}
288288

289-
for _, tt := range tests {
290-
testFormatRegexp(t, tt.StackTrace, tt.format, tt.want)
289+
for i, tt := range tests {
290+
testFormatRegexp(t, i, tt.StackTrace, tt.format, tt.want)
291291
}
292292
}

0 commit comments

Comments
 (0)