From 498fc325a9149e22617515a06b6a0c0e36827613 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 31 Jan 2023 14:22:53 -0700 Subject: [PATCH] store errors in operation (#2237) ## Description Adds the fault.Errors struct (now exported) to the operations base. stats.Errs is retained in the backup and restore wrappers to avoid breaking changes and allow for deserialization. We will continue to use the current error return until dependencies are fully updated to use Errors. ## Does this PR need a docs update or release note? - [x] :no_entry: No ## Type of change - [x] :broom: Tech Debt/Cleanup ## Issue(s) * #1970 ## Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/operations/backup.go | 3 +- .../operations/backup_integration_test.go | 6 ++++ src/internal/operations/backup_test.go | 2 +- src/internal/operations/operation.go | 20 +++++++---- src/internal/operations/restore.go | 2 +- src/internal/operations/restore_test.go | 8 +++-- src/pkg/backup/backup.go | 33 ++++++++++++++++--- src/pkg/backup/backup_test.go | 4 +++ src/pkg/repository/repository_load_test.go | 12 ++++--- 9 files changed, 69 insertions(+), 21 deletions(-) diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 247f9859b..b2a0c552c 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -44,7 +44,7 @@ type BackupOperation struct { // BackupResults aggregate the details of the result of the operation. type BackupResults struct { - stats.Errs + stats.Errs // deprecated in place of fault.Errors in the base operation. stats.ReadWrites stats.StartAndEndTime BackupID model.StableID `json:"backupID"` @@ -609,6 +609,7 @@ func (op *BackupOperation) createBackupModels( op.Selectors, op.Results.ReadWrites, op.Results.StartAndEndTime, + op.Errors, ) err = op.store.Put(ctx, model.BackupSchema, b) diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index c28159c1a..21d4009e0 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -153,6 +153,8 @@ func runAndCheckBackup( assert.Less(t, int64(0), bo.Results.BytesRead, "bytes read") assert.Less(t, int64(0), bo.Results.BytesUploaded, "bytes uploaded") assert.Equal(t, 1, bo.Results.ResourceOwners, "count of resource owners") + assert.NoError(t, bo.Errors.Err(), "incremental non-recoverable error") + assert.Empty(t, bo.Errors.Errs(), "incremental recoverable/iteration errors") assert.NoError(t, bo.Results.ReadErrors, "errors reading data") assert.NoError(t, bo.Results.WriteErrors, "errors writing data") assert.Equal(t, 1, mb.TimesCalled[events.BackupStart], "backup-start events") @@ -616,6 +618,8 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchange() { assert.Greater(t, bo.Results.BytesRead, incBO.Results.BytesRead, "incremental bytes read") assert.Greater(t, bo.Results.BytesUploaded, incBO.Results.BytesUploaded, "incremental bytes uploaded") assert.Equal(t, bo.Results.ResourceOwners, incBO.Results.ResourceOwners, "incremental backup resource owner") + assert.NoError(t, incBO.Errors.Err(), "incremental non-recoverable error") + assert.Empty(t, incBO.Errors.Errs(), "count incremental recoverable/iteration errors") assert.NoError(t, incBO.Results.ReadErrors, "incremental read errors") assert.NoError(t, incBO.Results.WriteErrors, "incremental write errors") assert.Equal(t, 1, incMB.TimesCalled[events.BackupStart], "incremental backup-start events") @@ -1039,6 +1043,8 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { // +4 on read/writes to account for metadata: 1 delta and 1 path for each type. assert.Equal(t, test.itemsWritten+4, incBO.Results.ItemsWritten, "incremental items written") assert.Equal(t, test.itemsRead+4, incBO.Results.ItemsRead, "incremental items read") + assert.NoError(t, incBO.Errors.Err(), "incremental non-recoverable error") + assert.Empty(t, incBO.Errors.Errs(), "incremental recoverable/iteration errors") assert.NoError(t, incBO.Results.ReadErrors, "incremental read errors") assert.NoError(t, incBO.Results.WriteErrors, "incremental write errors") assert.Equal(t, 1, incMB.TimesCalled[events.BackupStart], "incremental backup-start events") diff --git a/src/internal/operations/backup_test.go b/src/internal/operations/backup_test.go index 2a7a1ecce..f867e2a11 100644 --- a/src/internal/operations/backup_test.go +++ b/src/internal/operations/backup_test.go @@ -420,11 +420,11 @@ func (suite *BackupOpSuite) TestBackupOperation_PersistResults() { assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status") assert.Equal(t, test.stats.gc.Successful, op.Results.ItemsRead, "items read") - assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors") assert.Equal(t, test.stats.k.TotalFileCount, op.Results.ItemsWritten, "items written") assert.Equal(t, test.stats.k.TotalHashedBytes, op.Results.BytesRead, "bytes read") assert.Equal(t, test.stats.k.TotalUploadedBytes, op.Results.BytesUploaded, "bytes written") assert.Equal(t, test.stats.resourceCount, op.Results.ResourceOwners, "resource owners") + assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors") assert.Equal(t, test.stats.writeErr, op.Results.WriteErrors, "write errors") assert.Equal(t, now, op.Results.StartedAt, "started at") assert.Less(t, now, op.Results.CompletedAt, "completed at") diff --git a/src/internal/operations/operation.go b/src/internal/operations/operation.go index 24391997e..32122d682 100644 --- a/src/internal/operations/operation.go +++ b/src/internal/operations/operation.go @@ -13,6 +13,7 @@ import ( "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/store" ) @@ -52,9 +53,11 @@ const ( // Specific processes (eg: backups, restores, etc) are expected to wrap operation // with process specific details. type operation struct { - CreatedAt time.Time `json:"createdAt"` // datetime of the operation's creation - Options control.Options `json:"options"` - Status opStatus `json:"status"` + CreatedAt time.Time `json:"createdAt"` + + Errors *fault.Errors `json:"errors"` + Options control.Options `json:"options"` + Status opStatus `json:"status"` bus events.Eventer kopia *kopia.Wrapper @@ -69,11 +72,14 @@ func newOperation( ) operation { return operation{ CreatedAt: time.Now(), + Errors: fault.New(opts.FailFast), Options: opts, - bus: bus, - kopia: kw, - store: sw, - Status: InProgress, + + bus: bus, + kopia: kw, + store: sw, + + Status: InProgress, } } diff --git a/src/internal/operations/restore.go b/src/internal/operations/restore.go index 8ddaa8d29..db5ca9a93 100644 --- a/src/internal/operations/restore.go +++ b/src/internal/operations/restore.go @@ -44,7 +44,7 @@ type RestoreOperation struct { // RestoreResults aggregate the details of the results of the operation. type RestoreResults struct { - stats.Errs + stats.Errs // deprecated in place of fault.Errors in the base operation. stats.ReadWrites stats.StartAndEndTime } diff --git a/src/internal/operations/restore_test.go b/src/internal/operations/restore_test.go index 973a178b4..9bf21b806 100644 --- a/src/internal/operations/restore_test.go +++ b/src/internal/operations/restore_test.go @@ -104,10 +104,10 @@ func (suite *RestoreOpSuite) TestRestoreOperation_PersistResults() { assert.Equal(t, test.expectStatus.String(), op.Status.String(), "status") assert.Equal(t, len(test.stats.cs), op.Results.ItemsRead, "items read") - assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors") assert.Equal(t, test.stats.gc.Successful, op.Results.ItemsWritten, "items written") assert.Equal(t, test.stats.bytesRead.NumBytes, op.Results.BytesRead, "resource owners") assert.Equal(t, test.stats.resourceCount, op.Results.ResourceOwners, "resource owners") + assert.Equal(t, test.stats.readErr, op.Results.ReadErrors, "read errors") assert.Equal(t, test.stats.writeErr, op.Results.WriteErrors, "write errors") assert.Equal(t, now, op.Results.StartedAt, "started at") assert.Less(t, now, op.Results.CompletedAt, "completed at") @@ -293,8 +293,10 @@ func (suite *RestoreOpIntegrationSuite) TestRestore_Run() { assert.Less(t, 0, ro.Results.ItemsWritten, "restored items written") assert.Less(t, int64(0), ro.Results.BytesRead, "bytes read") assert.Equal(t, 1, ro.Results.ResourceOwners, "resource Owners") - assert.Zero(t, ro.Results.ReadErrors, "errors while reading restore data") - assert.Zero(t, ro.Results.WriteErrors, "errors while writing restore data") + assert.NoError(t, ro.Errors.Err(), "non-recoverable error") + assert.Empty(t, ro.Errors.Errs(), "recoverable errors") + assert.NoError(t, ro.Results.ReadErrors, "errors while reading restore data") + assert.NoError(t, ro.Results.WriteErrors, "errors while writing restore data") assert.Equal(t, suite.numItems, ro.Results.ItemsWritten, "backup and restore wrote the same num of items") assert.Equal(t, 1, mb.TimesCalled[events.RestoreStart], "restore-start events") assert.Equal(t, 1, mb.TimesCalled[events.RestoreEnd], "restore-end events") diff --git a/src/pkg/backup/backup.go b/src/pkg/backup/backup.go index 6d73498eb..d0d9ddffd 100644 --- a/src/pkg/backup/backup.go +++ b/src/pkg/backup/backup.go @@ -10,6 +10,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/stats" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -31,8 +32,11 @@ type Backup struct { // Selector used in this operation Selector selectors.Selector `json:"selectors"` + // Errors contains all errors aggregated during a backup operation. + Errors fault.ErrorsData `json:"errors"` + // stats are embedded so that the values appear as top-level properties - stats.Errs + stats.Errs // Deprecated, replaced with Errors. stats.ReadWrites stats.StartAndEndTime } @@ -46,6 +50,7 @@ func New( selector selectors.Selector, rw stats.ReadWrites, se stats.StartAndEndTime, + errs *fault.Errors, ) *Backup { return &Backup{ BaseModel: model.BaseModel{ @@ -59,6 +64,7 @@ func New( DetailsID: detailsID, Status: status, Selector: selector, + Errors: errs.Data(), ReadWrites: rw, StartAndEndTime: se, } @@ -102,7 +108,7 @@ type Printable struct { func (b Backup) MinimumPrintable() any { return Printable{ ID: b.ID, - ErrorCount: support.GetNumberOfErrors(b.ReadErrors) + support.GetNumberOfErrors(b.WriteErrors), + ErrorCount: b.errorCount(), StartedAt: b.StartedAt, Status: b.Status, Version: "0", @@ -125,8 +131,7 @@ func (b Backup) Headers() []string { // Values returns the values matching the Headers list for printing // out to a terminal in a columnar display. func (b Backup) Values() []string { - errCount := support.GetNumberOfErrors(b.ReadErrors) + support.GetNumberOfErrors(b.WriteErrors) - status := fmt.Sprintf("%s (%d errors)", b.Status, errCount) + status := fmt.Sprintf("%s (%d errors)", b.Status, b.errorCount()) return []string{ common.FormatTabularDisplayTime(b.StartedAt), @@ -135,3 +140,23 @@ func (b Backup) Values() []string { b.Selector.DiscreteOwner, } } + +func (b Backup) errorCount() int { + var errCount int + + // current tracking + if b.ReadErrors != nil || b.WriteErrors != nil { + return support.GetNumberOfErrors(b.ReadErrors) + support.GetNumberOfErrors(b.WriteErrors) + } + + // future tracking + if b.Errors.Err != nil || len(b.Errors.Errs) > 0 { + if b.Errors.Err != nil { + errCount++ + } + + errCount += len(b.Errors.Errs) + } + + return errCount +} diff --git a/src/pkg/backup/backup_test.go b/src/pkg/backup/backup_test.go index 1ddf4f4a2..8fbf8c62f 100644 --- a/src/pkg/backup/backup_test.go +++ b/src/pkg/backup/backup_test.go @@ -13,6 +13,7 @@ import ( "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/stats" "github.com/alcionai/corso/src/pkg/backup" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -40,6 +41,9 @@ func stubBackup(t time.Time) backup.Backup { DetailsID: "details", Status: "status", Selector: sel.Selector, + Errors: fault.ErrorsData{ + Errs: []error{errors.New("read"), errors.New("write")}, + }, Errs: stats.Errs{ ReadErrors: errors.New("1"), WriteErrors: errors.New("1"), diff --git a/src/pkg/repository/repository_load_test.go b/src/pkg/repository/repository_load_test.go index 744b57449..36714df71 100644 --- a/src/pkg/repository/repository_load_test.go +++ b/src/pkg/repository/repository_load_test.go @@ -184,8 +184,10 @@ func runBackupLoadTest( assert.Less(t, 0, b.Results.ItemsWritten, "items written") assert.Less(t, int64(0), b.Results.BytesUploaded, "bytes uploaded") assert.Equal(t, len(users), b.Results.ResourceOwners, "resource owners") - assert.Zero(t, b.Results.ReadErrors, "read errors") - assert.Zero(t, b.Results.WriteErrors, "write errors") + assert.NoError(t, b.Errors.Err(), "non-recoverable error") + assert.Empty(t, b.Errors.Errs(), "recoverable errors") + assert.NoError(t, b.Results.ReadErrors, "read errors") + assert.NoError(t, b.Results.WriteErrors, "write errors") }) } @@ -290,8 +292,10 @@ func doRestoreLoadTest( assert.Less(t, 0, r.Results.ItemsRead, "items read") assert.Less(t, 0, r.Results.ItemsWritten, "items written") assert.Equal(t, len(users), r.Results.ResourceOwners, "resource owners") - assert.Zero(t, r.Results.ReadErrors, "read errors") - assert.Zero(t, r.Results.WriteErrors, "write errors") + assert.NoError(t, r.Errors.Err(), "non-recoverable error") + assert.Empty(t, r.Errors.Errs(), "recoverable errors") + assert.NoError(t, r.Results.ReadErrors, "read errors") + assert.NoError(t, r.Results.WriteErrors, "write errors") assert.Equal(t, expectItemCount, r.Results.ItemsWritten, "backup and restore wrote the same count of items") ensureAllUsersInDetails(t, users, ds, "restore", name)