Skip to content

Outbox relay hardening + inbox accuracy docs#232

Merged
Vulthil merged 4 commits into
mainfrom
fix/outbox-broker-relay
Jun 16, 2026
Merged

Outbox relay hardening + inbox accuracy docs#232
Vulthil merged 4 commits into
mainfrom
fix/outbox-broker-relay

Conversation

@Vulthil

@Vulthil Vulthil commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Summary

Outbox/inbox-hardening work after #226, on one branch. No public API changes (every touched type is internal or private test-infrastructure).

  • Outbox relay hardening (0efc31f, 7df4274) — broker + domain-event relay paths.
  • Inbox accuracy docs (577e29e) — no code; corrects the inbox guarantee/poison-handling documentation.
  • Test-fixture hardening (0c7af81) — fixes the flaky integration-test infrastructure behind this PR's CI failures.

Outbox relay fixes

  • Type resolution no longer NREs. BrokerOutboxDispatcher.ResolveType and DomainEventOutboxDispatcher.GetOrAddMessageType returned null! for an unresolvable type, cached the null (poisoning the type for the process lifetime), then threw an opaque NullReferenceException deep in deserialization. They now throw a descriptive InvalidOperationException naming the type, and cache successes only. The dispatcher's PublishAsync/SendAsync reflection lookups now match by generic definition + arity instead of name alone.
  • Header values survive the relay round-trip. Broker-publish metadata serialized headers as Dictionary<string, object?>; on relay deserialize, System.Text.Json rematerialized every value as JsonElement. Because PublishContext stores reserved envelope fields (ResponseAddress, FaultAddress, ConversationId, InitiatorId) as strings under header keys and reads them back with value is string, those typed getters silently returned null after relay. A JsonConverter now normalizes header values back to CLR primitives on deserialize. Known minor limitation: integer headers normalize to long; reserved string fields round-trip exactly.
  • Trace context captured on the broker path. TransactionalPublishFilter.CreateRow now sets TraceParent/TraceState (gated on OutboxProcessingOptions.EnableTracing, mirroring the domain-event interceptor), so OutboxProcessor parents the relay span on the originating trace.

Inbox accuracy docs

Investigation of the "inbox poison-handling" backlog item concluded it needs no code: the RabbitMq transport already bounds retries (RetryPolicyDefinition.MaxRetryCount), publishes Fault<T>, and nacks to the dead-letter exchange (RabbitMqConsumerWorker), and the inbox filter runs inside that loop — an inbox-level max-attempts would be redundant and lossier. The docs now scope "exactly-once" to a transactional store (effectively-once on Cosmos), document the transport-delegation boundary (the guard ignores RetryCount), and document the concurrent-duplicate caveat (only the marker insert is serialized, so concurrent duplicates can each run the handler body once) — across the inbox README, docs/articles/inbox-pattern.md, the package doc, and the IIdempotencyStore / IdempotentConsumeFilter XML.

Test-fixture hardening

Both build failures on this PR were flaky integration-test infrastructure, not the changes (880/883 pass; different suites failed on consecutive runs). Fixed the races:

  • BaseIntegrationTestCase_scope/_client lazy init is now lock-guarded (was non-atomic ??=); ResetScope swaps the scope out under the lock before disposing; DisposeAsync is try/finally so the scope/client/derived factory dispose even if ResetAsync() throws (the Cannot access a disposed object at get_Sender signature from run 2).
  • BaseWebApplicationFactory.ResetAsync — hosted-service stop/start run under a bounded 30s timeout instead of CancellationToken.None, so a hung restart can't hang the suite.
  • BaseWebApplicationFactory.DisposeAsync — disposes containers even when initialization didn't complete, fixing a container leak that adds cumulative Docker pressure in CI (a likely contributor to the Aspire DCP-startup timeout from run 1).

Verified locally with Docker on Release: WebApi.Tests 8/8 and Vulthil.IntegrationTests 9/9 pass. The Aspire AppHostFixture DCP-startup timeout is environmental/Aspire-internal and not directly changed; the container-leak fix may relieve the resource pressure behind it.

Tests

New unit-test projects Vulthil.Messaging.Outbox.Tests (3 tests) and Vulthil.SharedKernel.Outbox.Tests (1 test): primitive-type preservation, typed-getter survival across the full capture→serialize→deserialize→reapply flow, and descriptive-throw on unresolved types for both dispatchers. Full Release solution build is clean.

@Vulthil Vulthil changed the title fix(outbox): harden relay type resolution and broker header/trace round-trip Outbox relay hardening + inbox accuracy docs Jun 16, 2026
@claude

claude Bot commented Jun 16, 2026

Copy link
Copy Markdown

Code review — bugs and CLAUDE.md compliance.

No correctness, concurrency, security, or resource-leak issues found in the changed code (JsonConverter round-trip, reflection matching by generic-definition + arity, lock-guarded init/dispose, and bounded CancellationTokenSource usage all check out). Found 2 CLAUDE.md violations, both the same rule.

1. tests/Vulthil.Messaging.Outbox.Tests/BrokerOutboxDispatcherTests.cs (L12) — the SUT is created inline with CreateInstance<BrokerOutboxDispatcher>() instead of the required lazy Target pattern.

2. tests/Vulthil.SharedKernel.Outbox.Tests/DomainEventOutboxDispatcherTests.cs (L11) — same: SUT created inline with CreateInstance<DomainEventOutboxDispatcher>().

Per .github/copilot-instructions.md:

Use a lazy Target pattern from CreateInstance<T> when inheriting from BaseUnitTestCase (or BaseUnitTestCase<T> when accessibility allows) for test classes in this repository.
Override the CreateInstance or CreateInstance methods and use the Target property to lazily create the instance under test.

Follow the established convention (e.g. DomainEventPublisherTests): add a Lazy<T> field, expose a Target property, wire it in the constructor via CreateInstance<T>, and reference Target in the test body instead of the local dispatcher.

@Vulthil
Vulthil merged commit 34f2ed2 into main Jun 16, 2026
5 checks passed
@Vulthil
Vulthil deleted the fix/outbox-broker-relay branch June 16, 2026 16:26
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