[Host] Abandon/dead letter (#356)

[EtherZa]

Added Abandon response to ConsumerErrorHandlerResult to be used by supported transports to dead letter the message. Most transports don't support the feature without some form of customised flow, but ASB and RabbitMQ both do.

Azure Service Bus
Implemented without restriction. Abandoned messages will be sent directly to the DLQ, failed messages will be retried until their maximum delivery count has been exceeded.

RabbitMQ
I don't have any experience with RabbitMQ, so I may have missed something; but the bus does not appear to limit how many times a message will be delivered if it is Nacked with requeue = true. As such, changing the code to fail with a requeue would be a dangerous breaking change as it could lead to an inifinite recursion in message processing for exisiting SMB users (unless I have missed something).

I have left the RabbitMq processing flow intact but with the expectation that a future change will provide tracking or some other safety net for the above scenario. Do you have a better option/insight into the RabbitMQ model?

Other transports
A NotSupportedException will be thrown if IConsumerErrorHandler returns with an Abandon state.

Application headers
With ASB currently being the major benefactor to this change, it becomes a bit of a moot point. ASB does not support header customisation when dead lettering a message (DeadLetterReason and DeadLetterErrorDescription are both set, but that is the extent of the customization permitted).

[EtherZa]

@zarusz the build action has not been triggered

[zarusz]

Hi @EtherZa,

This implementation looks great! I’ve left a few minor comments for your review—one of them might be more appropriate for a separate PR to address as a standalone feature.

Regarding the GitHub build trigger issue, I’m not entirely sure what’s happening. Could you try force-pushing again to see if that resolves it?

Thanks for your contribution!

[EtherZa]

This implementation looks great! I’ve left a few minor comments for your review—one of them might be more appropriate for a separate PR to address as a standalone feature.

Docs have been updated. Message properties are not as straight forward as it appears. I've added details to the comment above. Prob best to have it as a separate change.

Regarding the GitHub build trigger issue, I’m not entirely sure what’s happening. Could you try force-pushing again to see if that resolves it?

It's building again!

Thanks for your contribution!

It's a pleasure :)

---

I have added a RabbitMQ configuration option RequeueOnFailure() that will issue a Nack with requeue: true on Failure.

The default is still to Nack a message without a requeue but this can be changed by configuraing the switch per consumer. I have added a note to intro.md explicitly declaring that it is the developer's responsibility to Abandon all non-transient exceptions or face an inifinite loop with an example.

[EtherZa]

Okay, I don't quite understand the build trigger. I force pushed; it built and failed. I resolved the issue and force pushed again; but there does not appear to be a subsequent build?!

It looks like it was just delayed...

[sonarqubecloud]

Quality Gate failed

Failed conditions
66.0% Coverage on New Code (required ≥ 75%)

See analysis details on SonarQube Cloud

[zarusz]

@EtherZa I am happy to merge, however if possible perhaps you could increase coverage on some touched files to meet the coverage criteria?

[EtherZa]

@zarusz I'll look at what I can add, but most of the infringements are due to reformatting (remove trailing space, etc) or gates on the transports that don't support Abandon.

ie.

if (r.Result == ProcessResult.Abandon)
{
    throw new NotSupportedException("Transport does not support abandoning messages");
}

Do you want a test for each of these?

[zarusz]

Well, okay. Let's not bother.

Please give me a thumbs up if you plan no more changes and then I will merge.