[errors] de-specialize Transient in favor of Tags.

common/errors/annotate.go

Index: common/errors/annotate.go
diff --git a/common/errors/annotate.go b/common/errors/annotate.go
index 6d7707c87b2391ed109c5bd64740f3e21d212daf..604169df35eaf43c51a6971f2e863ab602648eda 100644
--- a/common/errors/annotate.go
+++ b/common/errors/annotate.go
@@ -24,13 +24,18 @@ import (
 	"github.com/luci/luci-go/common/runtime/goroutine"
 )
 
-// Datum is a single data entry value for StackContext.Data.
+// Datum is a single data entry value for stackContext.Data.
+//
+// It's a tuple of Value (the actual data value you care about), and
+// StackFormat, which is a fmt-style string for how this Datum should be
+// rendered when using RenderStack. If StackFormat is left empty, "%#v" will be
+// used.
 type Datum struct {
 	Value       interface{}
 	StackFormat string
 }
 
-// Data is used to add data to a StackContext.
+// Data is used to add data when Annotate'ing an error.
 type Data map[string]Datum
 
 type stack struct {
@@ -63,23 +68,23 @@ func (s *stack) findPointOfDivergence(other *stack) int {
 	return myIdx
 }
 
-// StackContexter is the interface that an error may implement if it has data
+// stackContexter is the interface that an error may implement if it has data
 // associated with a specific stack frame.
-type StackContexter interface {
-	StackContext() StackContext
+type stackContexter interface {
+	stackContext() stackContext
 }
 
-// StackFrameInfo holds a stack and an index into that stack for association
-// with StackContexts.
-type StackFrameInfo struct {
+// stackFrameInfo holds a stack and an index into that stack for association
+// with stackContexts.
+type stackFrameInfo struct {
 	frameIdx int
 	forStack *stack
 }
 
-// StackContext represents the annotation data associated with an error, or an
+// stackContext represents the annotation data associated with an error, or an
 // annotation of an error.
-type StackContext struct {
-	FrameInfo StackFrameInfo
+type stackContext struct {
+	FrameInfo stackFrameInfo

[dnj, 2017/06/24 14:53:54]

Let's make these internal members (lower-case)?

[iannucci, 2017/06/24 20:16:09]

Done.

 	// Reason is the publicly-facing reason, and will show up in the Error()
 	// string.
 	Reason string
@@ -89,7 +94,7 @@ type StackContext struct {
 	InternalReason string
 	Data           Data
 
-	Transient bool
+	Tags map[tagKey]interface{}
 }
 
 // We're looking for %(sometext) which is not preceded by a %. sometext may be
@@ -138,7 +143,7 @@ func (d Data) Format(format string) string {
 
 // RenderPublic renders the public error.Error()-style string for this frame,
 // using the Reason and Data to produce a human readable string.
-func (s *StackContext) RenderPublic(inner error) string {
+func (s *stackContext) RenderPublic(inner error) string {
 	if s.Reason == "" {
 		if inner != nil {
 			return inner.Error()
@@ -159,7 +164,7 @@ func (s *StackContext) RenderPublic(inner error) string {
 //  reason: "The literal content of the Reason field: %(key2)d"
 //  "key1" = "value"
 //  "key2" = 10
-func (s *StackContext) render() Lines {
+func (s *stackContext) render() Lines {
 	siz := len(s.Data)
 	if s.InternalReason != "" {
 		siz++
@@ -179,8 +184,8 @@ func (s *StackContext) render() Lines {
 	if s.Reason != "" {
 		ret = append(ret, fmt.Sprintf("reason: %q", s.Data.Format(s.Reason)))
 	}
-	if s.Transient {
-		ret = append(ret, "transient: true")
+	for key, val := range s.Tags {
+		ret = append(ret, fmt.Sprintf("tag[%q]: %#v", getTagDetail(key).description, val))
 	}
 
 	if len(s.Data) > 0 {
@@ -198,7 +203,7 @@ func (s *StackContext) render() Lines {
 }
 
 // AddData does a 'dict.update' addition of the data.
-func (s *StackContext) AddData(data Data) {
+func (s *stackContext) AddData(data Data) {

[dnj, 2017/06/24 14:53:54]

nit: Let's lower-case these method names too?

[iannucci, 2017/06/24 20:16:09]

Done.

 	if s.Data == nil {
 		s.Data = make(Data, len(data))
 	}
@@ -208,7 +213,7 @@ func (s *StackContext) AddData(data Data) {
 }
 
 // AddDatum adds a single data item to the Data in this frame
-func (s *StackContext) AddDatum(key string, value interface{}, format string) {
+func (s *stackContext) AddDatum(key string, value interface{}, format string) {
 	if s.Data == nil {
 		s.Data = Data{key: {value, format}}
 	} else {
@@ -218,31 +223,33 @@ func (s *StackContext) AddDatum(key string, value interface{}, format string) {
 
 type terminalStackError struct {
 	error
-	finfo StackFrameInfo
+	finfo stackFrameInfo
+	tags  map[tagKey]interface{}
 }
 
 var _ interface {
 	error
-	StackContexter
+	stackContexter
 } = (*terminalStackError)(nil)
 
-func (e *terminalStackError) StackContext() StackContext { return StackContext{FrameInfo: e.finfo} }
+func (e *terminalStackError) stackContext() stackContext {
+	return stackContext{FrameInfo: e.finfo, Tags: e.tags}
+}
 
 type annotatedError struct {
 	inner error
-	ctx   StackContext
+	ctx   stackContext
 }
 
 var _ interface {
 	error
-	StackContexter
+	stackContexter
 	Wrapped
 } = (*annotatedError)(nil)
 
 func (e *annotatedError) Error() string              { return e.ctx.RenderPublic(e.inner) }
-func (e *annotatedError) StackContext() StackContext { return e.ctx }
+func (e *annotatedError) stackContext() stackContext { return e.ctx }
 func (e *annotatedError) InnerError() error          { return e.inner }
-func (e *annotatedError) IsTransient() bool          { return e.ctx.Transient }
 
 // Annotator is a builder for annotating errors. Obtain one by calling Annotate
 // on an existing error or using Reason.
@@ -250,7 +257,7 @@ func (e *annotatedError) IsTransient() bool          { return e.ctx.Transient }
 // See the example test for Annotate to see how this is meant to be used.
 type Annotator struct {
 	inner error
-	ctx   StackContext
+	ctx   stackContext
 }
 
 // Reason adds a PUBLICLY READABLE reason string (for humans) to this error.
@@ -315,14 +322,22 @@ func (a *Annotator) Data(data Data) *Annotator {
 	return a
 }
 
-// Transient marks this error as transient. If the inner error is already
-// transient, this has no effect.
-func (a *Annotator) Transient() *Annotator {
+// Tag adds a tag with an optional value to this error.
+//
+// `value` is a uniary optional argument, and must be a simple type (i.e. has

[dnj, 2017/06/24 14:53:54]

nit: "unary"

[iannucci, 2017/06/24 20:16:09]

Done.

+// a reflect.Kind which is a base data type like bool, string, or int).
+func (a *Annotator) Tag(tags ...TagValue) *Annotator {
 	if a == nil {
 		return a
 	}
-	if !IsTransient(a.inner) {
-		a.ctx.Transient = true
+	for _, t := range tags {
+		if t == nil {
+			continue
+		}
+		if a.ctx.Tags == nil {
+			a.ctx.Tags = make(map[tagKey]interface{}, len(tags))
+		}
+		a.ctx.Tags[t.getKey()] = t.getValue()
 	}
 	return a
 }
@@ -538,7 +553,7 @@ func RenderStack(err error) *RenderedError {
 
 	lastAnnotatedFrame := 0
 	var wrappers = []Lines{}
-	getCurFrame := func(fi *StackFrameInfo) *RenderedFrame {
+	getCurFrame := func(fi *stackFrameInfo) *RenderedFrame {
 		if len(ret.Stacks) == 0 || ret.Stacks[len(ret.Stacks)-1].GoID != fi.forStack.id {
 			lastAnnotatedFrame = len(fi.forStack.frames) - 1
 			toAdd := &RenderedStack{
@@ -565,8 +580,8 @@ func RenderStack(err error) *RenderedError {
 	}
 
 	for err != nil {
-		if sc, ok := err.(StackContexter); ok {
-			ctx := sc.StackContext()
+		if sc, ok := err.(stackContexter); ok {
+			ctx := sc.stackContext()
 			if stk := ctx.FrameInfo.forStack; stk != nil {
 				frm := getCurFrame(&ctx.FrameInfo)
 				if rendered := ctx.render(); len(rendered) > 0 {
@@ -581,7 +596,7 @@ func RenderStack(err error) *RenderedError {
 		switch x := err.(type) {
 		case MultiError:
 			// TODO(riannucci): it's kinda dumb that we have to walk the MultiError
-			// twice (once in its StackContext method, and again here).
+			// twice (once in its stackContext method, and again here).
 			err = x.First()
 		case Wrapped:
 			err = x.InnerError()
@@ -613,7 +628,7 @@ func Annotate(err error) *Annotator {
 	if err == nil {
 		return nil
 	}
-	return &Annotator{err, StackContext{FrameInfo: StackFrameInfoForError(1, err)}}
+	return &Annotator{err, stackContext{FrameInfo: stackFrameInfoForError(1, err)}}
 }
 
 // Reason builds a new Annotator starting with reason. This allows you to use
@@ -625,16 +640,23 @@ func Annotate(err error) *Annotator {
 // Prefer this form to errors.New(fmt.Sprintf("..."))
 func Reason(reason string) *Annotator {
 	currentStack := captureStack(1)
-	frameInfo := StackFrameInfo{0, currentStack}
-	return (&Annotator{nil, StackContext{FrameInfo: frameInfo}}).Reason(reason)
+	frameInfo := stackFrameInfo{0, currentStack}
+	return (&Annotator{nil, stackContext{FrameInfo: frameInfo}}).Reason(reason)
 }
 
 // New is an API-compatible version of the standard errors.New function. Unlike
 // the stdlib errors.New, this will capture the current stack information at the
 // place this error was created.
-func New(msg string) error {
-	return &terminalStackError{errors.New(msg),
-		StackFrameInfo{forStack: captureStack(1)}}
+func New(msg string, tags ...TagValue) error {
+	tse := &terminalStackError{
+		errors.New(msg), stackFrameInfo{forStack: captureStack(1)}, nil}
+	if len(tags) > 0 {
+		tse.tags = make(map[tagKey]interface{}, len(tags))

[dnj, 2017/06/24 14:53:54]

Handle all of the cases that "Tag" handles (e.g., t == nil).

[iannucci, 2017/06/24 20:16:09]

Done.

+		for _, t := range tags {
+			tse.tags[t.getKey()] = t.getValue()
+		}
+	}
+	return tse
 }
 
 func captureStack(skip int) *stack {
@@ -651,8 +673,8 @@ func captureStack(skip int) *stack {
 
 func getCapturedStack(err error) (ret *stack) {
 	Walk(err, func(err error) bool {
-		if sc, ok := err.(StackContexter); ok {
-			ret = sc.StackContext().FrameInfo.forStack
+		if sc, ok := err.(stackContexter); ok {
+			ret = sc.stackContext().FrameInfo.forStack
 			return false
 		}
 		return true
@@ -660,46 +682,26 @@ func getCapturedStack(err error) (ret *stack) {
 	return
 }
 
-// StackFrameInfoForError returns a StackFrameInfo suitable for use to implement
-// the StackContexter interface.
+// stackFrameInfoForError returns a stackFrameInfo suitable for use to implement
+// the stackContexter interface.
 //
 // It skips the provided number of frames when collecting the current trace
 // (which should be equal to the number of functions between your error library
 // and the user's code).
 //
-// The returned StackFrameInfo will find the appropriate frame in the error's
+// The returned stackFrameInfo will find the appropriate frame in the error's
 // existing stack information (if the error was created with errors.New), or
 // include the current stack if it was not.
-func StackFrameInfoForError(skip int, err error) StackFrameInfo {
+func stackFrameInfoForError(skip int, err error) stackFrameInfo {
 	currentStack := captureStack(skip + 1)
 	currentlyCapturedStack := getCapturedStack(err)
 	if currentlyCapturedStack == nil || currentStack.id != currentlyCapturedStack.id {
 		// This is the very first annotation on this error OR
 		// We switched goroutines.
-		return StackFrameInfo{forStack: currentStack}
+		return stackFrameInfo{forStack: currentStack}
 	}
-	return StackFrameInfo{
+	return stackFrameInfo{
 		frameIdx: currentlyCapturedStack.findPointOfDivergence(currentStack),
 		forStack: currentlyCapturedStack,
 	}
 }
-
-// ExtractData walks the error and extracts the given key's data from
-// Annotations (e.g. data added with D(key, <value>) will return <value>).
-//
-// The first matching key encountered (e.g. highest up the callstack) will be
-// returned.
-//
-// If the error does not contain key at all, this returns nil.
-func ExtractData(err error, key string) (ret interface{}) {
-	Walk(err, func(err error) bool {
-		if sc, ok := err.(StackContexter); ok {
-			if d, ok := sc.StackContext().Data[key]; ok {
-				ret = d.Value
-				return false
-			}
-		}
-		return true
-	})
-	return
-}