Escalated by the local adversarial review of the moscow:must sweep branch bugfix/asb-small-musts-375-377-374 (which closed #375, #377 and #374). Pre-existing, outside that sweep's fixed membership, and its fix is a public failure-contract decision — so it is filed here with full scope rather than absorbed.
Root cause
ServiceBusMessageSender.Dispatch(IEnumerable<OutboundBrokeredMessage>, TransactionContext) starts each send inside an unguarded foreach and collects the tasks, only awaiting them after the loop completes. There is no try/catch around the loop, so any throw during enumeration exits the method with sends already in flight and never awaited.
Three fault sources: the caller's lazy sequence throwing on MoveNext, _senderFactory.Create(destination) throwing for a later element, and AsAzureServiceBusMessage() throwing on a later element.
References
src/Chatter.MessageBrokers.AzureServiceBus/src/Chatter.MessageBrokers.AzureServiceBus/Sending/ServiceBusMessageSender.cs:68-75 — the unguarded loop and the await Task.WhenAll it can skip
src/Chatter.MessageBrokers.AzureServiceBus/src/Chatter.MessageBrokers.AzureServiceBus/Sending/ServiceBusMessageSender.cs:66 — the using var scope disposed on the way out
src/Chatter.MessageBrokers/src/Chatter.MessageBrokers/Sending/IMessagingInfrastructureDispatcher.cs — the contract, which is silent on failure semantics
Two defects
- Unobserved task exceptions. Sends started before the fault are dropped on the floor. If any later faults, it surfaces as a
TaskScheduler.UnobservedTaskException at finalization — off-thread, unattributable, and potentially process-level depending on host configuration.
- Caller-visible partial dispatch. The caller observes the enumeration fault and has no way to learn that messages
1..k-1 were already handed to Azure Service Bus and may well have been delivered. Retrying the batch duplicates them.
Bounded impact
Real but narrow.
- Requires a fault during enumeration. The common failure — a send itself failing — is fully handled, because
Task.WhenAll observes it.
- Not a transaction-enlistment hazard. Under
TransactionScopeAsyncFlowOption.Enabled the ambient-transaction state flows on the ExecutionContext captured when each send started — inside the using, while suppression was active — so disposing the scope on the initiating thread cannot cause an in-flight send to retroactively enlist. Worth re-confirming against the SDK whenever this is fixed: in Azure.Messaging.ServiceBus 7.20.2, enlistment reads Transaction.Current inside AmqpSender.SendBatchInternalAsync, after the retry-policy and link-acquisition awaits, not in the synchronous prefix of SendMessagesAsync.
- Duplicate delivery is already a broker-level reality consumers must absorb. This widens the window rather than introducing a new class of failure.
Why this was deferred
Pre-existing. master's non-async Dispatch carries the identical unguarded foreach inside the identical using, so the 2.4.1 sweep did not introduce it. That release in fact narrowed the exposure: before it, the scope was disposed before any send completed on every dispatch; it is now confined to this fault path.
Fixing it means deciding IMessagingInfrastructureDispatcher.Dispatch's failure contract — whether the caller waits for in-flight sends before seeing the enumeration fault, which exception wins (the enumeration fault, an AggregateException, or a new partial-dispatch type), and whether partial-dispatch state becomes caller-visible. That is a public-contract decision affecting every implementation (ServiceBusMessageSender, RabbitMqSender, SqlServiceBrokerSender), it needs test-first development, and it fell outside the fixed six-issue sweep.
Related
Parent epic: #307
Escalated by the local adversarial review of the
moscow:mustsweep branchbugfix/asb-small-musts-375-377-374(which closed #375, #377 and #374). Pre-existing, outside that sweep's fixed membership, and its fix is a public failure-contract decision — so it is filed here with full scope rather than absorbed.Root cause
ServiceBusMessageSender.Dispatch(IEnumerable<OutboundBrokeredMessage>, TransactionContext)starts each send inside an unguardedforeachand collects the tasks, only awaiting them after the loop completes. There is notry/catcharound the loop, so any throw during enumeration exits the method with sends already in flight and never awaited.Three fault sources: the caller's lazy sequence throwing on
MoveNext,_senderFactory.Create(destination)throwing for a later element, andAsAzureServiceBusMessage()throwing on a later element.References
src/Chatter.MessageBrokers.AzureServiceBus/src/Chatter.MessageBrokers.AzureServiceBus/Sending/ServiceBusMessageSender.cs:68-75— the unguarded loop and theawait Task.WhenAllit can skipsrc/Chatter.MessageBrokers.AzureServiceBus/src/Chatter.MessageBrokers.AzureServiceBus/Sending/ServiceBusMessageSender.cs:66— theusing var scopedisposed on the way outsrc/Chatter.MessageBrokers/src/Chatter.MessageBrokers/Sending/IMessagingInfrastructureDispatcher.cs— the contract, which is silent on failure semanticsTwo defects
TaskScheduler.UnobservedTaskExceptionat finalization — off-thread, unattributable, and potentially process-level depending on host configuration.1..k-1were already handed to Azure Service Bus and may well have been delivered. Retrying the batch duplicates them.Bounded impact
Real but narrow.
Task.WhenAllobserves it.TransactionScopeAsyncFlowOption.Enabledthe ambient-transaction state flows on the ExecutionContext captured when each send started — inside theusing, while suppression was active — so disposing the scope on the initiating thread cannot cause an in-flight send to retroactively enlist. Worth re-confirming against the SDK whenever this is fixed: inAzure.Messaging.ServiceBus7.20.2, enlistment readsTransaction.CurrentinsideAmqpSender.SendBatchInternalAsync, after the retry-policy and link-acquisition awaits, not in the synchronous prefix ofSendMessagesAsync.Why this was deferred
Pre-existing.
master's non-asyncDispatchcarries the identical unguardedforeachinside the identicalusing, so the 2.4.1 sweep did not introduce it. That release in fact narrowed the exposure: before it, the scope was disposed before any send completed on every dispatch; it is now confined to this fault path.Fixing it means deciding
IMessagingInfrastructureDispatcher.Dispatch's failure contract — whether the caller waits for in-flight sends before seeing the enumeration fault, which exception wins (the enumeration fault, anAggregateException, or a new partial-dispatch type), and whether partial-dispatch state becomes caller-visible. That is a public-contract decision affecting every implementation (ServiceBusMessageSender,RabbitMqSender,SqlServiceBrokerSender), it needs test-first development, and it fell outside the fixed six-issue sweep.Related
Parent epic: #307