Skip to content

GH-4556: apply an ISendMyself published from a projection side effect - #4557

Closed
erdtsieck wants to merge 2 commits into
JasperFx:mainfrom
erdtsieck:bugfix/projection-side-effect-send-myself
Closed

erdtsieck wants to merge 2 commits into
JasperFx:mainfrom
erdtsieck:bugfix/projection-side-effect-send-myself

Conversation

@erdtsieck

Copy link
Copy Markdown
Contributor

Closes #4556.

Problem

MartenToWolverineMessageBatch always passes DeliveryOptions (at least the tenant id) to PublishAsync. MessageBus.PublishAsync only lets an ISendMyself apply itself when options == null. So a ToWebSocketGroup(), ToEndpoint(), DelayedFor(), ... published from RaiseSideEffects gets routed as the wrapper type and ends up as NoRoutes.

Fix

The batch now spots an ISendMyself and applies it against its own ProjectionSideEffectContext:

  • a MessageContext enlisted in the same Marten session through MartenEnvelopeTransaction, with the batch's storage, so durable sends still commit with the projection
  • flushed in AfterCommitAsync after the batch's own context
  • it overrides TrackEnvelopeCorrelation, the point every outgoing envelope passes through (publish, EndpointFor, topics), to stamp the side effect's tenant, correlation id, causation id and headers. Anything the message set on its own options wins.

I used a context per message instead of changing TenantId/CorrelationId on the shared batch context for the duration of ApplyAsync, because slices can publish concurrently.

I left the options == null guard in MessageBus alone, since it protects against the TimeoutMessage loop.

Tests

Bug_4556_projection_side_effects_honor_send_myself covers ToEndpoint() from an async projection, with metadata (correlation, causation, custom header asserted on the received envelope) and without. Both failed on main with NoRoutes for RoutedToEndpointMessage<SendMyselfCommand>.

Also still green locally: Bug_2545, Bug_4268, Bug_2382, Bug_2887, end_to_end_publish_messages_through_marten_to_wolverine, multi_tenancy_queue_usage, and the MartenTests.AncillaryStores namespace (46 tests).

…e effect

MartenToWolverineMessageBatch always hands DeliveryOptions to PublishAsync, and
MessageBus only lets an ISendMyself apply itself when there are none, so
ToWebSocketGroup(), ToEndpoint() and friends were routed as the wrapper type and
dropped as NoRoutes.

Each such message is now applied against its own MessageContext, enlisted in the
projection batch's Marten session and flushed after commit, which stamps the side
effect's tenant, correlation id, causation id and headers on whatever it sends.
Options the message sets itself win.
jeremydmiller added a commit that referenced this pull request Sep 23, 2026
`unknown_tenant_problem_details_4516.without_the_opt_in_the_unknown_tenant_still_escapes`
went red on CI the moment #4551 merged, and it takes the whole `test` job with it -- so
every open PR is red regardless of its contents (#4553, #4557, #4558 at the time of
writing, two of them from contributors).

The test was self contradictory. It asked Alba to assert a 404 inside a scenario it
expected to THROW:

    await Should.ThrowAsync<UnknownTenantIdException>(async () =>
        await host.Scenario(x =>
        {
            x.Get.Url("/gh4516/tenanted?tenantId=ghost");
            x.StatusCodeShouldBe(404);      // <- asserted on a request expected to blow up
        }));

Locally the UnknownTenantIdException propagated before Alba evaluated its assertions, so
the expected exception won and the test passed. On CI the host turned the exception into
a 500 first, so Alba's own 404 assertion fired and raised ScenarioAssertionException
instead of UnknownTenantIdException -- a different type, so Should.ThrowAsync failed.

The control's actual purpose is narrower than what it asserted: it exists to show that
MapUnknownTenantToNotFound() is what produces the 404, not something else in the pipeline.
It is not a claim about HOW an unmapped failure surfaces, and that is exactly the part
that varies by environment. It now uses IgnoreStatusCode() and asserts only that the
response is not a mapped 404, tolerating the other surfacing -- the exception escaping the
scenario -- and nothing else.


Claude-Session: https://claude.ai/code/session_01VDUrBeB4tTnKj4AExCS1nj

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jeremydmiller

Copy link
Copy Markdown
Member

Thank you for this @erdtsieck — the diagnosis was exactly right, and the ProjectionSideEffectContext
approach (stamping the metadata in TrackEnvelopeCorrelation, the one point every outgoing envelope
passes through, rather than mapping it onto a single DeliveryOptions) is the design we kept.

Closing in favour of #4575, which carries your commit authorship and adds the review follow-up:

  • The correlation id went null. MessageMetadata.CorrelationId is plain null unless the
    projection set one, so taking it over unconditionally stamped null — stripping the correlation id
    that every other side-effect message out of the same batch carries. Reproduced with a plain message
    published from the same slice as the control; it now falls back to the batch context's id.
  • _sendsThemselves was never cleared. Marten only discards its cached message batch after a
    successful commit (SaveChangesAsync doesn't reset it in a finally), so a session retrying a
    failed commit gets the same batch back and would accumulate — and re-flush — every context from
    every failed attempt for as long as it lived. Now cleared on flush, lazily allocated, and the
    Add/iterate locking is symmetric.
  • Polecat and Fisher had the identical defect. PolecatToWolverineMessageBatch and
    FisherToWolverineMessageBatch were byte-identical to Marten's down to the doc comments, and both
    still dropped every ISendMyself as NoRoutes — confirmed with a red baseline on all three stores.
    Rather than triplicate the fix, the behaviour moved into a store-agnostic
    ProjectionSideEffectSink that all three bridges delegate to.
  • ToTopic() was the one wrapper that still wouldn't have worked. BroadcastToTopicAsync never
    called TrackEnvelopeCorrelation at all, so it carried no causation id or custom headers (and no
    Source/CorrelationId/Store either — for every topic broadcast, not just side effects). Fixed
    in its own commit there so it can be taken separately.

One note on the test: the schema clean ran after StartAsync(), which drops mt_event_progression
out from under a daemon that is already polling it — a reliable source of CI flakiness. #4575 drops
the schema before the host starts instead.

jeremydmiller added a commit that referenced this pull request Sep 23, 2026
…, on all three stores (#4575)

MessageBus only lets an ISendMyself apply itself when no DeliveryOptions are
passed -- the guard that keeps TimeoutMessage from looping. A projection side
effect always passes some, at minimum the tenant id, so a ToEndpoint(),
DelayedFor() or SignalR ToWebSocketGroup() published out of RaiseSideEffects was
routed as the wrapper type, found no subscriber, and was dropped as NoRoutes.

The bridge now spots an ISendMyself and applies it against its own
ProjectionSideEffectContext: a MessageContext enlisted in the same store session,
with the batch's storage, so a durable send still commits with the projection.
The metadata is stamped in TrackEnvelopeCorrelation -- the one point every
outgoing envelope passes through -- rather than mapped onto a single
DeliveryOptions that only the outermost send would have seen.

Beyond the original diagnosis in #4557:

- The correlation id is only taken over when the projection actually supplied
  one. MessageMetadata.CorrelationId is plain null otherwise, so stamping it
  unconditionally stripped the correlation id every other side effect message
  from the same batch carries. It falls back to the batch context's id now.

- The side effect contexts are cleared once flushed. Marten only discards its
  cached message batch after a SUCCESSFUL commit -- SaveChangesAsync does not
  reset it in a finally -- so a session retrying a failed commit gets the same
  batch back and would otherwise accumulate, and re-flush, every context from
  every failed attempt for as long as the session lived.

- Polecat and Fisher had the identical defect. The behaviour moved into the store
  agnostic ProjectionSideEffectSink and all three IMessageBatch implementations
  delegate to it, so the three bridges cannot drift apart again.

- BroadcastToTopicAsync never called TrackEnvelopeCorrelation at all, so a topic
  broadcast went out with no Source, CorrelationId, ParentId or Store -- for
  every topic send in every application, not just projection side effects.

Supersedes #4557. Closes #4556.

Co-Authored-By: Anne Erdtsieck <anne.erdtsieck@topicus.nl>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 24, 2026
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.

ISendMyself published from a projection side effect is dropped (e.g. ToWebSocketGroup, ToEndpoint)

2 participants