BrighterCommand / BrighterCommand/Brighter

Generated Azure Service Bus conformance tests never receive: the subscription is created lazily, after the send

Open
#4,309 2 comments 0 reactions 1 assignee Claimed by @iancooper View on GitHub
Bug Maintenance
Dominant language
C#
Stars
2.5k
Forks
296
Avg merge
1d 11h
Merged PRs (30d)
21

Description

## Summary

The generated Azure Service Bus conformance tests (#4240) never receive the message they send. The cause
is an ordering gap: **`AzureServiceBusChannelFactory` provisions no infrastructure**, and the ASB
subscription is instead created lazily on the first *receive*. The test sends before that happens, and an
ASB topic with no subscription attached silently **discards** the message.

## The mechanism

1. `AzureServiceBusChannelFactory.CreateAsyncChannel`
(`src/Paramore.Brighter.MessagingGateway.AzureServiceBus/AzureServiceBusChannelFactory.cs:75-87`)
constructs a `ChannelAsync` around a consumer and returns. It creates nothing on the broker.
`CreateAsyncChannelAsync` (line 97-99) is just `Task.FromResult(CreateAsyncChannel(...))`.
2. The subscription is created inside `AzureServiceBusTopicConsumer.EnsureChannelAsync`
(`AzureServiceBusTopicConsumer.cs:79`, the `CreateSubscriptionAsync` call is at `:98`), which runs
only from `AcknowledgeAsync` (`AzureServiceBusConsumer.cs:106`), `ReceiveAsync` (`:177`),
`NackAsync` (`:243`), `RejectAsync` (`:300`) and `PurgeAsync`
(`AzureServiceBusTopicConsumer.cs:76`). There is no sync `EnsureChannel` — the abstract member at
`AzureServiceBusConsumer.cs:372` is async-only and every sync entry point is sync-over-async, so
both pumps hit the same lazy path.
3. Every generated test does: `CreateProducerAsync` → `CreateChannelAsync` (creates **nothing**) →
**send** → receive. Note that `CreateProducerAsync` does **not** create the topic either:
`AzureServiceBusMessageProducerFactory.CreateAsync` is `Task.FromResult(Create())` (`:96-99`) and
`Create()` (`:67-94`) only `new`s up producer instances. The **topic** is created lazily during
the send, via `SendWithDelayAsync` → `GetSenderAsync` → `EnsureChannelExistsAsync`
(`AzureServiceBusMessageProducer.cs:248`), and
`AzureServiceBusTopicMessageProducer.EnsureChannelExistsAsync` (`:61-88`) creates the topic
**only**, never a subscription.
4. So at the instant of publish the topic has just been created microseconds earlier and provably has
zero subscriptions. The message is dropped. The first `ReceiveAsync` then creates the
subscription — too late. Every read returns `MT_NONE`.

## Evidence

From the `azure-ci` job of run
[`34127954869` / job `101763183832`](https://github.com/BrighterCommand/Brighter/actions/runs/34127954869),
which ran the ASB conformance suites un-skipped: `Total tests: 161 / Passed: 131 / Failed: 30`. The 30
failures split into three distinct causes:

| count | failure | cause |
| --- | --- | --- |
| **26** | `Assert.NotEqual() Failure: Values are equal` (the received message was `MT_NONE`) | **this issue** |
| 2 | `System.UriFormatException` at `AzureServiceBusMessageCreator.cs:329` | #4310 — the two delayed-send tests, the only ones that *did* receive |
| 2 | `ServiceBusException : SubCode=40900. Conflict` on the topic entity | a **third, untracked** defect: a concurrent topic-creation race in `EnsureChannelExistsAsync`, hit by the two `When_multiple_threads_try_to_post_a_message_at_the_same_time` tests |

The 26 include the basic post/receive companion, not just the canonical behaviours. The tests that
*passed* include `When_infrastructure_missing_and_assume/validate_channel_should_throw_exception` and
`When_posting_a_message_but_no_broker_created_should_throw_exception` — consistent with the diagnosis,
since those never expect a successful receive.

Two independent controls confirm the diagnosis:

**1. The hand-written ASB tests pass — because they pre-create the subscription explicitly.**
`tests/Paramore.Brighter.AzureServiceBus.Tests/MessagingGateway/When_posting_a_message_via_the_producer.cs:65`:

```csharp
_administrationClient.CreateSubscriptionAsync(_topicName, channelName, new AzureServiceBusSubscriptionConfiguration())
.GetAwaiter()
.GetResult();
```

only *then* does it build the channel and send. The generated gateway provider has no equivalent step.

This holds three for three: `When_consuming_a_message_via_the_consumer.cs:94` and
`When_posting_a_large_message_via_the_producer.cs:66-68` (queue, topic **and** subscription) do the
same. Every remaining hand-written ASB file is a pure mapping unit test that never touches a broker.
There is **no** hand-written counter-example that sends and receives without pre-creating — which is
why this defect survived until the generated suite exercised the untouched path.

**2. The one generated test that does receive is the only one whose send is deferred.**
`When_sending_a_delayed_message_should_deliver_after_delay` calls `ReceiveAsync` at t≈0 for its 2 s
"before-delay" arm — which creates the subscription — and the message is only scheduled for t=5 s. By
then the subscription exists, so it *is* delivered. It is the sole exception, and it is the exception that
proves the rule. (It then fails on a separate defect, #4310.)

The CI log clinches this: both delayed tests' `UriFormatException` stack frame is
`When_sending_a_delayed_message_should_deliver_after_delay.cs(76,0)` — **line 76 is the receive inside
the after-delay loop**, not the before-delay arm at line 67. The before-delay assertion passed
(correctly `MT_NONE`); the after-delay receive returned a genuine broker message.

## ASB is the outlier among the test gateway providers

Every other broker-backed provider does one of two things; ASB does neither.

- **AWS** does both. `SnsStandardMessageGatewayProvider.cs:191-217` fires a guarded 100 ms
`Receive`/`ReceiveAsync` after building the channel, *and* the AWS `ChannelFactory` already
provisions eagerly (`src/Paramore.Brighter.MessagingGateway.AWSSQS/ChannelFactory.cs:116` and
`:130`, both passed `_subscription.MakeChannels`).
- **RMQ** primes. Its `ChannelFactory`
(`src/Paramore.Brighter.MessagingGateway.RMQ.Async/ChannelFactory.cs:84-123`) is just as inert as
ASB's, so `RmqClassicMessageGatewayProvider.cs:91-132` compensates with the same guarded priming
receive, commented *"Ensuring that the queue exists before return the channel"*.
- **GCP** needs no priming: `GcpPubSubChannelFactory.CreateAsyncChannelAsync` calls
`await EnsureSubscriptionExistsAsync(...)` at
`src/Paramore.Brighter.MessagingGateway.GcpPubSub/GcpPubSubChannelFactory.cs:74`, *before* building
the channel.

So the field splits into "factory provisions eagerly" (AWS, GCP) and "provider primes with a short
receive" (RMQ, and AWS redundantly) — which is also the two available shapes for a fix here.

## Scope

The **queue** path is likely unaffected: `AzureServiceBusQueueMessageProducer.EnsureChannelExistsAsync`
(`:62-89`) creates the *same* queue entity the consumer later reads from, so a send-before-receive
still lands. It is the topic/subscription split that loses the message, and the test provider is
topic-mode only (`CreatePublication` never sets `UseServiceBusQueue`), so all 30 failures are
topic-mode.

## Suggested fix

Provision the subscription in the test gateway provider
(`tests/Paramore.Brighter.AzureServiceBus.Tests/MessagingGateway/AzureServiceBusMessageGatewayProvider.cs`,
`CreateChannel` / `CreateChannelAsync`) before returning the channel, as the hand-written tests do. That
is the smaller, in-scope change for #4240.

## Open design question

Is lazy subscription creation *itself* a gateway defect? A caller that builds a producer and a consumer
and then sends will silently lose the first message, with no error anywhere. Arguably
`CreateChannel`/`CreateChannelAsync` should honour `OnMissingChannel.Create` eagerly, which is what a
reader would expect from a factory that takes the subscription's `MakeChannels` setting — and which is
what AWS and GCP already do. Confirmed: `AzureServiceBusChannelFactory.GetAndCheckSubscription`
(`:101-114`) is the only place the subscription is inspected, and it checks nothing but the type and
`TimeOut >= 400ms`. `MakeChannels` is read **nowhere** in the factory; it is honoured later, and only
by the consumer (`AzureServiceBusTopicConsumer.cs:81`, `:92`). Worth a decision before the test-side
workaround calcifies.

## Blocks

The 11 Azure Service Bus cells in the conformance ledger are all `Deferred` and cannot be measured until
this and #4310 are both fixed. Fixing this issue will convert most of the 26 `MT_NONE` failures into
#4310 failures until #4310 is also fixed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.