Skip to content

GH-4059: reject conflicting sending modes on shared listeners - #4506

Closed
tmorejon wants to merge 3 commits into
JasperFx:mainfrom
tmorejon:GH-4059-shared-endpoint-mode-collision
Closed

tmorejon wants to merge 3 commits into
JasperFx:mainfrom
tmorejon:GH-4059-shared-endpoint-mode-collision

Conversation

@tmorejon

Copy link
Copy Markdown
Contributor

Summary

Reject conflicting sending and listening modes on a shared endpoint at startup, regardless of configuration order.

Root cause

Sending and listening use the same Endpoint.Mode. SendInline(), UseDurableOutbox(), and BufferedInMemory() assign it through delayed configuration, so whichever configuration ran last could silently change the other direction’s behavior.

Changes

Record the listener’s intended mode and any explicitly configured sending mode. After delayed configuration is applied, ListenerConfigurationValidator throws an InvalidListenerConfigurationException when they conflict. Send-only endpoints and shared endpoints with matching modes remain valid.

Added regression tests for all three sending methods in both configuration orders, plus startup failure and valid configurations.

Verification

  • Reproduced the missing validation before the fix.
  • Wolverine CoreTests target passed: full solution build and 3,022 tests on .NET 9.
  • All 37 listener validation tests passed on .NET 10.
  • Relevant project builds and git diff --check passed.

Fixes #4059

Sending and listening shared Endpoint.Mode, so delayed configuration order
could silently change listener behavior. Record both requested modes and
reject conflicts at startup. Added regression tests for all three sending
methods in both orders.

AI model used: OpenAI Codex (GPT-5).
…dropping SendInline()

These two endpoints relied on SendInline() sharing Endpoint.Mode to put their
LISTENER in Inline as well, which is the very behavior this PR now rejects.
Removing SendInline() resolves the conflict, but it also silently drops each
listener back to BufferedInMemory -- and both tests assert on behavior only the
Inline listening path produces:

- Pulsar: UseNativeRedelivery() is implemented in PulsarListener.DeferAsync,
  reached only when the listener is itself the channel callback. Buffered, the
  failure is handled in-process, Pulsar never redelivers, and the test times out
  after 30s.
- Redis: an Inline listener dead-letters through MoveToErrorsAsync, which writes
  the exception-type / failed-at / attempts fields these tests read. A buffered
  one goes through TryBuildDeadLetterSender and the envelope mapper instead,
  which writes a different shape -- and trips the duplicate
  wolverine-wolverine-protocol-version field reported as JasperFxGH-4595.

Ask for Inline on both directions explicitly so the endpoints keep the mode they
actually had, and the validator sees a matching pair.

Full suites green: Wolverine.Redis.Tests 246/246, Wolverine.Pulsar.Tests 252/252.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jeremydmiller

Copy link
Copy Markdown
Member

Thank you for this, and apologies for how it played out — the duplication is on our side, not yours.

You opened this on 2026-09-20. #4582 was written later without spotting that this PR already existed, and that is the one that got merged, so this branch now conflicts with main. That should not have happened and I am sorry for the wasted effort.

On the substance, your diagnosis was exactly right — the root cause paragraph in your description is precisely it: sending and listening share Endpoint.Mode, all three sending methods assign it through delayed configuration, and whichever block ran last silently decided both directions. Recording the listener's intended mode and the explicitly configured sending mode, then validating after delayed configuration is applied, is the same design that landed.

The one place we went a different way is the verdict. #4582 warns rather than throwing, because a single Mode property cannot express "send inline, receive durably" — a coherent thing to want — so rejecting refuses a configuration that is meaningful rather than mistaken, with no remedy but giving one of the two calls up. Separating the two into a distinct SendingMode is the real fix and needs its own migration story; that is where a rejection belongs. #4582 did keep a hard failure for the case where a processing guarantee is actually lost (PartitionProcessingByGroupId() on an endpoint a publish rule reset to Inline), and made that message name SendInline() as the cause.

Your 27 transport-test fixes are being landed separately, with you as the commit author: #4597. They are real misconfigurations regardless of warn-vs-reject, and #4582's warning now reports every one of them. The Redis dead letter ones in particular were worth keeping — those tests were relying on the bug, taking their listener's Inline mode from the SendInline() on the publishing side, which your change makes explicit with ProcessInline(). That is a latent trap anyone tidying up that call would have hit.

Closing this in favour of #4582 (core fix) + #4597 (your test fixes). Please do keep sending PRs — and if you want to take the SendingMode split that would remove this whole class of problem at the root, that one is still open.

jeremydmiller added a commit that referenced this pull request Sep 24, 2026
… test suites (#4597)

Extracted from @tmorejon's #4506, which found these while building a validator for GH-4059 and is the
original work here. The core fix landed separately as #4582 with a WARNING rather than a rejection --
"send inline, receive durably" is a coherent thing to want that a single Endpoint.Mode cannot express --
but these 27 files are real misconfigurations either way, and #4582's warning now reports every one of
them.

Most are a `.SendInline()` on a publish rule for an endpoint whose listener separately asked for
UseDurableInbox() or ProcessInParallelWithNativeAcks(); whichever configuration block Wolverine applied
last silently decided the mode for both directions.

The Redis dead letter tests are the interesting ones, and the reason this is worth landing rather than
leaving as noise: they were RELYING on the bug. Their listener needs Inline, because only an Inline
listener takes RedisStreamListener.MoveToErrorsAsync -- a buffered one dead-letters through
TryBuildDeadLetterSender instead -- and it was silently getting that from the SendInline() on the
publishing side sharing Endpoint.Mode. They now say ProcessInline() outright.

Verified: Wolverine.Redis.Tests DeadLetterQueueTests 6 green, which is where these fixes overlap the
GH-4559 rewrite of disabled_dead_letter_queue_should_not_create_dead_letter_stream (#4579).


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

Co-authored-by: Jorge L. Torres M <tmorejon.jl@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@tmorejon

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed explanation and for preserving the transport test fixes with my authorship.

The warn-vs-reject reasoning makes sense, especially given that a single Mode cannot represent different send/receive behavior.

I appreciate you splitting out the test fixes. I'll keep an eye on #4597, and I'd be interested in looking at the SendingMode split as a follow-up.

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.

SendInline() on a shared endpoint silently overwrites the listener's EndpointMode

2 participants