From ea2d9cecebe88fa88730f783cf632f22df42254d Mon Sep 17 00:00:00 2001
From: ashmrtn <ashmrtnz@alcion.ai>
Date: Fri, 20 Jan 2023 13:40:48 -0800
Subject: [PATCH] Report errors when building details (#2206)

## Description

Report an error if an item in details that was sourced from a base snapshot doesn't have a previous path. Items that don't have a previous path cannot be sourced from a base snapshot's backup details meaning the item will be stored but will not be searchable/restorable by users

Logging was not used because no context is available in the kopia callback that is checking if the previous path is nil

## 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
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :broom: Tech Debt/Cleanup

## Issue(s)

* closes #1915

## Test Plan

- [ ] :muscle: Manual
- [x] :zap: Unit test
- [ ] :green_heart: E2E
---
 src/internal/kopia/upload.go      |  8 +++++++-
 src/internal/kopia/upload_test.go | 34 ++++++++++++++++++++++++++++++-
 src/internal/kopia/wrapper.go     |  5 +++--
 3 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/src/internal/kopia/upload.go b/src/internal/kopia/upload.go
index d9320f2a6..52452ffa9 100644
--- a/src/internal/kopia/upload.go
+++ b/src/internal/kopia/upload.go
@@ -134,6 +134,7 @@ type corsoProgress struct {
 	toMerge    map[string]path.Path
 	mu         sync.RWMutex
 	totalBytes int64
+	errs       *multierror.Error
 }
 
 // Kopia interface function used as a callback when kopia finishes processing a
@@ -162,8 +163,13 @@ func (cp *corsoProgress) FinishedFile(relativePath string, err error) {
 	// These items were sourced from a base snapshot or were cached in kopia so we
 	// never had to materialize their details in-memory.
 	if d.info == nil {
-		// TODO(ashmrtn): We should probably be returning an error here?
 		if d.prevPath == nil {
+			cp.errs = multierror.Append(cp.errs, errors.Errorf(
+				"item sourced from previous backup with no previous path. Service: %s, Category: %s",
+				d.repoPath.Service().String(),
+				d.repoPath.Category().String(),
+			))
+
 			return
 		}
 
diff --git a/src/internal/kopia/upload_test.go b/src/internal/kopia/upload_test.go
index a3a865cd8..ff86db13b 100644
--- a/src/internal/kopia/upload_test.go
+++ b/src/internal/kopia/upload_test.go
@@ -468,7 +468,7 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() {
 
 					for k, v := range ci {
 						if cachedTest.cached {
-							cp.CachedFile(k, 42)
+							cp.CachedFile(k, v.totalBytes)
 						}
 
 						cp.FinishedFile(k, v.err)
@@ -489,6 +489,38 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() {
 	}
 }
 
+func (suite *CorsoProgressUnitSuite) TestFinishedFileCachedNoPrevPathErrors() {
+	t := suite.T()
+	bd := &details.Builder{}
+	cachedItems := map[string]testInfo{
+		suite.targetFileName: {
+			info:       &itemDetails{info: nil, repoPath: suite.targetFilePath},
+			err:        nil,
+			totalBytes: 100,
+		},
+	}
+	cp := corsoProgress{
+		UploadProgress: &snapshotfs.NullUploadProgress{},
+		deets:          bd,
+		pending:        map[string]*itemDetails{},
+	}
+
+	for k, v := range cachedItems {
+		cp.put(k, v.info)
+	}
+
+	require.Len(t, cp.pending, len(cachedItems))
+
+	for k, v := range cachedItems {
+		cp.CachedFile(k, v.totalBytes)
+		cp.FinishedFile(k, v.err)
+	}
+
+	assert.Empty(t, cp.pending)
+	assert.Empty(t, bd.Details().Entries)
+	assert.Error(t, cp.errs.ErrorOrNil())
+}
+
 func (suite *CorsoProgressUnitSuite) TestFinishedFileBuildsHierarchyNewItem() {
 	t := suite.T()
 	// Order of folders in hierarchy from root to leaf (excluding the item).
diff --git a/src/internal/kopia/wrapper.go b/src/internal/kopia/wrapper.go
index feab0e687..8171c853a 100644
--- a/src/internal/kopia/wrapper.go
+++ b/src/internal/kopia/wrapper.go
@@ -160,10 +160,11 @@ func (w Wrapper) BackupCollections(
 		progress,
 	)
 	if err != nil {
-		return nil, nil, nil, err
+		combinedErrs := multierror.Append(nil, err, progress.errs)
+		return nil, nil, nil, combinedErrs.ErrorOrNil()
 	}
 
-	return s, progress.deets, progress.toMerge, nil
+	return s, progress.deets, progress.toMerge, progress.errs.ErrorOrNil()
 }
 
 func (w Wrapper) makeSnapshotWithRoot(