Save the scraped domain-event envelopes before committing the tenant transaction (GH-3744) - #3754
Merged
Merged
Conversation
…transaction (GH-3744) Under managed database-per-tenant multi-tenancy, a domain event scraped by PublishDomainEventsFromEntityFrameworkCore was published but its durable envelope was never persisted: the aggregate committed and the envelope silently vanished. EFCorePersistenceFrameProvider adds a SaveChangesAsync postprocessor and then appends the commit frame AFTER it, so CommitTenantedDbContextTransaction ran the scrapers and committed with no second save. On an enrolled (Wolverine-mapped) DbContext a durable route persists its envelope by adding an entity to the change tracker (EfCoreEnvelopeTransaction.PersistOutgoingAsync / PersistIncomingAsync), so everything the scrape produced was still sitting in the tracker when the transaction committed. The failure is quiet in the worst way: the domain event is still PUBLISHED in memory, so tracked-session assertions -- including the existing domain_events_scraping_with_managed_multi_tenancy test -- pass while the durable row is missing. Verified with the reporter's own repro from the issue (managed tenancy + durable local queue + testcontainers Postgres): before: Aggregate rows: 1, Durable envelope rows: 0 ("BUG REPRODUCED") after: Aggregate rows: 1, Durable envelope rows: 1 and re-confirmed by reverting this frame alone, which puts the failure straight back. No automated regression test yet. Two attempts built on EfCoreTests.MultiTenancy passed with AND without the fix and were discarded rather than committed as false assurance -- ItemsDbContext is not enrolled by default, so it takes the raw-ADO branch of PersistOutgoingAsync that this change does not touch, and the main store and tenant databases use different schemas, which makes the obvious row assertion look at the wrong table. A valid test needs an enrolled DbContext on the managed-tenancy path. EfCoreEnvelopeTransaction.CommitAsync has the same scrape-then-commit shape on the single-DbContext path and is a suspected sibling defect, but the repro does not exercise it and this change is not needed to fix it, so it is deliberately left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3744.
The bug
Under managed database-per-tenant multi-tenancy, a domain event scraped by
PublishDomainEventsFromEntityFrameworkCoreis published but its durable envelope is never persisted.The aggregate commits; the envelope silently vanishes.
Root cause
EFCorePersistenceFrameProvideradds aSaveChangesAsyncpostprocessor, and thenapplyEagerCommitOrLightweightFlushappends the commit frame after it. SoCommitTenantedDbContextTransactionruns the scrapers and commits with no second save.On an enrolled (Wolverine-mapped)
DbContexta durable route persists its envelope by adding an entityto the change tracker (
EfCoreEnvelopeTransaction.PersistOutgoingAsync/PersistIncomingAsync), soeverything the scrape produced was still sitting in the tracker when the transaction committed.
The failure is quiet in the worst way: the domain event is still published in memory, so
tracked-session assertions pass while the durable row is missing. That includes the existing
domain_events_scraping_with_managed_multi_tenancytest, which asserts ontracked.Sent— it passesboth before and after this change.
The fix
One
SaveChangesAsyncbetween the scrape loop and the commit, so the envelopes land inside thetransaction. A no-op when the scrape produced nothing.
Verification
Run with the reporter's own repro from the issue (managed tenancy + durable local queue + Testcontainers
Postgres), referencing this branch's source directly rather than a packed feed:
6.24.0Aggregate rows: 1/Durable envelope rows: 0→BUG REPRODUCEDAggregate rows: 1/Durable envelope rows: 1→ clean exit0→BUG REPRODUCEDThe passing run also emits the
INSERT INTO wolverine.wolverine_incoming_envelopesthat is entirelyabsent from the baseline output.
Regression check — full
EfCoreTests.MultiTenancy: 78 failed / 115 passed with this change, and78 failed / 115 passed on unmodified
main. Identical; those failures are pre-existing SQL Serverconnect errors in my environment, unrelated to this change.
What this PR deliberately does NOT include
No automated regression test. Two attempts built on
EfCoreTests.MultiTenancypassed with andwithout the fix, and I discarded them rather than commit false assurance. The reasons are worth knowing
for whoever writes the real one:
ItemsDbContextis not enrolled by default, so it takes the raw-ADO branch ofPersistOutgoingAsync— which this change does not touch..Enroll<ItemsDbContext>()is required toexercise the change-tracker path at all.
"count rows in
wolverine.wolverine_outgoing_envelopes" assertion looks at the wrong table and readsa missing table as zero.
A valid test needs an enrolled
DbContexton the managed-tenancy path, asserting against the schema theenrolled context actually maps to. Happy to follow up with that if you'd rather it land together.
EfCoreEnvelopeTransaction.CommitAsyncis left alone. It has the same scrape-then-commit shape onthe single-
DbContextpath and looks like a sibling defect, but the repro does not exercise it and thisfix does not need it. I had that change in and backed it out once I confirmed the codegen frame alone is
sufficient — adding a
SaveChangesAsyncthere would flush any pending tracked change on a widely-usedpath, and I did not want to ship that unverified. Worth its own issue.
🤖 Generated with Claude Code