Follow-up from the test-suite review (batches merged as #92–#96).
Gap
DataModel.AddRangeFromSync: on a DbUpdateException the catch block logs, writes last-failed-import.json (DumpFailedSync), and rethrows. Neither the dump-file side effect nor the rethrow is asserted anywhere.
|
async Task ISyncable.AddRangeFromSync(IEnumerable<Commit> commits) |
|
{ |
|
commits = commits.ToArray(); |
|
try |
|
{ |
|
await using var repo = await _crdtRepositoryFactory.CreateRepository(); |
|
using var locked = await repo.Lock(); |
|
repo.ClearChangeTracker(); |
|
_timeProvider.TakeLatestTime(commits.Select(c => c.HybridDateTime)); |
|
var (oldestChange, newCommits) = await repo.FilterExistingCommits(commits.ToArray()); |
|
//no changes added |
|
if (oldestChange is null || newCommits is []) return; |
|
|
|
await using var transaction = await repo.BeginTransactionAsync(); |
|
var updatedCommits = await repo.AddCommits(newCommits); |
|
await UpdateSnapshots(repo, updatedCommits); |
|
await ValidateCommits(repo); |
|
await transaction.CommitAsync(); |
|
} |
|
catch (DbUpdateException e) |
|
{ |
|
_logger.LogError(e, "Failed to sync commits, check {FailedImportPath} for more details", _crdtConfig.Value.FailedSyncOutputPath); |
|
await DumpFailedSync(new |
|
{ |
|
ExceptionMessage = e.ToString(), |
|
Commits = commits.DefaultOrder(), |
|
Objects = e.Entries.Select(entry => entry.Entity) |
|
}); |
|
throw; |
|
} |
|
} |
|
private async Task DumpFailedSync(object data) |
|
{ |
|
try |
|
{ |
|
Directory.CreateDirectory(_crdtConfig.Value.FailedSyncOutputPath); |
|
await using var failedImport = |
|
File.Create(Path.Combine(_crdtConfig.Value.FailedSyncOutputPath, "last-failed-import.json")); |
|
await JsonSerializer.SerializeAsync(failedImport, data, _serializerOptions); |
|
} |
|
catch (Exception e) |
|
{ |
|
_logger.LogError(e, "Failed to dump failed import"); |
|
} |
Suggested test
AddRangeFromSync_OnDbUpdateException_DumpsFailedImportAndRethrows
Why it matters
This is the sync failure-diagnostics contract — if it silently breaks, all sync debugging output is lost with no signal. The test needs a way to force a DbUpdateException during import (e.g. a conflicting/duplicate commit) and to point FailedSyncOutputPath at a temp directory.
Follow-up from the test-suite review (batches merged as #92–#96).
Gap
DataModel.AddRangeFromSync: on aDbUpdateExceptionthe catch block logs, writeslast-failed-import.json(DumpFailedSync), and rethrows. Neither the dump-file side effect nor the rethrow is asserted anywhere.harmony/src/SIL.Harmony/DataModel.cs
Lines 138 to 168 in 03f609f
harmony/src/SIL.Harmony/DataModel.cs
Lines 170 to 182 in 03f609f
Suggested test
AddRangeFromSync_OnDbUpdateException_DumpsFailedImportAndRethrowsWhy it matters
This is the sync failure-diagnostics contract — if it silently breaks, all sync debugging output is lost with no signal. The test needs a way to force a
DbUpdateExceptionduring import (e.g. a conflicting/duplicate commit) and to pointFailedSyncOutputPathat a temp directory.