Outbox relay hardening + inbox accuracy docs#232
Conversation
…lved types in the broker relay
…-event relay cannot resolve a type
…legated poison handling
… races and unbounded host restarts
|
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. 2. Per
Follow the established convention (e.g. |
Summary
Outbox/inbox-hardening work after #226, on one branch. No public API changes (every touched type is
internalor private test-infrastructure).0efc31f,7df4274) — broker + domain-event relay paths.577e29e) — no code; corrects the inbox guarantee/poison-handling documentation.0c7af81) — fixes the flaky integration-test infrastructure behind this PR's CI failures.Outbox relay fixes
BrokerOutboxDispatcher.ResolveTypeandDomainEventOutboxDispatcher.GetOrAddMessageTypereturnednull!for an unresolvable type, cached the null (poisoning the type for the process lifetime), then threw an opaqueNullReferenceExceptiondeep in deserialization. They now throw a descriptiveInvalidOperationExceptionnaming the type, and cache successes only. The dispatcher'sPublishAsync/SendAsyncreflection lookups now match by generic definition + arity instead of name alone.Dictionary<string, object?>; on relay deserialize, System.Text.Json rematerialized every value asJsonElement. BecausePublishContextstores reserved envelope fields (ResponseAddress, FaultAddress, ConversationId, InitiatorId) as strings under header keys and reads them back withvalue is string, those typed getters silently returnednullafter relay. AJsonConverternow normalizes header values back to CLR primitives on deserialize. Known minor limitation: integer headers normalize tolong; reserved string fields round-trip exactly.TransactionalPublishFilter.CreateRownow setsTraceParent/TraceState(gated onOutboxProcessingOptions.EnableTracing, mirroring the domain-event interceptor), soOutboxProcessorparents 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), publishesFault<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 ignoresRetryCount), 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 theIIdempotencyStore/IdempotentConsumeFilterXML.Test-fixture hardening
Both
buildfailures 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/_clientlazy init is nowlock-guarded (was non-atomic??=);ResetScopeswaps the scope out under the lock before disposing;DisposeAsyncistry/finallyso the scope/client/derived factory dispose even ifResetAsync()throws (theCannot access a disposed objectatget_Sendersignature from run 2).BaseWebApplicationFactory.ResetAsync— hosted-service stop/start run under a bounded 30s timeout instead ofCancellationToken.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
AppHostFixtureDCP-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) andVulthil.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.