ASB: rejected messages dead-lettered with blank reason/description

[Jonny-Freemarket]

Symptom

Handler-rejected Azure Service Bus messages are dead-lettered with blank DeadLetterReason and DeadLetterErrorDescription. The real reason exists only in logs, so DLQ triage means pivoting to log search and correlating by sequence number / message id.

Root cause

AzureServiceBusConsumer.RejectAsync computes the reason and description, logs them, then settles with the no-reason overload:

var reasonString = reason is null ? nameof(RejectionReason.DeliveryError) : reason.RejectionReason.ToString();
var description  = reason is null ? "unknown" : reason.Description ?? "unknown";
Log.DeadLetteringMessage(Logger, message.Id.Value, lockToken, reasonString, description); // logged…
await ServiceBusReceiver!.DeadLetterAsync(lockToken);                                      // …then dropped

git blame: these fields were introduced in #3918 (Universal DLQ) and wired only into the log line; the no-reason settle predates it and was never updated. Sync Reject delegates here via BrighterAsyncContext.Run, so it shares the behaviour. This is the only Brighter-initiated dead-letter call site — ASB-native auto-deadlettering (TTL expiry, max-delivery-count) already populates the columns itself.

Present on 10.4.1 and master. The SDK (Azure.Messaging.ServiceBus 7.20.1) already ships DeadLetterMessageAsync(message, deadLetterReason, deadLetterErrorDescription, ct) (4096-char cap, requires PeekLock — which Brighter uses); the gateway just never calls it. Consistent with ADR 0045-provide-dlq-where-missing ("defer to native") — this makes the deferral non-lossy rather than pulling ASB into the produced-message Universal DLQ path.

Proposed fix

Add a forwarding overload to IServiceBusReceiverWrapper, implement it via the existing CreateMessageShiv pattern (as Complete/DeadLetter/Abandon do) with 4096-char truncation, and change the single settle call to pass reasonString/description. Result: DeadLetterReason ← enum name, DeadLetterErrorDescription ← description text. Test-first, covering async + sync paths.

Questions

@iancooper:

1. Confirm the unforwarded reason in Add Universal Support for Dead Letter Channels #3918 is an oversight and forwarding to the native overload is acceptable?
2. The overload must go on the public IServiceBusReceiverWrapper (the consumer calls through it) — a breaking change for external implementers. A default interface method isn't viable (netstandard2.0 is in the target matrix). Accept the interface addition, or prefer a different shape?

[iancooper]

Confirm the unforwarded reason in #3918 is an oversight and forwarding to the native overload is acceptable?

Yeah, the Universal DLQ may not have picked up some provider-specific work. so this is an oversight

The overload must go on the public IServiceBusReceiverWrapper (the consumer calls through it) — a breaking change for external implementers.

We don't worry about implementors in a scenario like this. It's only likely to effect mocks etc. for very sophisticated users