remove local bus struct (#4435)

removes the local bus from fault in favor of a single Bus that operates both within a local and global instance, and can be passed downstream independent of context.  Also includes some code separation in the fault package for readability.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/pkg/fault/alert.go

@@ -0,0 +1,70 @@
+package fault
+
+import (
+	"github.com/alcionai/corso/src/cli/print"
+)
+
+var _ print.Printable = &Alert{}
+
+// Alerts are informational-only notifications.  The purpose of alerts is to
+// provide a means of end-user communication about important events without
+// needing to generate runtime failures or recoverable errors. When generating
+// an alert, no other fault feature (failure, recoverable, skip, etc) should
+// be in use.  IE: Errors do not also get alerts, since the error itself is a
+// form of end-user communication already.
+type Alert struct {
+	Item    Item   `json:"item"`
+	Message string `json:"message"`
+}
+
+// String complies with the stringer interface.
+func (a *Alert) String() string {
+	msg := "<nil>"
+
+	if a != nil {
+		msg = a.Message
+	}
+
+	if len(msg) == 0 {
+		msg = "<missing>"
+	}
+
+	return "Alert: " + msg
+}
+
+func (a Alert) MinimumPrintable() any {
+	return a
+}
+
+// Headers returns the human-readable names of properties of a skipped Item
+// for printing out to a terminal.
+func (a Alert) Headers() []string {
+	return []string{"Action", "Message", "Container", "Name", "ID"}
+}
+
+// Values populates the printable values matching the Headers list.
+func (a Alert) Values() []string {
+	var cn string
+
+	acn, ok := a.Item.Additional[AddtlContainerName]
+	if ok {
+		str, ok := acn.(string)
+		if ok {
+			cn = str
+		}
+	}
+
+	return []string{"Alert", a.Message, cn, a.Item.Name, a.Item.ID}
+}
+
+func NewAlert(message, namespace, itemID, name string, addtl map[string]any) *Alert {
+	return &Alert{
+		Message: message,
+		Item: Item{
+			Namespace:  namespace,
+			ID:         itemID,
+			Name:       name,
+			Additional: addtl,
+		},
+	}
+}

src/pkg/fault/alert_test.go

@@ -0,0 +1,88 @@
+package fault_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/alcionai/corso/src/internal/tester"
+	"github.com/alcionai/corso/src/pkg/fault"
+)
+
+type AlertUnitSuite struct {
+	tester.Suite
+}
+
+func TestAlertUnitSuite(t *testing.T) {
+	suite.Run(t, &AlertUnitSuite{Suite: tester.NewUnitSuite(t)})
+}
+
+func (suite *AlertUnitSuite) TestAlert_String() {
+	var (
+		t = suite.T()
+		a fault.Alert
+	)
+
+	assert.Contains(t, a.String(), "Alert: <missing>")
+
+	a = fault.Alert{
+		Item:    fault.Item{},
+		Message: "",
+	}
+	assert.Contains(t, a.String(), "Alert: <missing>")
+
+	a = fault.Alert{
+		Item: fault.Item{
+			ID: "item_id",
+		},
+		Message: "msg",
+	}
+	assert.NotContains(t, a.String(), "item_id")
+	assert.Contains(t, a.String(), "Alert: msg")
+}
+
+func (suite *AlertUnitSuite) TestNewAlert() {
+	t := suite.T()
+	addtl := map[string]any{"foo": "bar"}
+	a := fault.NewAlert("message-to-show", "ns", "item_id", "item_name", addtl)
+
+	expect := fault.Alert{
+		Item: fault.Item{
+			Namespace:  "ns",
+			ID:         "item_id",
+			Name:       "item_name",
+			Additional: addtl,
+		},
+		Message: "message-to-show",
+	}
+
+	assert.Equal(t, expect, *a)
+}
+
+func (suite *AlertUnitSuite) TestAlert_HeadersValues() {
+	addtl := map[string]any{
+		fault.AddtlContainerID:   "cid",
+		fault.AddtlContainerName: "cname",
+	}
+
+	table := []struct {
+		name   string
+		alert  *fault.Alert
+		expect []string
+	}{
+		{
+			name:   "new alert",
+			alert:  fault.NewAlert("message-to-show", "ns", "id", "name", addtl),
+			expect: []string{"Alert", "message-to-show", "cname", "name", "id"},
+		},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			t := suite.T()
+
+			assert.Equal(t, []string{"Action", "Message", "Container", "Name", "ID"}, test.alert.Headers())
+			assert.Equal(t, test.expect, test.alert.Values())
+		})
+	}
+}

src/pkg/fault/example_fault_test.go

@@ -441,3 +441,28 @@ func ExampleBus_AddSkip() {
 
 	// Output: skipped processing file: malware_detected
 }
+
+// ExampleBus_AddAlert showcases when to use AddAlert.
+func ExampleBus_AddAlert() {
+	errs := fault.New(false)
+
+	// Some events should be communicated to the end user without recording an
+	// error to the operation.  Logs aren't sufficient because we don't promote
+	// log messages to the terminal.  But errors and skips are too heavy and hacky
+	// to use.  In these cases, we can create informational Alerts.
+	//
+	// Only the message gets shown to the user.  But since we're persisting this
+	// data along with the backup details and other fault info, we have the option
+	// of packing any other contextual data that we want.
+	errs.AddAlert(ctx, fault.NewAlert(
+		"something important happened!",
+		"deduplication-namespace",
+		"file-id",
+		"file-name",
+		map[string]any{"foo": "bar"}))
+
+	// later on, after processing, end users can scrutinize the alerts.
+	fmt.Println(errs.Alerts()[0].String())
+
+	// Alert: something important happened!
+}

src/pkg/fault/fault.go

@@ -15,11 +15,24 @@ import (
 	"github.com/alcionai/corso/src/pkg/logger"
 )
 
+// temporary hack identifier
+// see: https://github.com/alcionai/corso/pull/2510#discussion_r1113532530
+// TODO: https://github.com/alcionai/corso/issues/4003
+const LabelForceNoBackupCreation = "label_forces_no_backup_creations"
+
 type Bus struct {
 	mu *sync.Mutex
 
+	// When creating a local bus, the parent property retains a pointer
+	// to the root Bus.  Even in the case of multiple chained creations of
+	// local busses, the parent reference remains the original root bus,
+	// and does not create a linked list of lineage.  Any errors and failures
+	// created by a local instance will get fielded to the parent.  But only
+	// local errors will returned by property getter funcs.
+	parent *Bus
+
 	// Failure probably identifies errors that were added to the bus
-	// or localBus via AddRecoverable, but which were promoted
+	// or a local Bus via AddRecoverable, but which were promoted
 	// to the failure position due to failFast=true configuration.
 	// Alternatively, the process controller might have set failure
 	// by calling Fail(err).
@@ -58,67 +71,61 @@ func New(failFast bool) *Bus {
 	}
 }
 
+// Local constructs a new bus with a local reference to handle error aggregation
+// in a constrained scope.  This allows the caller to review recoverable errors and
+// failures within only the current codespace, as opposed to the global set of errors.
+// The function that spawned the local bus should always return `bus.Failure()` to
+// ensure that hard failures are propagated back upstream.
+func (e *Bus) Local() *Bus {
+	parent := e.parent
+
+	// only use e if it is already the root instance
+	if parent == nil {
+		parent = e
+	}
+
+	return &Bus{
+		mu:       &sync.Mutex{},
+		parent:   parent,
+		failFast: parent.failFast,
+	}
+}
+
 // FailFast returns the failFast flag in the bus.
 func (e *Bus) FailFast() bool {
 	return e.failFast
 }
 
-// Failure returns the primary error.  If not nil, this
-// indicates the operation exited prior to completion.
-func (e *Bus) Failure() error {
-	return e.failure
-}
-
-// Recovered returns the slice of errors that occurred in
-// recoverable points of processing.  This is often during
-// iteration where a single failure (ex: retrieving an item),
-// doesn't require the entire process to end.
-func (e *Bus) Recovered() []error {
-	return slices.Clone(e.recoverable)
-}
-
-// Skipped returns the slice of items that were permanently
-// skipped during processing.
-func (e *Bus) Skipped() []Skipped {
-	return slices.Clone(e.skipped)
-}
-
-// Alerts returns the slice of alerts generated during runtime.
-func (e *Bus) Alerts() []Alert {
-	return slices.Clone(e.alerts)
-}
-
 // Fail sets the non-recoverable error (ie: bus.failure)
 // in the bus.  If a failure error is already present,
 // the error gets added to the recoverable slice for
 // purposes of tracking.
-//
-// TODO: Return Data, not Bus.  The consumers of a failure
-// should care about the state of data, not the communication
-// pattern.
 func (e *Bus) Fail(err error) *Bus {
 	if err == nil {
 		return e
 	}
 
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
 	return e.setFailure(err)
 }
 
 // setErr handles setting bus.failure.  Sync locking gets
 // handled upstream of this call.
 func (e *Bus) setFailure(err error) *Bus {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
 	if e.failure == nil {
 		e.failure = err
-		return e
+	} else {
-	}
-
 		// technically not a recoverable error: we're using the
 		// recoverable slice as an overflow container here to
 		// ensure everything is tracked.
 		e.recoverable = append(e.recoverable, err)
+	}
+
+	if e.parent != nil {
+		e.parent.setFailure(err)
+	}
 
 	return e
 }
@@ -127,17 +134,11 @@ func (e *Bus) setFailure(err error) *Bus {
 // errors (ie: bus.recoverable).  If failFast is true, the first
 // added error will get copied to bus.failure, causing the bus
 // to identify as non-recoverably failed.
-//
-// TODO: nil return, not Bus, since we don't want people to return
-// from errors.AddRecoverable().
 func (e *Bus) AddRecoverable(ctx context.Context, err error) {
 	if err == nil {
 		return
 	}
 
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
 	e.logAndAddRecoverable(ctx, err, 1)
 }
 
@@ -158,19 +159,77 @@ func (e *Bus) logAndAddRecoverable(ctx context.Context, err error, skip int) {
 // gets handled upstream of this call.  Returns true if the
 // error is a failure, false otherwise.
 func (e *Bus) addRecoverableErr(err error) bool {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
 	var isFail bool
 
 	if e.failure == nil && e.failFast {
-		e.setFailure(err)
+		if e.failure == nil {
+			e.failure = err
+		} else {
+			// technically not a recoverable error: we're using the
+			// recoverable slice as an overflow container here to
+			// ensure everything is tracked.
+			e.recoverable = append(e.recoverable, err)
+		}
+
+		if e.parent != nil {
+			e.parent.setFailure(err)
+		}
 
 		isFail = true
 	}
 
 	e.recoverable = append(e.recoverable, err)
 
+	// local bus instances must promote errors to the root bus.
+	if e.parent != nil {
+		e.parent.addRecoverableErr(err)
+	}
+
 	return isFail
 }
 
+// ---------------------------------------------------------------------------
+// Non-error adders
+// ---------------------------------------------------------------------------
+
+// AddAlert appends a record of an Alert message to the fault bus.
+// Importantly, alerts are not errors, exceptions, or skipped items.
+// An alert should only be generated if no other fault functionality
+// is in use, but that we still want the end user to clearly and
+// plainly receive a notification about a runtime event.
+func (e *Bus) AddAlert(ctx context.Context, a *Alert) {
+	if a == nil {
+		return
+	}
+
+	e.logAndAddAlert(ctx, a, 1)
+}
+
+// logs the error and adds an alert.
+func (e *Bus) logAndAddAlert(ctx context.Context, a *Alert, trace int) {
+	logger.CtxStack(ctx, trace+1).
+		With("alert", a).
+		Info("alert: " + a.Message)
+	e.addAlert(a)
+}
+
+func (e *Bus) addAlert(a *Alert) *Bus {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+
+	e.alerts = append(e.alerts, *a)
+
+	// local bus instances must promote alerts to the root bus.
+	if e.parent != nil {
+		e.parent.addAlert(a)
+	}
+
+	return e
+}
+
 // AddSkip appends a record of a Skipped item to the fault bus.
 // Importantly, skipped items are not the same as recoverable
 // errors.  An item should only be skipped under the following
@@ -186,9 +245,6 @@ func (e *Bus) AddSkip(ctx context.Context, s *Skipped) {
 		return
 	}
 
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
 	e.logAndAddSkip(ctx, s, 1)
 }
 
@@ -196,44 +252,28 @@ func (e *Bus) AddSkip(ctx context.Context, s *Skipped) {
 func (e *Bus) logAndAddSkip(ctx context.Context, s *Skipped, trace int) {
 	logger.CtxStack(ctx, trace+1).
 		With("skipped", s).
-		Info("skipped item")
+		Info("skipped an item")
 	e.addSkip(s)
 }
 
 func (e *Bus) addSkip(s *Skipped) *Bus {
-	e.skipped = append(e.skipped, *s)
-	return e
-}
-
-// AddAlert appends a record of an Alert message to the fault bus.
-// Importantly, alerts are not errors, exceptions, or skipped items.
-// An alert should only be generated if no other fault functionality
-// is in use, but that we still want the end user to clearly and
-// plainly receive a notification about a runtime event.
-func (e *Bus) AddAlert(ctx context.Context, a *Alert) {
-	if a == nil {
-		return
-	}
-
 	e.mu.Lock()
 	defer e.mu.Unlock()
 
-	e.logAndAddAlert(ctx, a, 1)
+	e.skipped = append(e.skipped, *s)
-}
 
-// logs the error and adds an alert.
+	// local bus instances must promote skipped items to the root bus.
-func (e *Bus) logAndAddAlert(ctx context.Context, a *Alert, trace int) {
+	if e.parent != nil {
-	logger.CtxStack(ctx, trace+1).
+		e.parent.addSkip(s)
-		With("alert", a).
+	}
-		Info("alert: " + a.Message)
-	e.addAlert(a)
-}
 
-func (e *Bus) addAlert(a *Alert) *Bus {
-	e.alerts = append(e.alerts, *a)
 	return e
 }
 
+// ---------------------------------------------------------------------------
+// Results
+// ---------------------------------------------------------------------------
+
 // Errors returns the plain record of errors that were aggregated
 // within a fult Bus.
 func (e *Bus) Errors() *Errors {
@@ -249,6 +289,39 @@ func (e *Bus) Errors() *Errors {
 	}
 }
 
+// Failure returns the primary error.  If not nil, this
+// indicates the operation exited prior to completion.
+// If the bus is a local instance, this only returns the
+// local failure, and will not return parent data.
+func (e *Bus) Failure() error {
+	return e.failure
+}
+
+// Recovered returns the slice of errors that occurred in
+// recoverable points of processing.  This is often during
+// iteration where a single failure (ex: retrieving an item),
+// doesn't require the entire process to end.
+// If the bus is a local instance, this only returns the
+// local recovered errors, and will not return parent data.
+func (e *Bus) Recovered() []error {
+	return slices.Clone(e.recoverable)
+}
+
+// Skipped returns the slice of items that were permanently
+// skipped during processing.
+// If the bus is a local instance, this only returns the
+// local skipped items, and will not return parent data.
+func (e *Bus) Skipped() []Skipped {
+	return slices.Clone(e.skipped)
+}
+
+// Alerts returns the slice of alerts generated during runtime.
+// If the bus is a local alerts, this only returns the
+// local failure, and will not return parent data.
+func (e *Bus) Alerts() []Alert {
+	return slices.Clone(e.alerts)
+}
+
 // ItemsAndRecovered returns the items that failed along with other
 // recoverable errors
 func (e *Bus) ItemsAndRecovered() ([]Item, []error) {
@@ -275,10 +348,6 @@ func (e *Bus) ItemsAndRecovered() ([]Item, []error) {
 	return maps.Values(is), non
 }
 
-// ---------------------------------------------------------------------------
-// Errors Data
-// ---------------------------------------------------------------------------
-
 // Errors provides the errors data alone, without sync controls
 // or adders/setters.  Expected to get called at the end of processing,
 // as a way to aggregate results.
@@ -360,6 +429,10 @@ func UnmarshalErrorsTo(e *Errors) func(io.ReadCloser) error {
 	}
 }
 
+// ---------------------------------------------------------------------------
+// Print compatibility
+// ---------------------------------------------------------------------------
+
 // Print writes the DetailModel Entries to StdOut, in the format
 // requested by the caller.
 func (e *Errors) PrintItems(
@@ -430,73 +503,3 @@ func (pec printableErrCore) Values() []string {
 
 	return []string{pec.Msg}
 }
-
-// ---------------------------------------------------------------------------
-// Local aggregator
-// ---------------------------------------------------------------------------
-
-// Local constructs a new local bus to handle error aggregation in a
-// constrained scope.  Local busses shouldn't be passed down  to other
-// funcs, and the function that spawned the local bus should always
-// return `local.Failure()` to ensure that hard failures are propagated
-// back upstream.
-func (e *Bus) Local() *LocalBus {
-	return &LocalBus{
-		mu:  &sync.Mutex{},
-		bus: e,
-	}
-}
-
-type LocalBus struct {
-	mu      *sync.Mutex
-	bus     *Bus
-	current error
-}
-
-func (e *LocalBus) AddRecoverable(ctx context.Context, err error) {
-	if err == nil {
-		return
-	}
-
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
-	if e.current == nil && e.bus.failFast {
-		e.current = err
-	}
-
-	e.bus.logAndAddRecoverable(ctx, err, 1)
-}
-
-// AddSkip appends a record of a Skipped item to the local bus.
-// Importantly, skipped items are not the same as recoverable
-// errors.  An item should only be skipped under the following
-// conditions.  All other cases should be handled as errors.
-// 1. The conditions for skipping the item are well-known and
-// well-documented.  End users need to be able to understand
-// both the conditions and identifications of skips.
-// 2. Skipping avoids a permanent and consistent failure.  If
-// the underlying reason is transient or otherwise recoverable,
-// the item should not be skipped.
-func (e *LocalBus) AddSkip(ctx context.Context, s *Skipped) {
-	if s == nil {
-		return
-	}
-
-	e.mu.Lock()
-	defer e.mu.Unlock()
-
-	e.bus.logAndAddSkip(ctx, s, 1)
-}
-
-// Failure returns the failure that happened within the local bus.
-// It does not return the underlying bus.Failure(), only the failure
-// that was recorded within the local bus instance.  This error should
-// get returned by any func which created a local bus.
-func (e *LocalBus) Failure() error {
-	return e.current
-}
-
-// temporary hack identifier
-// see: https://github.com/alcionai/corso/pull/2510#discussion_r1113532530
-const LabelForceNoBackupCreation = "label_forces_no_backup_creations"

src/pkg/fault/fault_test.go

@@ -189,25 +189,6 @@ func (suite *FaultErrorsUnitSuite) TestAdd() {
 	assert.Len(t, n.Recovered(), 2)
 }
 
-func (suite *FaultErrorsUnitSuite) TestAddSkip() {
-	t := suite.T()
-
-	ctx, flush := tester.NewContext(t)
-	defer flush()
-
-	n := fault.New(true)
-	require.NotNil(t, n)
-
-	n.Fail(assert.AnError)
-	assert.Len(t, n.Skipped(), 0)
-
-	n.AddRecoverable(ctx, assert.AnError)
-	assert.Len(t, n.Skipped(), 0)
-
-	n.AddSkip(ctx, fault.OwnerSkip(fault.SkipMalware, "ns", "id", "name", nil))
-	assert.Len(t, n.Skipped(), 1)
-}
-
 func (suite *FaultErrorsUnitSuite) TestErrors() {
 	t := suite.T()
 

src/pkg/fault/item.go

@@ -11,15 +11,15 @@ const (
 	AddtlMalwareDesc   = "malware_description"
 )
 
-type itemType string
+type ItemType string
 
 const (
-	FileType          itemType = "file"
+	FileType          ItemType = "file"
-	ContainerType     itemType = "container"
+	ContainerType     ItemType = "container"
-	ResourceOwnerType itemType = "resource_owner"
+	ResourceOwnerType ItemType = "resource_owner"
 )
 
-func (it itemType) Printable() string {
+func (it ItemType) Printable() string {
 	switch it {
 	case FileType:
 		return "File"
@@ -62,7 +62,7 @@ type Item struct {
 	Name string `json:"name"`
 
 	// tracks the type of item represented by this entry.
-	Type itemType `json:"type"`
+	Type ItemType `json:"type"`
 
 	// Error() of the causal error, or a sentinel if this is the
 	// source of the error.  In case of ID collisions, the first
@@ -138,7 +138,7 @@ func OwnerErr(cause error, namespace, id, name string, addtl map[string]any) *It
 }
 
 // itemErr produces a Item of the provided type for tracking erroneous items.
-func itemErr(t itemType, cause error, namespace, id, name string, addtl map[string]any) *Item {
+func itemErr(t ItemType, cause error, namespace, id, name string, addtl map[string]any) *Item {
 	return &Item{
 		Namespace:  namespace,
 		ID:         id,
@@ -148,183 +148,3 @@ func itemErr(t itemType, cause error, namespace, id, name string, addtl map[stri
 		Additional: addtl,
 	}
 }
-
-// ---------------------------------------------------------------------------
-// Skipped Items
-// ---------------------------------------------------------------------------
-
-// skipCause identifies the well-known conditions to Skip an item.  It is
-// important that skip cause enumerations do not overlap with general error
-// handling.  Skips must be well known, well documented, and consistent.
-// Transient failures, undocumented or unknown conditions, and arbitrary
-// handling should never produce a skipped item. Those cases should get
-// handled as normal errors.
-type skipCause string
-
-const (
-	// SkipMalware identifies a malware detection case.  Files that graph
-	// api identifies as malware cannot be downloaded or uploaded, and will
-	// permanently fail any attempts to backup or restore.
-	SkipMalware skipCause = "malware_detected"
-
-	// SkipBigOneNote identifies that a file was skipped because it
-	// was big OneNote file and we can only download OneNote files which
-	// are less that 2GB in size.
-	//nolint:lll
-	// https://support.microsoft.com/en-us/office/restrictions-and-limitations-in-onedrive-and-sharepoint-64883a5d-228e-48f5-b3d2-eb39e07630fa#onenotenotebooks
-	SkipBigOneNote skipCause = "big_one_note_file"
-)
-
-var _ print.Printable = &Skipped{}
-
-// Skipped items are permanently unprocessable due to well-known conditions.
-// In order to skip an item, the following conditions should be met:
-// 1. The conditions for skipping the item are well-known and
-// well-documented.  End users need to be able to understand
-// both the conditions and identifications of skips.
-// 2. Skipping avoids a permanent and consistent failure.  If
-// the underlying reason is transient or otherwise recoverable,
-// the item should not be skipped.
-//
-// Skipped wraps Item primarily to minimize confusion when sharing the
-// fault interface.  Skipped items are not errors, and Item{} errors are
-// not the basis for a Skip.
-type Skipped struct {
-	Item Item `json:"item"`
-}
-
-// String complies with the stringer interface.
-func (s *Skipped) String() string {
-	if s == nil {
-		return "<nil>"
-	}
-
-	return "skipped " + s.Item.Error() + ": " + s.Item.Cause
-}
-
-// HasCause compares the underlying cause against the parameter.
-func (s *Skipped) HasCause(c skipCause) bool {
-	if s == nil {
-		return false
-	}
-
-	return s.Item.Cause == string(c)
-}
-
-func (s Skipped) MinimumPrintable() any {
-	return s
-}
-
-// Headers returns the human-readable names of properties of a skipped Item
-// for printing out to a terminal.
-func (s Skipped) Headers() []string {
-	return []string{"Action", "Type", "Name", "Container", "Cause"}
-}
-
-// Values populates the printable values matching the Headers list.
-func (s Skipped) Values() []string {
-	var cn string
-
-	acn, ok := s.Item.Additional[AddtlContainerName]
-	if ok {
-		str, ok := acn.(string)
-		if ok {
-			cn = str
-		}
-	}
-
-	return []string{"Skip", s.Item.Type.Printable(), s.Item.Name, cn, s.Item.Cause}
-}
-
-// ContainerSkip produces a Container-kind Item for tracking skipped items.
-func ContainerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
-	return itemSkip(ContainerType, cause, namespace, id, name, addtl)
-}
-
-// FileSkip produces a File-kind Item for tracking skipped items.
-func FileSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
-	return itemSkip(FileType, cause, namespace, id, name, addtl)
-}
-
-// OnwerSkip produces a ResourceOwner-kind Item for tracking skipped items.
-func OwnerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
-	return itemSkip(ResourceOwnerType, cause, namespace, id, name, addtl)
-}
-
-// itemSkip produces a Item of the provided type for tracking skipped items.
-func itemSkip(t itemType, cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
-	return &Skipped{
-		Item: Item{
-			Namespace:  namespace,
-			ID:         id,
-			Name:       name,
-			Type:       t,
-			Cause:      string(cause),
-			Additional: addtl,
-		},
-	}
-}
-
-// ---------------------------------------------------------------------------
-// Alerts
-// ---------------------------------------------------------------------------
-
-var _ print.Printable = &Alert{}
-
-// Alerts are informational-only notifications.  The purpose of alerts is to
-// provide a means of end-user communication about important events without
-// needing to generate runtime failures or recoverable errors. When generating
-// an alert, no other fault feature (failure, recoverable, skip, etc) should
-// be in use.  IE: Errors do not also get alerts, since the error itself is a
-// form of end-user communication already.
-type Alert struct {
-	Item    Item   `json:"item"`
-	Message string `json:"message"`
-}
-
-// String complies with the stringer interface.
-func (a Alert) String() string {
-	msg := a.Message
-	if len(msg) == 0 {
-		msg = "<nil>"
-	}
-
-	return "Alert: " + msg
-}
-
-func (a Alert) MinimumPrintable() any {
-	return a
-}
-
-// Headers returns the human-readable names of properties of a skipped Item
-// for printing out to a terminal.
-func (a Alert) Headers() []string {
-	return []string{"Action", "Message", "Container", "Name", "ID"}
-}
-
-// Values populates the printable values matching the Headers list.
-func (a Alert) Values() []string {
-	var cn string
-
-	acn, ok := a.Item.Additional[AddtlContainerName]
-	if ok {
-		str, ok := acn.(string)
-		if ok {
-			cn = str
-		}
-	}
-
-	return []string{"Alert", a.Message, cn, a.Item.Name, a.Item.ID}
-}
-
-func NewAlert(message, namespace, itemID, name string, addtl map[string]any) *Alert {
-	return &Alert{
-		Message: message,
-		Item: Item{
-			Namespace:  namespace,
-			ID:         itemID,
-			Name:       name,
-			Additional: addtl,
-		},
-	}
-}

src/pkg/fault/item_test.go

@@ -1,4 +1,4 @@
-package fault
+package fault_test
 
 import (
 	"testing"
@@ -8,6 +8,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/alcionai/corso/src/internal/tester"
+	"github.com/alcionai/corso/src/pkg/fault"
 )
 
 type ItemUnitSuite struct {
@@ -21,28 +22,28 @@ func TestItemUnitSuite(t *testing.T) {
 func (suite *ItemUnitSuite) TestItem_Error() {
 	var (
 		t = suite.T()
-		i *Item
+		i *fault.Item
 	)
 
 	assert.Contains(t, i.Error(), "nil")
 
-	i = &Item{}
+	i = &fault.Item{}
 	assert.Contains(t, i.Error(), "unknown type")
 
-	i = &Item{Type: FileType}
+	i = &fault.Item{Type: fault.FileType}
-	assert.Contains(t, i.Error(), FileType)
+	assert.Contains(t, i.Error(), fault.FileType)
 }
 
 func (suite *ItemUnitSuite) TestContainerErr() {
 	t := suite.T()
 	addtl := map[string]any{"foo": "bar"}
-	i := ContainerErr(clues.New("foo"), "ns", "id", "name", addtl)
+	i := fault.ContainerErr(clues.New("foo"), "ns", "id", "name", addtl)
 
-	expect := Item{
+	expect := fault.Item{
 		Namespace:  "ns",
 		ID:         "id",
 		Name:       "name",
-		Type:       ContainerType,
+		Type:       fault.ContainerType,
 		Cause:      "foo",
 		Additional: addtl,
 	}
@@ -53,13 +54,13 @@ func (suite *ItemUnitSuite) TestContainerErr() {
 func (suite *ItemUnitSuite) TestFileErr() {
 	t := suite.T()
 	addtl := map[string]any{"foo": "bar"}
-	i := FileErr(clues.New("foo"), "ns", "id", "name", addtl)
+	i := fault.FileErr(clues.New("foo"), "ns", "id", "name", addtl)
 
-	expect := Item{
+	expect := fault.Item{
 		Namespace:  "ns",
 		ID:         "id",
 		Name:       "name",
-		Type:       FileType,
+		Type:       fault.FileType,
 		Cause:      "foo",
 		Additional: addtl,
 	}
@@ -70,13 +71,13 @@ func (suite *ItemUnitSuite) TestFileErr() {
 func (suite *ItemUnitSuite) TestOwnerErr() {
 	t := suite.T()
 	addtl := map[string]any{"foo": "bar"}
-	i := OwnerErr(clues.New("foo"), "ns", "id", "name", addtl)
+	i := fault.OwnerErr(clues.New("foo"), "ns", "id", "name", addtl)
 
-	expect := Item{
+	expect := fault.Item{
 		Namespace:  "ns",
 		ID:         "id",
 		Name:       "name",
-		Type:       ResourceOwnerType,
+		Type:       fault.ResourceOwnerType,
 		Cause:      "foo",
 		Additional: addtl,
 	}
@@ -86,23 +87,23 @@ func (suite *ItemUnitSuite) TestOwnerErr() {
 
 func (suite *ItemUnitSuite) TestItemType_Printable() {
 	table := []struct {
-		t      itemType
+		t      fault.ItemType
 		expect string
 	}{
 		{
-			t:      FileType,
+			t:      fault.FileType,
 			expect: "File",
 		},
 		{
-			t:      ContainerType,
+			t:      fault.ContainerType,
 			expect: "Container",
 		},
 		{
-			t:      ResourceOwnerType,
+			t:      fault.ResourceOwnerType,
 			expect: "Resource Owner",
 		},
 		{
-			t:      itemType("foo"),
+			t:      fault.ItemType("foo"),
 			expect: "Unknown",
 		},
 	}
@@ -118,30 +119,30 @@ func (suite *ItemUnitSuite) TestItem_HeadersValues() {
 		err   = assert.AnError
 		cause = err.Error()
 		addtl = map[string]any{
-			AddtlContainerID:   "cid",
+			fault.AddtlContainerID:   "cid",
-			AddtlContainerName: "cname",
+			fault.AddtlContainerName: "cname",
 		}
 	)
 
 	table := []struct {
 		name   string
-		item   *Item
+		item   *fault.Item
 		expect []string
 	}{
 		{
 			name:   "file",
-			item:   FileErr(assert.AnError, "ns", "id", "name", addtl),
+			item:   fault.FileErr(assert.AnError, "ns", "id", "name", addtl),
-			expect: []string{"Error", FileType.Printable(), "name", "cname", cause},
+			expect: []string{"Error", fault.FileType.Printable(), "name", "cname", cause},
 		},
 		{
 			name:   "container",
-			item:   ContainerErr(assert.AnError, "ns", "id", "name", addtl),
+			item:   fault.ContainerErr(assert.AnError, "ns", "id", "name", addtl),
-			expect: []string{"Error", ContainerType.Printable(), "name", "cname", cause},
+			expect: []string{"Error", fault.ContainerType.Printable(), "name", "cname", cause},
 		},
 		{
 			name:   "owner",
-			item:   OwnerErr(assert.AnError, "ns", "id", "name", nil),
+			item:   fault.OwnerErr(assert.AnError, "ns", "id", "name", nil),
-			expect: []string{"Error", ResourceOwnerType.Printable(), "name", "", cause},
+			expect: []string{"Error", fault.ResourceOwnerType.Printable(), "name", "", cause},
 		},
 	}
 	for _, test := range table {
@@ -153,175 +154,3 @@ func (suite *ItemUnitSuite) TestItem_HeadersValues() {
 		})
 	}
 }
-
-func (suite *ItemUnitSuite) TestSkipped_String() {
-	var (
-		t = suite.T()
-		i *Skipped
-	)
-
-	assert.Contains(t, i.String(), "nil")
-
-	i = &Skipped{Item{}}
-	assert.Contains(t, i.String(), "unknown type")
-
-	i = &Skipped{Item{Type: FileType}}
-	assert.Contains(t, i.Item.Error(), FileType)
-}
-
-func (suite *ItemUnitSuite) TestContainerSkip() {
-	t := suite.T()
-	addtl := map[string]any{"foo": "bar"}
-	i := ContainerSkip(SkipMalware, "ns", "id", "name", addtl)
-
-	expect := Item{
-		Namespace:  "ns",
-		ID:         "id",
-		Name:       "name",
-		Type:       ContainerType,
-		Cause:      string(SkipMalware),
-		Additional: addtl,
-	}
-
-	assert.Equal(t, Skipped{expect}, *i)
-}
-
-func (suite *ItemUnitSuite) TestFileSkip() {
-	t := suite.T()
-	addtl := map[string]any{"foo": "bar"}
-	i := FileSkip(SkipMalware, "ns", "id", "name", addtl)
-
-	expect := Item{
-		Namespace:  "ns",
-		ID:         "id",
-		Name:       "name",
-		Type:       FileType,
-		Cause:      string(SkipMalware),
-		Additional: addtl,
-	}
-
-	assert.Equal(t, Skipped{expect}, *i)
-}
-
-func (suite *ItemUnitSuite) TestOwnerSkip() {
-	t := suite.T()
-	addtl := map[string]any{"foo": "bar"}
-	i := OwnerSkip(SkipMalware, "ns", "id", "name", addtl)
-
-	expect := Item{
-		Namespace:  "ns",
-		ID:         "id",
-		Name:       "name",
-		Type:       ResourceOwnerType,
-		Cause:      string(SkipMalware),
-		Additional: addtl,
-	}
-
-	assert.Equal(t, Skipped{expect}, *i)
-}
-
-func (suite *ItemUnitSuite) TestSkipped_HeadersValues() {
-	addtl := map[string]any{
-		AddtlContainerID:   "cid",
-		AddtlContainerName: "cname",
-	}
-
-	table := []struct {
-		name   string
-		skip   *Skipped
-		expect []string
-	}{
-		{
-			name:   "file",
-			skip:   FileSkip(SkipMalware, "ns", "id", "name", addtl),
-			expect: []string{"Skip", FileType.Printable(), "name", "cname", string(SkipMalware)},
-		},
-		{
-			name:   "container",
-			skip:   ContainerSkip(SkipMalware, "ns", "id", "name", addtl),
-			expect: []string{"Skip", ContainerType.Printable(), "name", "cname", string(SkipMalware)},
-		},
-		{
-			name:   "owner",
-			skip:   OwnerSkip(SkipMalware, "ns", "id", "name", nil),
-			expect: []string{"Skip", ResourceOwnerType.Printable(), "name", "", string(SkipMalware)},
-		},
-	}
-	for _, test := range table {
-		suite.Run(test.name, func() {
-			t := suite.T()
-
-			assert.Equal(t, []string{"Action", "Type", "Name", "Container", "Cause"}, test.skip.Headers())
-			assert.Equal(t, test.expect, test.skip.Values())
-		})
-	}
-}
-
-func (suite *ItemUnitSuite) TestAlert_String() {
-	var (
-		t = suite.T()
-		a Alert
-	)
-
-	assert.Contains(t, a.String(), "Alert: <nil>")
-
-	a = Alert{
-		Item:    Item{},
-		Message: "",
-	}
-	assert.Contains(t, a.String(), "Alert: <nil>")
-
-	a = Alert{
-		Item: Item{
-			ID: "item_id",
-		},
-		Message: "msg",
-	}
-	assert.NotContains(t, a.String(), "item_id")
-	assert.Contains(t, a.String(), "Alert: msg")
-}
-
-func (suite *ItemUnitSuite) TestNewAlert() {
-	t := suite.T()
-	addtl := map[string]any{"foo": "bar"}
-	a := NewAlert("message-to-show", "ns", "item_id", "item_name", addtl)
-
-	expect := Alert{
-		Item: Item{
-			Namespace:  "ns",
-			ID:         "item_id",
-			Name:       "item_name",
-			Additional: addtl,
-		},
-		Message: "message-to-show",
-	}
-
-	assert.Equal(t, expect, *a)
-}
-
-func (suite *ItemUnitSuite) TestAlert_HeadersValues() {
-	addtl := map[string]any{
-		AddtlContainerID:   "cid",
-		AddtlContainerName: "cname",
-	}
-
-	table := []struct {
-		name   string
-		alert  *Alert
-		expect []string
-	}{
-		{
-			name:   "new alert",
-			alert:  NewAlert("message-to-show", "ns", "id", "name", addtl),
-			expect: []string{"Alert", "message-to-show", "cname", "name", "id"},
-		},
-	}
-	for _, test := range table {
-		suite.Run(test.name, func() {
-			t := suite.T()
-
-			assert.Equal(t, []string{"Action", "Message", "Container", "Name", "ID"}, test.alert.Headers())
-			assert.Equal(t, test.expect, test.alert.Values())
-		})
-	}
-}

src/pkg/fault/skipped.go

@@ -0,0 +1,117 @@
+package fault
+
+import (
+	"github.com/alcionai/corso/src/cli/print"
+)
+
+// skipCause identifies the well-known conditions to Skip an item.  It is
+// important that skip cause enumerations do not overlap with general error
+// handling.  Skips must be well known, well documented, and consistent.
+// Transient failures, undocumented or unknown conditions, and arbitrary
+// handling should never produce a skipped item. Those cases should get
+// handled as normal errors.
+type skipCause string
+
+const (
+	// SkipMalware identifies a malware detection case.  Files that graph
+	// api identifies as malware cannot be downloaded or uploaded, and will
+	// permanently fail any attempts to backup or restore.
+	SkipMalware skipCause = "malware_detected"
+
+	// SkipBigOneNote identifies that a file was skipped because it
+	// was big OneNote file and we can only download OneNote files which
+	// are less that 2GB in size.
+	//nolint:lll
+	// https://support.microsoft.com/en-us/office/restrictions-and-limitations-in-onedrive-and-sharepoint-64883a5d-228e-48f5-b3d2-eb39e07630fa#onenotenotebooks
+	SkipBigOneNote skipCause = "big_one_note_file"
+)
+
+var _ print.Printable = &Skipped{}
+
+// Skipped items are permanently unprocessable due to well-known conditions.
+// In order to skip an item, the following conditions should be met:
+// 1. The conditions for skipping the item are well-known and
+// well-documented.  End users need to be able to understand
+// both the conditions and identifications of skips.
+// 2. Skipping avoids a permanent and consistent failure.  If
+// the underlying reason is transient or otherwise recoverable,
+// the item should not be skipped.
+//
+// Skipped wraps Item primarily to minimize confusion when sharing the
+// fault interface.  Skipped items are not errors, and Item{} errors are
+// not the basis for a Skip.
+type Skipped struct {
+	Item Item `json:"item"`
+}
+
+// String complies with the stringer interface.
+func (s *Skipped) String() string {
+	if s == nil {
+		return "<nil>"
+	}
+
+	return "skipped " + s.Item.Error() + ": " + s.Item.Cause
+}
+
+// HasCause compares the underlying cause against the parameter.
+func (s *Skipped) HasCause(c skipCause) bool {
+	if s == nil {
+		return false
+	}
+
+	return s.Item.Cause == string(c)
+}
+
+func (s Skipped) MinimumPrintable() any {
+	return s
+}
+
+// Headers returns the human-readable names of properties of a skipped Item
+// for printing out to a terminal.
+func (s Skipped) Headers() []string {
+	return []string{"Action", "Type", "Name", "Container", "Cause"}
+}
+
+// Values populates the printable values matching the Headers list.
+func (s Skipped) Values() []string {
+	var cn string
+
+	acn, ok := s.Item.Additional[AddtlContainerName]
+	if ok {
+		str, ok := acn.(string)
+		if ok {
+			cn = str
+		}
+	}
+
+	return []string{"Skip", s.Item.Type.Printable(), s.Item.Name, cn, s.Item.Cause}
+}
+
+// ContainerSkip produces a Container-kind Item for tracking skipped items.
+func ContainerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
+	return itemSkip(ContainerType, cause, namespace, id, name, addtl)
+}
+
+// FileSkip produces a File-kind Item for tracking skipped items.
+func FileSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
+	return itemSkip(FileType, cause, namespace, id, name, addtl)
+}
+
+// OnwerSkip produces a ResourceOwner-kind Item for tracking skipped items.
+func OwnerSkip(cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
+	return itemSkip(ResourceOwnerType, cause, namespace, id, name, addtl)
+}
+
+// itemSkip produces a Item of the provided type for tracking skipped items.
+func itemSkip(t ItemType, cause skipCause, namespace, id, name string, addtl map[string]any) *Skipped {
+	return &Skipped{
+		Item: Item{
+			Namespace:  namespace,
+			ID:         id,
+			Name:       name,
+			Type:       t,
+			Cause:      string(cause),
+			Additional: addtl,
+		},
+	}
+}

src/pkg/fault/skipped_test.go

@@ -0,0 +1,146 @@
+package fault_test
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
+
+	"github.com/alcionai/corso/src/internal/tester"
+	"github.com/alcionai/corso/src/pkg/fault"
+)
+
+type SkippedUnitSuite struct {
+	tester.Suite
+}
+
+func TestSkippedUnitSuite(t *testing.T) {
+	suite.Run(t, &SkippedUnitSuite{Suite: tester.NewUnitSuite(t)})
+}
+
+func (suite *SkippedUnitSuite) TestSkipped_String() {
+	var (
+		t = suite.T()
+		i *fault.Skipped
+	)
+
+	assert.Contains(t, i.String(), "nil")
+
+	i = &fault.Skipped{fault.Item{}}
+	assert.Contains(t, i.String(), "unknown type")
+
+	i = &fault.Skipped{
+		fault.Item{
+			Type: fault.FileType,
+		},
+	}
+	assert.Contains(t, i.Item.Error(), fault.FileType)
+}
+
+func (suite *SkippedUnitSuite) TestContainerSkip() {
+	t := suite.T()
+	addtl := map[string]any{"foo": "bar"}
+	i := fault.ContainerSkip(fault.SkipMalware, "ns", "id", "name", addtl)
+
+	expect := fault.Item{
+		Namespace:  "ns",
+		ID:         "id",
+		Name:       "name",
+		Type:       fault.ContainerType,
+		Cause:      string(fault.SkipMalware),
+		Additional: addtl,
+	}
+
+	assert.Equal(t, fault.Skipped{expect}, *i)
+}
+
+func (suite *SkippedUnitSuite) TestFileSkip() {
+	t := suite.T()
+	addtl := map[string]any{"foo": "bar"}
+	i := fault.FileSkip(fault.SkipMalware, "ns", "id", "name", addtl)
+
+	expect := fault.Item{
+		Namespace:  "ns",
+		ID:         "id",
+		Name:       "name",
+		Type:       fault.FileType,
+		Cause:      string(fault.SkipMalware),
+		Additional: addtl,
+	}
+
+	assert.Equal(t, fault.Skipped{expect}, *i)
+}
+
+func (suite *SkippedUnitSuite) TestOwnerSkip() {
+	t := suite.T()
+	addtl := map[string]any{"foo": "bar"}
+	i := fault.OwnerSkip(fault.SkipMalware, "ns", "id", "name", addtl)
+
+	expect := fault.Item{
+		Namespace:  "ns",
+		ID:         "id",
+		Name:       "name",
+		Type:       fault.ResourceOwnerType,
+		Cause:      string(fault.SkipMalware),
+		Additional: addtl,
+	}
+
+	assert.Equal(t, fault.Skipped{expect}, *i)
+}
+
+func (suite *SkippedUnitSuite) TestSkipped_HeadersValues() {
+	addtl := map[string]any{
+		fault.AddtlContainerID:   "cid",
+		fault.AddtlContainerName: "cname",
+	}
+
+	table := []struct {
+		name   string
+		skip   *fault.Skipped
+		expect []string
+	}{
+		{
+			name:   "file",
+			skip:   fault.FileSkip(fault.SkipMalware, "ns", "id", "name", addtl),
+			expect: []string{"Skip", fault.FileType.Printable(), "name", "cname", string(fault.SkipMalware)},
+		},
+		{
+			name:   "container",
+			skip:   fault.ContainerSkip(fault.SkipMalware, "ns", "id", "name", addtl),
+			expect: []string{"Skip", fault.ContainerType.Printable(), "name", "cname", string(fault.SkipMalware)},
+		},
+		{
+			name:   "owner",
+			skip:   fault.OwnerSkip(fault.SkipMalware, "ns", "id", "name", nil),
+			expect: []string{"Skip", fault.ResourceOwnerType.Printable(), "name", "", string(fault.SkipMalware)},
+		},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			t := suite.T()
+
+			assert.Equal(t, []string{"Action", "Type", "Name", "Container", "Cause"}, test.skip.Headers())
+			assert.Equal(t, test.expect, test.skip.Values())
+		})
+	}
+}
+
+func (suite *SkippedUnitSuite) TestBus_AddSkip() {
+	t := suite.T()
+
+	ctx, flush := tester.NewContext(t)
+	defer flush()
+
+	n := fault.New(true)
+	require.NotNil(t, n)
+
+	n.Fail(assert.AnError)
+	assert.Len(t, n.Skipped(), 0)
+
+	n.AddRecoverable(ctx, assert.AnError)
+	assert.Len(t, n.Skipped(), 0)
+
+	n.AddSkip(ctx, fault.OwnerSkip(fault.SkipMalware, "ns", "id", "name", nil))
+	assert.Len(t, n.Skipped(), 1)
+}