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?

- [ ]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [x]  No 

## Type of change

- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Test
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

## Issue(s)

* closes #1915

## Test Plan

- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
ashmrtn 2023-01-20 13:40:48 -08:00 committed by GitHub
parent cd77f43fb2
commit ea2d9ceceb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 43 additions and 4 deletions

View File

@ -134,6 +134,7 @@ type corsoProgress struct {
toMerge map[string]path.Path toMerge map[string]path.Path
mu sync.RWMutex mu sync.RWMutex
totalBytes int64 totalBytes int64
errs *multierror.Error
} }
// Kopia interface function used as a callback when kopia finishes processing a // 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 // These items were sourced from a base snapshot or were cached in kopia so we
// never had to materialize their details in-memory. // never had to materialize their details in-memory.
if d.info == nil { if d.info == nil {
// TODO(ashmrtn): We should probably be returning an error here?
if d.prevPath == nil { 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 return
} }

View File

@ -468,7 +468,7 @@ func (suite *CorsoProgressUnitSuite) TestFinishedFile() {
for k, v := range ci { for k, v := range ci {
if cachedTest.cached { if cachedTest.cached {
cp.CachedFile(k, 42) cp.CachedFile(k, v.totalBytes)
} }
cp.FinishedFile(k, v.err) 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() { func (suite *CorsoProgressUnitSuite) TestFinishedFileBuildsHierarchyNewItem() {
t := suite.T() t := suite.T()
// Order of folders in hierarchy from root to leaf (excluding the item). // Order of folders in hierarchy from root to leaf (excluding the item).

View File

@ -160,10 +160,11 @@ func (w Wrapper) BackupCollections(
progress, progress,
) )
if err != nil { 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( func (w Wrapper) makeSnapshotWithRoot(