From c2f22ebc8523869bebea61c180e16a0daac747f0 Mon Sep 17 00:00:00 2001 From: Keepers Date: Mon, 15 Jul 2024 17:09:56 -0600 Subject: [PATCH] Improve the error trace instead of printing out the absolute Path for each error (which is a bit overkill in most situations), instead write the func name, and the parentdir + file:line where the error was generated or wrapped. --- clues_test.go | 6 +++--- datanode.go | 36 ++++++++++++++++++++----------- err.go | 40 ++++++++++++++++++++++++++--------- err_fmt_test.go | 56 ++++++++++++++++++++++++------------------------- 4 files changed, 85 insertions(+), 53 deletions(-) diff --git a/clues_test.go b/clues_test.go index 56a0bea..a46b9c9 100644 --- a/clues_test.go +++ b/clues_test.go @@ -677,9 +677,9 @@ func TestAddComment_trace(t *testing.T) { comments := dn.Comments() stack := comments.String() expected := commentRE( - "TestAddComment_trace", "clues_test.go", "one", - "addCommentToCtx", "clues_test.go", "two", - "TestAddComment_trace", "clues_test.go", `three$`) + "TestAddComment_trace", "clues/clues_test.go", "one", + "addCommentToCtx", "clues/clues_test.go", "two", + "TestAddComment_trace", "clues/clues_test.go", `three$`) commentMatches(t, expected, stack) } diff --git a/datanode.go b/datanode.go index 24f1949..cacf3a4 100644 --- a/datanode.go +++ b/datanode.go @@ -226,8 +226,6 @@ func (dn *dataNode) Slice() []any { type comment struct { // the func name in which the comment was created. Caller string - // the directory path of the file owning the Caller. - Dir string // the name of the file owning the caller. File string // the comment message itself. @@ -249,13 +247,11 @@ func newComment( values ...any, ) comment { caller := getCaller(depth + 1) - longTrace := getTrace(depth + 1) - dir, file := path.Split(longTrace) + _, _, parentFileLine := getDirAndFile(depth + 1) return comment{ Caller: caller, - Dir: dir, - File: file, + File: parentFileLine, Message: fmt.Sprintf(template, values...), } } @@ -376,14 +372,30 @@ func makeNodeID() string { return uns[:4] + uns[len(uns)-4:] } -// getTrace retrieves the file and line number of the caller. Depth is the -// skip-caller count. Clues funcs that call this one should provide either -// `1` (if they do not already have a depth value), or `depth+1` otherwise`. +// getDirAndFile retrieves the file and line number of the caller. +// Depth is the skip-caller count. Clues funcs that call this one should +// provide either `1` (if they do not already have a depth value), or `depth+1` +// otherwise`. // -// Formats to: `:` -func getTrace(depth int) string { +// formats: +// dir `absolute/os/path/to/parent/folder` +// fileAndLine `:` +// parentAndFileAndLine `/:` +func getDirAndFile( + depth int, +) (dir, fileAndLine, parentAndFileAndLine string) { _, file, line, _ := runtime.Caller(depth + 1) - return fmt.Sprintf("%s:%d", file, line) + dir, file = path.Split(file) + + fileLine := fmt.Sprintf("%s:%d", file, line) + parentFileLine := fileLine + + parent := path.Base(dir) + if len(parent) > 0 { + parentFileLine = path.Join(parent, fileLine) + } + + return dir, fileLine, parentFileLine } // getCaller retrieves the func name of the caller. Depth is the skip-caller diff --git a/err.go b/err.go index 782d348..6ff1bdb 100644 --- a/err.go +++ b/err.go @@ -25,8 +25,11 @@ type Err struct { // in pre-order traversal. stack []error - // location is used for printing %+v stack traces - location string + // the func name in which the error was created. + // /: + file string + // the name of the file owning the caller. + caller string // msg is the message for this error. msg string @@ -53,10 +56,13 @@ func newErr( m map[string]any, traceDepth int, ) *Err { + _, _, file := getDirAndFile(traceDepth + 1) + return &Err{ - e: e, - location: getTrace(traceDepth + 1), - msg: msg, + e: e, + file: file, + caller: getCaller(traceDepth + 1), + msg: msg, data: &dataNode{ id: makeNodeID(), values: m, @@ -93,10 +99,13 @@ func toStack( stack []error, traceDepth int, ) *Err { + _, _, file := getDirAndFile(traceDepth + 1) + return &Err{ - e: e, - location: getTrace(traceDepth + 1), - stack: stack, + e: e, + file: file, + caller: getCaller(traceDepth + 1), + stack: stack, data: &dataNode{ id: makeNodeID(), values: map[string]any{}, @@ -449,7 +458,17 @@ func formatPlusV(err *Err, s fmt.State, verb rune) { } write(s, verb, err.msg) - write(s, verb, "\n\t%s", err.location) + + parts := []string{} + if len(err.caller) > 0 { + parts = append(parts, err.caller) + } + + if len(err.file) > 0 { + parts = append(parts, err.file) + } + + write(s, verb, "\n\t%s", strings.Join(parts, " - ")) } // Format ensures stack traces are printed appropariately. @@ -871,7 +890,8 @@ func (err *Err) SkipCaller(depth int) *Err { depth = 0 } - err.location = getTrace(depth + 1) + _, _, err.file = getDirAndFile(depth + 1) + err.caller = getCaller(depth + 1) return err } diff --git a/err_fmt_test.go b/err_fmt_test.go index a082447..c4ca1c9 100644 --- a/err_fmt_test.go +++ b/err_fmt_test.go @@ -1544,14 +1544,14 @@ func TestWithComment(t *testing.T) { return withCommentWrapper(err, "fisher") }, expect: commentRE( - `withCommentWrapper`, `err_test.go`, `fisher`, - `withCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `fisher`, + `withCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), expectWrapped: commentRE( - `withCommentWrapper`, `err_test.go`, `fisher`, - `withCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `fisher`, + `withCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), expectStacked: commentRE( - `withCommentWrapper`, `err_test.go`, `fisher`, - `withCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `fisher`, + `withCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), }, { name: "error formatted comment", @@ -1559,14 +1559,14 @@ func TestWithComment(t *testing.T) { return withCommentWrapper(err, "%d", 42) }, expect: commentRE( - `withCommentWrapper`, `err_test.go`, `42`, - `withCommentWrapper`, `err_test.go`, `42 - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `42`, + `withCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), expectWrapped: commentRE( - `withCommentWrapper`, `err_test.go`, `42`, - `withCommentWrapper`, `err_test.go`, `42 - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `42`, + `withCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), expectStacked: commentRE( - `withCommentWrapper`, `err_test.go`, `42`, - `withCommentWrapper`, `err_test.go`, `42 - repeat$`), + `withCommentWrapper`, `clues/err_test.go`, `42`, + `withCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), }, } for _, test := range table { @@ -1610,14 +1610,14 @@ func TestWithComment(t *testing.T) { return cluesWithCommentWrapper(err, "fisher") }, expect: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `fisher`, - `cluesWithCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), expectWrapped: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `fisher`, - `cluesWithCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), expectStacked: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `fisher`, - `cluesWithCommentWrapper`, `err_test.go`, `fisher - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `fisher - repeat$`), }, { name: "clues.Err formatted comment", @@ -1625,14 +1625,14 @@ func TestWithComment(t *testing.T) { return cluesWithCommentWrapper(err, "%d", 42) }, expect: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `42`, - `cluesWithCommentWrapper`, `err_test.go`, `42 - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `42`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), expectWrapped: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `42`, - `cluesWithCommentWrapper`, `err_test.go`, `42 - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `42`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), expectStacked: commentRE( - `cluesWithCommentWrapper`, `err_test.go`, `42`, - `cluesWithCommentWrapper`, `err_test.go`, `42 - repeat$`), + `cluesWithCommentWrapper`, `clues/err_test.go`, `42`, + `cluesWithCommentWrapper`, `clues/err_test.go`, `42 - repeat$`), }, } for _, test := range table2 { @@ -1686,7 +1686,7 @@ func TestErrCore_String(t *testing.T) { Label("label"). Core(), expectS: `{"message", [label], {key:value}}`, - expectVPlus: `{msg:"message", labels:[label], values:{key:value}}`, + expectVPlus: `{msg:"message", labels:[label], values:{key:value}, comments:[]}`, }, { name: "message only", @@ -1694,7 +1694,7 @@ func TestErrCore_String(t *testing.T) { New("message"). Core(), expectS: `{"message"}`, - expectVPlus: `{msg:"message", labels:[], values:{}}`, + expectVPlus: `{msg:"message", labels:[], values:{}, comments:[]}`, }, { name: "labels only", @@ -1703,7 +1703,7 @@ func TestErrCore_String(t *testing.T) { Label("label"). Core(), expectS: `{[label]}`, - expectVPlus: `{msg:"", labels:[label], values:{}}`, + expectVPlus: `{msg:"", labels:[label], values:{}, comments:[]}`, }, { name: "values only", @@ -1712,7 +1712,7 @@ func TestErrCore_String(t *testing.T) { With("key", "value"). Core(), expectS: `{{key:value}}`, - expectVPlus: `{msg:"", labels:[], values:{key:value}}`, + expectVPlus: `{msg:"", labels:[], values:{key:value}, comments:[]}`, }, } for _, test := range table {