From 7050b0c9d29fe0a10144df4f3e413b1b99dfc2cf Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Thu, 21 Sep 2023 11:22:48 +0530 Subject: [PATCH] Improve error handling during backup persistence (#4320) We were not catching missing snapshot ID sooner. Fixed it. --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * https://github.com/alcionai/corso/issues/4305 #### Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/operations/backup.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 547133403..00b280c80 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -856,12 +856,23 @@ func (op *BackupOperation) createBackupModels( // are generated during the serialization process. errs := fault.New(true) + // We don't persist a backup if there were non-recoverable errors seen + // during the operation, regardless of the failure policy. Unlikely we'd + // hit this here as the preceding code should already take care of it. + if op.Errors.Failure() != nil { + return clues.Wrap(op.Errors.Failure(), "non-recoverable failure").WithClues(ctx) + } + if deets == nil { return clues.New("no backup details to record").WithClues(ctx) } ctx = clues.Add(ctx, "details_entry_count", len(deets.Entries)) + if len(snapID) == 0 { + return clues.New("no snapshot ID to record").WithClues(ctx) + } + err := sscw.Collect(ctx, streamstore.DetailsCollector(deets)) if err != nil { return clues.Wrap(err, "collecting details for persistence").WithClues(ctx)