ensure lowest-level data takes priority (#44)

changes data cascade behavior in errors to prioritize the earliest-seeded data in all cases. Ex: if an earlier error return contains a k:v pair, a higher layer wrap on that error should not be able to replace the value for that key, whether through With() calls or WC(ctx). The ultimate result of this is that no caller will need to consider if they're choosing the "right time" to add a value or embed the context. It's always going to be correct, with priority favoring the earliest error.
alcionai · Mar 22, 2024 · bbc6bfe · bbc6bfe
1 parent 069212a
commit bbc6bfe
Show file tree

Hide file tree

Showing 3 changed files with 88 additions and 37 deletions.
diff --git a/err.go b/err.go
@@ -371,14 +371,13 @@ func (err *Err) values() map[string]any {
 	}
 
 	vals := map[string]any{}
+	maps.Copy(vals, err.data.Map())
+	maps.Copy(vals, inErr(err.e))
 
 	for _, se := range err.stack {
 		maps.Copy(vals, inErr(se))
 	}
 
-	maps.Copy(vals, inErr(err.e))
-	maps.Copy(vals, err.data.Map())
-
 	return vals
 }
 

diff --git a/err_fmt_test.go b/err_fmt_test.go
@@ -24,7 +24,7 @@ func plusRE(lines ...string) string {
 		s += lines[i]
 
 		if len(lines[i+1]) > 0 {
-			s += `\t.+/` + lines[i+1]
+			s += `\t.*/?` + lines[i+1]
 		}
 	}
 
@@ -94,6 +94,8 @@ var (
 	cluesStack      = func(err error) error { return clues.Stack(globalSentinel, err) }
 )
 
+const cluesInitFileLoc = `(<autogenerated>:\d+|.+/clues/.+go:?\d+)\n`
+
 func TestFmt(t *testing.T) {
 	type expect struct {
 		v    string
@@ -146,7 +148,7 @@ func TestFmt(t *testing.T) {
 				q:    `"errors wrap: an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -234,7 +236,7 @@ func TestFmt(t *testing.T) {
 				q:    `"an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -295,7 +297,7 @@ func TestFmt(t *testing.T) {
 				q:    `"clues wrap": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -358,7 +360,7 @@ func TestFmt(t *testing.T) {
 				q:    `"an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -408,7 +410,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, "",
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -427,13 +429,13 @@ func TestFmt(t *testing.T) {
 				q:    `"sentinel": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
 					`runtime.goexit\n`, `runtime/.*:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -453,7 +455,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, "",
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -473,7 +475,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -496,7 +498,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, "",
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -516,13 +518,13 @@ func TestFmt(t *testing.T) {
 				q:    `"clues wrap": "sentinel": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
 					`runtime.goexit\n`, `runtime/.*:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -543,7 +545,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, "",
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -564,7 +566,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`an error\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -589,7 +591,7 @@ func TestFmt(t *testing.T) {
 					`an error\n`, "",
 					`clues wrap\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -608,14 +610,14 @@ func TestFmt(t *testing.T) {
 				q:    `"sentinel": "clues wrap": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
 					`runtime.goexit\n`, `runtime/.*:\d+\n`,
 					`clues wrap\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -636,7 +638,7 @@ func TestFmt(t *testing.T) {
 					`an error\n`, "",
 					`clues wrap\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -657,7 +659,7 @@ func TestFmt(t *testing.T) {
 					`an error\n`, `err_fmt_test.go:\d+\n`,
 					`clues wrap\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -689,7 +691,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, ``,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -724,7 +726,7 @@ func TestFmt(t *testing.T) {
 					`testing.tRunner\n`, `testing.go:\d+\n`,
 					`runtime.goexit\n`, `runtime/.*:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -763,7 +765,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, ``,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -795,7 +797,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -831,7 +833,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, ``,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -868,7 +870,7 @@ func TestFmt(t *testing.T) {
 					`testing.tRunner\n`, `testing.go:\d+\n`,
 					`runtime.goexit\n`, `runtime/.*:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -909,7 +911,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, ``,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -943,7 +945,7 @@ func TestFmt(t *testing.T) {
 				plus: plusRE(
 					`bot\n`, `err_fmt_test.go:\d+\n`,
 					`sentinel\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -1064,7 +1066,7 @@ func TestFmt_nestedFuncs(t *testing.T) {
 				q:    `"top wrap": "mid wrap": "bottom wrap": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -1140,7 +1142,7 @@ func TestFmt_nestedFuncs(t *testing.T) {
 				q:    `"an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,
@@ -1220,7 +1222,7 @@ func TestFmt_nestedFuncs(t *testing.T) {
 				q:    `"top": "mid": "bottom": "an error"`,
 				plus: plusRE(
 					`an error\n`, "",
-					`github.com/alcionai/clues_test.init\n`, `clues/.+go:\d+\n`,
+					`github.com/alcionai/clues_test.init\n`, cluesInitFileLoc,
 					`runtime.doInit1?\n`, `proc.go:\d+\n`,
 					`runtime.doInit\n`, `proc.go:\d+\n`,
 					`runtime.main\n`, `proc.go:\d+\n`,

diff --git a/err_test.go b/err_test.go
@@ -343,6 +343,56 @@ func TestWithClues(t *testing.T) {
 	}
 }
 
+func TestValuePriority(t *testing.T) {
+	table := []struct {
+		name   string
+		err    error
+		expect msa
+	}{
+		{
+			name: "lowest data wins",
+			err: func() error {
+				ctx := clues.Add(context.Background(), "in-ctx", 1)
+				// the last addition to a ctx should take priority
+				ctx = clues.Add(ctx, "in-ctx", 2)
+
+				err := clues.NewWC(ctx, "err").With("in-err", 1)
+				// the first addition to an error should take priority
+				err = clues.StackWC(ctx, err).With("in-err", 2)
+
+				return err
+			}(),
+			expect: msa{"in-ctx": 2, "in-err": 1},
+		},
+		{
+			name: "last stack wins",
+			err: func() error {
+				ctx := clues.Add(context.Background(), "in-ctx", 1)
+				err := clues.NewWC(ctx, "last in stack").With("in-err", 1)
+				err = clues.Stack(
+					clues.New("first in stack").With("in-err", 2),
+					err)
+				return err
+			}(),
+			expect: msa{"in-ctx": 1, "in-err": 1},
+		},
+		{
+			name: ".With wins over ctx",
+			err: func() error {
+				ctx := clues.Add(context.Background(), "k", 1)
+				err := clues.NewWC(ctx, "last in stack").With("k", 2)
+				return err
+			}(),
+			expect: msa{"k": 2},
+		},
+	}
+	for _, test := range table {
+		t.Run(test.name, func(t *testing.T) {
+			mustEquals(t, test.expect, clues.InErr(test.err).Map(), true)
+		})
+	}
+}
+
 func TestUnwrap(t *testing.T) {
 	e := errors.New("cause")
 	we := clues.Wrap(e, "outer")
@@ -505,7 +555,7 @@ func TestErrValues_stacks(t *testing.T) {
 				clues.New("mid").With("k2", "v2"),
 				clues.New("base").With("k", "v3"),
 			),
-			expect: msa{"k": "v", "k2": "v2"},
+			expect: msa{"k": "v3", "k2": "v2"},
 		},
 		{
 			name: "double double",
@@ -539,7 +589,7 @@ func TestErrValues_stacks(t *testing.T) {
 				),
 			),
 			expect: msa{
-				"k":  "v",
+				"k":  "v4",
 				"k2": "v2",
 				"k3": "v3",
 			},
@@ -584,7 +634,7 @@ func TestErrValues_stacks(t *testing.T) {
 					"right-stack"),
 			),
 			expect: msa{
-				"k":  "v",
+				"k":  "v4",
 				"k2": "v2",
 				"k3": "v3",
 			},