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.
alcionai · Jul 15, 2024 · c2f22eb · c2f22eb
1 parent 94f93ee
commit c2f22eb
Show file tree

Hide file tree

Showing 4 changed files with 85 additions and 53 deletions.
diff --git 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
@@ -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: `<file>:<line>`
-func getTrace(depth int) string {
+// formats:
+// dir `absolute/os/path/to/parent/folder`
+// fileAndLine `<file>:<line>`
+// parentAndFileAndLine `<parent>/<file>:<line>`
+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
@@ -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.
+	// <parentFolder>/<filename>:<line>
+	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
@@ -1544,29 +1544,29 @@ 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",
 			commenter: func(err error) error {
 				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,29 +1610,29 @@ 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",
 			commenter: func(err *clues.Err) error {
 				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,15 +1686,15 @@ 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",
 			core: clues.
 				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 {