Skip to content

SaveChangesAsync wraps a single DcbConcurrencyException in AggregateException (Marten throws it directly) #394

Description

@jeremydmiller

Surfaced by the compliance-library adoption (#393). Not a bug — a deliberate-looking divergence that is worth an explicit decision, since it is the one place the shared suites had to accommodate both products instead of asserting one behavior.

Behavior

DocumentSessionBase.SaveChangesAsync batches the DCB assertions into a single command, postprocesses them in a loop collecting failures into a list, and then throws unconditionally:

var dcbExceptions = new List<Exception>();
await using var dcbReader = await ExecuteReaderAsync(dcbBatch, token);
for (var i = 0; i < dcbAssertions.Count; i++)
{
    await dcbAssertions[i].PostprocessAsync(dcbReader, dcbExceptions, token);
    ...
}

if (dcbExceptions.Count > 0)
{
    throw new AggregateException(dcbExceptions);   // even when Count == 1
}

So a session with one FetchForWritingByTags boundary that loses a concurrency race throws AggregateException containing a single DcbConcurrencyException. Marten throws the DcbConcurrencyException directly.

Consequence for users: the documented retry pattern does not port. This is the shape our own docs sample shows, and it silently does not catch on Polecat:

try { await session.SaveChangesAsync(); }
catch (DcbConcurrencyException ex) { /* reload and retry */ }

Our existing test encodes the wrapped shape (Should.ThrowAsync<AggregateException> then InnerExceptions.ShouldContain(...)), so it is at least known behavior rather than an accident.

Options

  1. Unwrap the single casethrow dcbExceptions.Count == 1 ? dcbExceptions[0] : new AggregateException(dcbExceptions). Gives Marten parity for the overwhelmingly common one-boundary session and makes the documented retry pattern work. Behavioral change for anyone currently catching AggregateException.
  2. Keep as is and document the difference explicitly in the DCB docs, including a Polecat-flavored retry sample.

Current handling in the shared suites

Absorbed, not forced either way. EventStoreComplianceSuite.ShouldFailWithAsync<T> accepts the exception directly or flattened out of an AggregateException, so the three affected compliance tests assert the semantics on both stores. If option 1 lands, that helper keeps working unchanged.

Related: marten#5115, marten#5119.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions