Publish Then Save, Save Then Publish

Part 12 established that GroupFlights' dispatcher delivers at-most-once with no outbox and no replay. That guarantee interacts with a second decision every handler makes and almost none of them documents: do you save first and then publish, or publish first and then save?

The estate answers both ways. Five times one way, once the other, and the one is a payment.

The safe ordering, five times

// src/Sales/.../Commands/AcceptOffer/AcceptOfferCommandHandler.cs:29-39
public async Task HandleAsync(AcceptOfferCommand command, CancellationToken cancellationToken = default)
{
    var offer = await _offerRepository.GetOfferById(command.OfferId, cancellationToken);
    var user = _userContextAccessor.Get();
    offer.AcceptVariant(command.VariantId, _clock, user?.UserId);

    await _offerRepository.UpdateOffer(offer, cancellationToken);
    await _eventDispatcher.PublishMultiple(offer.DomainEvents
        .Select(@event => @event.RemapToPublicEvent())
        .SelectMany(e => e), cancellationToken);
}

Load, mutate, persist, then publish. UpdateOffer ends in SaveChangesAsync, so by the time PublishMultiple runs the state change is committed. If the save throws, the handler never reaches the publish, and no event is emitted for something that did not happen.

The same shape appears at SetReservationChangeFeasibilityCommandHandler.cs:36-40, ChangeReservationCommandHandler.cs:37, MarkTicketsIssuedCommandHandler.cs:32, and twice in ReservationConfirmationService. Five handlers, one convention, held.

Under at-most-once delivery, save-then-publish gives you the failure mode you want: the database may be ahead of the announcement, and it is never behind. Downstream modules can be stale; they cannot be wrong. Every reconciliation strategy in existence assumes exactly that, and it is why the outbox pattern exists — an outbox is save-then-publish with the publish made durable.

Note also what makes the ordering possible here: offer.DomainEvents is available after the save because the aggregate holds its own event queue. The estate collects events in the model rather than in a SaveChanges interceptor, which means the handler chooses the ordering explicitly. That is more visible than a framework doing it, and correspondingly easier to get wrong.

The inverted ordering, once

// src/Finance/GroupFlights.Finance.Core/Services/PaymentService.cs:85-91
var updatedPayment = payment with { Payed = true };
_dbContext.Entry(payment).CurrentValues.SetValues(updatedPayment);

await _eventDispatcher.PublishAsync(new PaymentCompleted(payment.PaymentId), cancellationToken);

await _dbContext.SaveChangesAsync(cancellationToken);

Mutate the tracked entity, publish, then save.

PublishAsync enqueues into the static ConcurrentQueue and returns a completed task. It cannot be undone. So if SaveChangesAsync throws — a constraint violation, a connection drop, a timeout — the transaction rolls back, Payments.Payed stays false, and PaymentCompleted is already sitting in the queue waiting for the next tick.

Within five seconds, that phantom event fans out. Sales' PaymentCompletedEventHandler looks the payment up in PaymentRegistry, finds the reservation, and marks a required payment as settled. Postsale's handler does the equivalent for a change request and calls OnChangePayed(). Two modules now believe money arrived for a payment row that says it did not.

There is no compensating action. There is no way to un-publish. There is no correlation id to trace the fan-out back to the failed save. And because Task.WhenAll in the drain loop reports only the first of N exceptions (Part 12), even the handler failures that do occur may not all be logged.

The fix is moving one line down two. It is genuinely that small, and that is the point: the difference between a correct estate and a phantom-event estate is the position of a single statement, and nothing in the compiler, the type system or the test suite has an opinion about it.

The mirror hazard, four lines up the file

The same method's sibling has the same problem pointed at an external system.

// PaymentService.cs, SetupPayment (line numbers relative to the method)
await _dbContext.AddAsync(new Payment(
    paymentSetup.PaymentId,
    paymentSetup.PayerId,
    paymentSetup.Amount,
    paymentSetup.DueDate,
    secret,
    false
), cancellationToken);

await _paymentGatewayFacade.SetUpPayment(paymentSetup, payer, secret, cancellationToken);

await _dbContext.SaveChangesAsync(cancellationToken);

The HTTP call to the payment gateway fires before the local save. A failed save leaves a payment live at the gateway and absent from the database.

And then the trace runs all the way through, which is what makes this the estate's richest single teaching artefact. The gateway holds the payment in a queue and auto-pays on its own five-second timer:

// src/_ExternalSystems/FakePaymentGateway/FakePaymentProcessor.cs:63-80
public async Task AutoPay(SetUpPaymentWithMetadata paymentToAutoPay)
{
    var httpClientFactory = _serviceProvider.GetService<IHttpClientFactory>();
    var httpClient = httpClientFactory.CreateClient("GroupFlights.Finance");

    var result = await httpClient.PostAsJsonAsync(paymentToAutoPay.WebhookUrl,
        new PaymentWebhookDto(paymentToAutoPay.PaymentId, paymentToAutoPay.Secret),
        CancellationToken.None);

    try
    {
        result.EnsureSuccessStatusCode();
    }
    catch (Exception ex)
    {
        _logger.LogError(ex.Message, ex);
    }
}

Follow it. The webhook arrives at /finance/payment-webhook. OnPaymentPayed looks the payment up, finds nothing, throws DoesNotExistException, which maps to a 404. Back in the gateway, EnsureSuccessStatusCode() throws, the catch logs it — and the payment was already removed from the queue by TryDequeue before AutoPay was ever called.

Both legs of the payment conversation are at-most-once. The gateway does not retry, does not persist, and has no dead-letter path; a restart loses everything pending, leaving Payments.Payed = false forever with no reconciliation job anywhere in the estate. Money is “paid” at the gateway and the system will never learn about it.

Three smaller details in that method compound it, and each is a distinct lesson:

• The PostAsJsonAsync is outside the try. Only EnsureSuccessStatusCode() is inside it. A connection failure — the most likely error, given both processes bind HTTPS on localhost and dotnet dev-certs https --trust is an undocumented prerequisite — throws from line 68 and escapes into DoWork's catch, taking the whole batch with it.
• _logger.LogError(ex.Message, ex) has its arguments transposed. The message becomes the template and the exception becomes a format argument, so the stack trace is dropped and a message containing a brace corrupts the entry. Two files over, the same estate gets it right.
• CreateClient("GroupFlights.Finance") asks for a named client that was never configured. AddHttpClient() registers only the default factory, so an unregistered name silently returns a default client — no base address, no timeout, no retry policy, no circuit breaker. The name is decorative, and the same mistake appears in the other direction on the Finance side.

The webhook is not idempotent

One more, because it closes the loop. OnPaymentPayed has no if (payment.Payed) return; guard. A duplicate delivery re-runs the secret comparison, re-sets Payed = true, and re-publishes PaymentCompleted, which both Sales and Postsale handle again.

Webhooks are re-delivered by definition. Every real gateway retries; this fake one happens not to, which is the only reason the missing guard is currently harmless. The estate is protected by a property of its test double, not by its own code.

What to actually take away

The pair is the lesson, not the defect. One repository, one team, one week of work, containing the same decision made correctly five times and incorrectly once — with the incorrect one landing on the flow that moves money.

Three rules fall out of it, in ascending order of how much they cost to ignore:

1. Persist, then announce. Under at-most-once delivery this is the only ordering whose failure mode is recoverable. Stale is survivable; wrong is not.
2. Treat an external HTTP call as a commit. SetupPayment fails not because the ordering of two local operations is wrong but because one of them is not local. Anything you cannot roll back belongs after everything you can.
3. Idempotence is not optional at a boundary you do not control. The guard costs one line, and the only thing standing in for it here is a test double that happens not to retry.

Next, five seconds is a teaching decision — why the tick exists at all, what it costs the README's own walkthrough, and the DI-scope trap waiting one refactor away.