From 3fbe6980bc305829fd1631d584bddaa2849a6f43 Mon Sep 17 00:00:00 2001 From: Ceri Date: Tue, 12 Mar 2024 12:17:05 +0000 Subject: [PATCH 1/4] Include causal chain in StackString(). --- errors.go | 18 ++++++++++++++++-- errors_test.go | 12 ++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index dffc743..76c3ada 100644 --- a/errors.go +++ b/errors.go @@ -160,10 +160,24 @@ func (p *Error) StackTrace() []uintptr { // StackString formats the stack as a beautiful string with newlines func (p *Error) StackString() string { + // TODO: Use a string builder. stackStr := "" - for _, frame := range p.StackFrames { - stackStr = fmt.Sprintf("%s\n %s:%d in %s", stackStr, frame.Filename, frame.Line, frame.Method) + terr := p + for terr != nil { + if len(stackStr) != 0 && len(terr.StackFrames) > 0 { + stackStr = fmt.Sprintf("%s\n---\n", stackStr) + } + for _, frame := range terr.StackFrames { + stackStr = fmt.Sprintf("%s\n %s:%d in %s", stackStr, frame.Filename, frame.Line, frame.Method) + } + + if tcause, ok := terr.cause.(*Error); ok { + terr = tcause + } else { + break + } } + return stackStr } diff --git a/errors_test.go b/errors_test.go index cdf1be0..9dd49e1 100644 --- a/errors_test.go +++ b/errors_test.go @@ -583,3 +583,15 @@ func TestSetIsUnexpected(t *testing.T) { err.SetIsUnexpected(false) assert.False(t, *err.IsUnexpected) } + +func failyFunction() error { + return InternalService("halp", "I'm in trouble", nil) +} + +func TestStackStringChasesCausalChain(t *testing.T) { + err := Augment(failyFunction(), "something may be up", nil) + terr := err.(*Error) + ss := terr.StackString() + t.Log(ss) + assert.Contains(t, ss, "failyFunction") +} From 745d41c65d9dd02016a4f402475494dd93e35ddd Mon Sep 17 00:00:00 2001 From: Ceri Date: Tue, 12 Mar 2024 12:21:28 +0000 Subject: [PATCH 2/4] Use string builder instead of repeated, re-allocating sprintfs. --- errors.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/errors.go b/errors.go index 76c3ada..cebb87a 100644 --- a/errors.go +++ b/errors.go @@ -160,15 +160,14 @@ func (p *Error) StackTrace() []uintptr { // StackString formats the stack as a beautiful string with newlines func (p *Error) StackString() string { - // TODO: Use a string builder. - stackStr := "" + var buffer strings.Builder terr := p for terr != nil { - if len(stackStr) != 0 && len(terr.StackFrames) > 0 { - stackStr = fmt.Sprintf("%s\n---\n", stackStr) + if buffer.Len() != 0 && len(terr.StackFrames) > 0 { + fmt.Fprintf(&buffer, "\n---") } for _, frame := range terr.StackFrames { - stackStr = fmt.Sprintf("%s\n %s:%d in %s", stackStr, frame.Filename, frame.Line, frame.Method) + fmt.Fprintf(&buffer, "\n %s:%d in %s", frame.Filename, frame.Line, frame.Method) } if tcause, ok := terr.cause.(*Error); ok { @@ -178,7 +177,7 @@ func (p *Error) StackString() string { } } - return stackStr + return buffer.String() } // VerboseString returns the error message, stack trace and params From 29ed05195c42acde581a6f4fe7706e2c6f60a745 Mon Sep 17 00:00:00 2001 From: Ceri Date: Tue, 12 Mar 2024 15:04:00 +0000 Subject: [PATCH 3/4] Update docstring. --- errors.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/errors.go b/errors.go index cebb87a..bf876a7 100644 --- a/errors.go +++ b/errors.go @@ -158,7 +158,9 @@ func (p *Error) StackTrace() []uintptr { return out } -// StackString formats the stack as a beautiful string with newlines +// StackString formats the stacks from the terror chain as a string. If we +// encounter more than one terror in the chain with a stack frame, we'll print +// each one, separated by three hyphens on their own line. func (p *Error) StackString() string { var buffer strings.Builder terr := p From acf7d7aeed4fb020499005d58c6a52a11503d078 Mon Sep 17 00:00:00 2001 From: Ceri Date: Tue, 12 Mar 2024 15:27:32 +0000 Subject: [PATCH 4/4] Add limits. --- errors.go | 21 +++++++++++++++++++-- errors_test.go | 21 +++++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index bf876a7..737daf7 100644 --- a/errors.go +++ b/errors.go @@ -162,20 +162,37 @@ func (p *Error) StackTrace() []uintptr { // encounter more than one terror in the chain with a stack frame, we'll print // each one, separated by three hyphens on their own line. func (p *Error) StackString() string { + // 32,000 seems like a reasonable limit for a stack trace. Otherwise, we risk + // overwhelming downstream systems. + return StackStringWithMaxSize(p, 32000) +} + +func StackStringWithMaxSize(p *Error, sizeLimit int) string { + // if we run into this many causes, we've likely run into something absurd. Like + // a self causing error. + const maxCausalDepth = 1024 var buffer strings.Builder terr := p + var causalDepth int +outer: for terr != nil { if buffer.Len() != 0 && len(terr.StackFrames) > 0 { fmt.Fprintf(&buffer, "\n---") } for _, frame := range terr.StackFrames { + // 10 seems like a reasonable estimate of how large the rest of the line would be. + estimatedLineLen := len(frame.Filename) + len(frame.Method) + 16 + if estimatedLineLen+buffer.Len() > sizeLimit { + break outer + } fmt.Fprintf(&buffer, "\n %s:%d in %s", frame.Filename, frame.Line, frame.Method) } - if tcause, ok := terr.cause.(*Error); ok { + if tcause, ok := terr.cause.(*Error); ok && causalDepth < maxCausalDepth { terr = tcause + causalDepth += 1 } else { - break + break outer } } diff --git a/errors_test.go b/errors_test.go index 9dd49e1..4aa9cd2 100644 --- a/errors_test.go +++ b/errors_test.go @@ -595,3 +595,24 @@ func TestStackStringChasesCausalChain(t *testing.T) { t.Log(ss) assert.Contains(t, ss, "failyFunction") } + +func TestCircularErrorProducesFiniteOutputWithStackFrames(t *testing.T) { + orig := failyFunction() + err := Augment(orig, "something may be up", nil) + terr := err.(*Error) + terr.cause = terr + terr.StackFrames = orig.(*Error).StackFrames + ss := terr.StackString() + + // The default field size limit used in elastic-slog. It's kind of arbitrary, but it'll do for now. + assert.Less(t, len(ss), 32000) + assert.GreaterOrEqual(t, len(ss), 32000-1000) +} +func TestCircularErrorProducesFiniteOutputWithoutStackFrames(t *testing.T) { + err := Augment(failyFunction(), "something may be up", nil) + terr := err.(*Error) + terr.cause = terr + ss := terr.StackString() + // There's no actual stack in the causal cycle, so we don't render anything here. + assert.Empty(t, ss) +}