Skip to content

fix(efcore): suppress duplicate FlushOutgoingMessages in Eager-mode HTTP chains#2663

Merged
jeremydmiller merged 1 commit intomainfrom
fix-efcore-outbox-double-flush
May 2, 2026
Merged

fix(efcore): suppress duplicate FlushOutgoingMessages in Eager-mode HTTP chains#2663
jeremydmiller merged 1 commit intomainfrom
fix-efcore-outbox-double-flush

Conversation

@jeremydmiller
Copy link
Copy Markdown
Member

Summary

Fixes a code-generation bug in the EF Core transactional middleware reported via the sample at https://github.com/dmytro-pryvedeniuk/outbox.

For an HTTP endpoint that takes a DbContext and publishes a cascading message, the generated handler emitted messageContext.FlushOutgoingMessagesAsync() BEFORE efCoreEnvelopeTransaction.CommitAsync(...):

// Added by EF Core Transaction Middleware
var result_of_SaveChangesAsync = await _tasksContext.SaveChangesAsync(...);

// Have to flush outgoing messages just in case Marten did nothing because of #536
await messageContext.FlushOutgoingMessagesAsync().ConfigureAwait(false);  // <-- early flush

await efCoreEnvelopeTransaction.CommitAsync(...).ConfigureAwait(false);    // <-- this commits + re-flushes

Runtime sequence: SaveChangesAsync writes the wolverine_outgoing row inside the open EF Core transaction (uncommitted). The early flush sends the cascading envelope through the transport sender, which then asks IMessageOutbox.DeleteOutgoingAsync (running on a separate connection) to remove the row — but the INSERT is still uncommitted and invisible to that connection, so the DELETE no-ops. The EF Core commit then makes the INSERT visible and the row is left stranded for the durability agent to re-send (at-least-once instead of the exactly-once semantics the outbox is supposed to provide).

Cause: EFCorePersistenceFrameProvider.ApplyTransactionSupport adds the FlushOutgoingMessages postprocessor whenever chain.RequiresOutbox() && chain.ShouldFlushOutgoingMessages(). The second condition is true for HttpChain but false for HandlerChain, which is why the bug was HTTP-only. In Eager mode the chain ALSO has EnrollDbContextInTransaction wrapping the body in a try block ending with efCoreEnvelopeTransaction.CommitAsync(...), and CommitAsync already calls _messaging.FlushOutgoingMessagesAsync() AFTER the DB commit. So in Eager mode the postprocessor is both redundant and unsafe.

Fix: gate the postprocessor add on mode != TransactionMiddlewareMode.Eager. Lightweight mode keeps it (no EnrollDbContextInTransaction wrap, so the postprocessor is the only flush trigger after SaveChangesAsync).

Tests

  • src/Http/Wolverine.Http.Tests/Bug_efcore_outbox_flush_before_commit.cs — failing-then-passing test that asserts the /ef/publish HttpChain has zero FlushOutgoingMessages postprocessors. Inspects the postprocessor collection directly so the assertion works in both Dynamic and Static codegen modes. Confirmed to fail on main and pass after the fix.

  • src/Persistence/EfCoreTests/Bug_efcore_outbox_flush_before_commit.cs — parameterised over TransactionMiddlewareMode.{Eager, Lightweight}, runs:

    1. Codegen check — handler chain never gets a FlushOutgoingMessages postprocessor in either mode (today protected by HandlerChain.ShouldFlushOutgoingMessages=false; locked down so a future change there can't silently introduce a double flush).
    2. Round-trip cleanup invariant — handler with [AutoApplyTransactions] publishes a cascading message to a UseDurableLocalQueues destination, and after TrackActivity completes the wolverine_outgoing_envelopes table is empty. Fills the gap that none of the existing EFCoreTests verified end-to-end outbox-row deletion after a durable send under the EF Core transactional middleware.

Test plan

  • CI: .NET workflow green
  • CI: efcore workflow green
  • CI: http workflow green
  • Local: 4/4 EFCoreTests Bug_efcore_outbox_flush_before_commit pass (2 codegen × 2 modes, 2 cleanup × 2 modes)
  • Local: 6/6 Wolverine.Http.Tests/using_efcore.cs + Bug_efcore_outbox_flush_before_commit pass

🤖 Generated with Claude Code

…n Eager mode

Reported via the sample at https://github.com/dmytro-pryvedeniuk/outbox. An HTTP
endpoint that takes a DbContext and publishes a cascading message generated code
that calls `messageContext.FlushOutgoingMessagesAsync()` BEFORE
`efCoreEnvelopeTransaction.CommitAsync(...)`:

    // Added by EF Core Transaction Middleware
    var result_of_SaveChangesAsync = await _tasksContext.SaveChangesAsync(...);

    // Have to flush outgoing messages just in case Marten did nothing because of #536
    await messageContext.FlushOutgoingMessagesAsync().ConfigureAwait(false);  // <-- early flush

    await efCoreEnvelopeTransaction.CommitAsync(...).ConfigureAwait(false);    // <-- this commits + re-flushes

Sequence at runtime: SaveChangesAsync writes the wolverine_outgoing row inside
the open EF Core transaction (uncommitted). The early flush sends the cascading
envelope through the transport sender, which then asks
IMessageOutbox.DeleteOutgoingAsync (running on a separate connection) to remove
the row — but the INSERT is still uncommitted and invisible to that connection,
so the DELETE no-ops. The EF Core commit then makes the INSERT visible and the
row is left stranded for the durability agent to re-send (at-least-once instead
of the exactly-once semantics the outbox is supposed to provide).

Cause: EFCorePersistenceFrameProvider.ApplyTransactionSupport adds the
FlushOutgoingMessages postprocessor whenever `chain.RequiresOutbox() &&
chain.ShouldFlushOutgoingMessages()` — the second condition is true for HttpChain
but false for HandlerChain, which is why the bug was HTTP-only. In Eager mode the
chain ALSO has EnrollDbContextInTransaction wrapping the body in a try block
ending with `efCoreEnvelopeTransaction.CommitAsync(...)`, and CommitAsync already
calls `_messaging.FlushOutgoingMessagesAsync()` AFTER the DB commit. So in Eager
mode the postprocessor is both redundant and unsafe.

Fix: gate the postprocessor add on `mode != TransactionMiddlewareMode.Eager`.
Lightweight mode keeps it (no EnrollDbContextInTransaction wrap, so the
postprocessor is the only flush trigger after SaveChangesAsync).

Tests:

- src/Http/Wolverine.Http.Tests/Bug_efcore_outbox_flush_before_commit.cs:
  failing-then-passing test that asserts the /ef/publish HttpChain has zero
  FlushOutgoingMessages postprocessors. Inspects the postprocessor collection
  directly rather than the generated source string so the assertion works
  whether codegen ran in Dynamic or Static mode.

- src/Persistence/EfCoreTests/Bug_efcore_outbox_flush_before_commit.cs:
  parameterised over TransactionMiddlewareMode.{Eager, Lightweight}:
    1. Codegen check — handler chain never gets a FlushOutgoingMessages
       postprocessor in either mode (today protected by HandlerChain
       .ShouldFlushOutgoingMessages=false; locked down so a future change
       there can't silently introduce a double flush).
    2. Round-trip cleanup invariant — handler with [AutoApplyTransactions]
       publishes a cascading message to a UseDurableLocalQueues destination,
       and after TrackActivity completes the wolverine_outgoing_envelopes
       table is empty. Fills the gap that none of the existing EFCoreTests
       (per-search) verified end-to-end outbox-row deletion after a durable
       send under the EF Core transactional middleware.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller jeremydmiller merged commit 9e50071 into main May 2, 2026
21 checks passed
@jeremydmiller jeremydmiller deleted the fix-efcore-outbox-double-flush branch May 2, 2026 20:26
jeremydmiller added a commit that referenced this pull request May 2, 2026
Patch release. Single fix:

- WolverineFx.EntityFrameworkCore: suppress the duplicate
  FlushOutgoingMessages postprocessor in Eager-mode HTTP chains so cascading
  messages aren't sent before the EF Core transaction commits. Reported via
  https://github.com/dmytro-pryvedeniuk/outbox. See PR #2663.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant