diff --git a/ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/LwwApplier.cs b/ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/LwwApplier.cs index 153dd7a..a384b77 100644 --- a/ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/LwwApplier.cs +++ b/ZB.MOM.WW.LocalDb/src/ZB.MOM.WW.LocalDb.Replication/Internal/LwwApplier.cs @@ -98,6 +98,9 @@ internal sealed class LwwApplier(SqliteLocalDb db, Func? utcNow // the row died), or an upsert whose new row carries the OLD version and therefore loses // the next LWW comparison it should win. A per-entry savepoint makes the pair // all-or-nothing without widening the poison catch to the whole batch. + // + // Opened OUTSIDE the try on purpose: a SAVEPOINT that failed opened nothing, so the + // handler's ROLLBACK TO would name a savepoint that does not exist and fault in turn. await sql.EnterEntryAsync(ct); try { @@ -117,8 +120,27 @@ internal sealed class LwwApplier(SqliteLocalDb db, Func? utcNow // statement, not the transaction, so the batch continues after dead-lettering. // Any other exception propagates with the savepoint still open, and disposing the // outer transaction unwinds the whole batch (fail-closed). - await sql.RollbackEntryAsync(ct); - await sql.DeadLetterAsync(entry, ex.Message, utcNow, ct); + try + { + await sql.RollbackEntryAsync(ct); + await sql.DeadLetterAsync(entry, ex.Message, utcNow, ct); + } + catch (Exception recoveryFailure) when (recoveryFailure is not OperationCanceledException) + { + // Recovering from the poison entry itself failed — a wedged connection, an I/O + // error, a dead-letter INSERT that faults. The outcome is unchanged and still + // fail-closed (nothing commits; disposing the batch transaction unwinds it), + // but reporting only the recovery failure would throw away the ORIGINAL cause, + // which is the one that says WHY this entry is poison. Both are carried out: + // the original as InnerException (AggregateException surfaces the first inner + // there), the recovery failure alongside it with its own stack intact. + throw new AggregateException( + $"Failed to dead-letter a poison oplog entry for table '{entry.TableName}' " + + $"(seq {entry.Seq}): {recoveryFailure.Message}. The entry's own fault is the " + + "inner exception. The batch is not committed.", + ex, recoveryFailure); + } + deadLettered++; } } diff --git a/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/LwwApplierTests.cs b/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/LwwApplierTests.cs index 75a353a..4f7826c 100644 --- a/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/LwwApplierTests.cs +++ b/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/LwwApplierTests.cs @@ -543,6 +543,41 @@ public sealed class LwwApplierTests : IDisposable Assert.Equal(("LOCAL", (long?)5), await ReadOrder(db, 1)); } + [Fact] + public async Task Apply_DeadLetterFailure_ReportsBothCausesAndCommitsNothing() + { + // Two faults stacked: the entry is poison (its ledger write aborts) AND recovering from it + // fails (the dead-letter INSERT aborts too). Reporting only the second would discard the + // one that says WHY the entry was poison, leaving an operator with "dead-letter failed" + // and no cause. + using var db = await NewOrdersDb(); + await db.ExecuteAsync("INSERT INTO orders (id, sku, qty) VALUES (1, 'LOCAL', 5)"); + var localHlc = await RowVersionHlc(db, Pk1); + await PoisonRowVersionWriteFor(db, Pk1); + await db.ExecuteAsync( + "CREATE TRIGGER poison_dl BEFORE INSERT ON __localdb_dead_letter " + + "BEGIN SELECT RAISE(ABORT, 'dead-letter write failed'); END"); + + var ex = await Assert.ThrowsAsync(() => new LwwApplier(db).ApplyBatchAsync( + [ + Entry(95, "{\"id\":2}", "{\"id\":2,\"sku\":\"BEFORE\",\"qty\":2}", localHlc + 1000, "remote-node"), + Entry(96, Pk1, null, localHlc + 1001, "remote-node", tombstone: true), + ], default)); + + // The entry's own fault first (InnerException), the recovery fault carried alongside it. + Assert.Contains("ledger write failed", ex.InnerException!.Message); + Assert.Contains("dead-letter write failed", ex.InnerExceptions[1].Message); + Assert.Contains("orders", ex.Message); + Assert.Contains("96", ex.Message); + + // Fail-closed: the batch never commits, so even the entry that applied cleanly is gone. + Assert.Null(await ReadOrder(db, 2)); + Assert.Equal(("LOCAL", (long?)5), await ReadOrder(db, 1)); + var watermark = await db.QueryAsync( + "SELECT last_applied_remote_seq FROM __localdb_peer_state WHERE id = 1", x => x.GetInt64(0)); + Assert.Equal(0, watermark[0]); + } + [Fact] public async Task Apply_ReplaceExistingRow_DoesNotFireCaptureTriggers() { diff --git a/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/RegistrationTests.cs b/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/RegistrationTests.cs index ef95e29..d875b86 100644 --- a/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/RegistrationTests.cs +++ b/ZB.MOM.WW.LocalDb/tests/ZB.MOM.WW.LocalDb.Tests/RegistrationTests.cs @@ -141,6 +141,23 @@ public sealed class RegistrationTests : IDisposable Assert.Equal(3, (await Triggers(db)).Count); } + [Fact] + public async Task Register_SelfReferencingCascade_Throws() + { + // A table can be its own cascading parent: replacing a row would delete that row's + // children in the SAME table, uncaptured. The guard reads every foreign key in the + // database, so it does not need a special case — but the README promises this, so it is + // pinned. + using var db = Create(); + await db.ExecuteAsync( + "CREATE TABLE nodes (id INTEGER PRIMARY KEY, " + + "parent_id INTEGER REFERENCES nodes(id) ON DELETE CASCADE, name TEXT)"); + + var ex = Assert.Throws(() => db.RegisterReplicated("nodes")); + Assert.Contains("nodes.parent_id", ex.Message); + Assert.Contains("ON DELETE CASCADE", ex.Message); + } + [Fact] public async Task Register_CascadeDeleteChild_IsAllowed() {