search

Azure / azure-service-bus-dotnet

☁️ .NET Standard client library for Azure Service Bus
https://azure.microsoft.com/services/service-bus
Other
235 stars 120 forks source link

Add the possibility to modify properties when abandoning a message #671

Open 0xced opened 5 years ago

0xced commented 5 years ago

This is useful to store exception details when abandoning a message and still keeping the convenience of registering a message or session handler with AutoComplete = true.

Replaces #646

0xced commented 5 years ago

Are we giving promises that everytime we run into exception, we would be updating the properties?

Only when the exception happens in the user callback, i.e. the handler passed to void IReceiverClient.RegisterMessageHandler(Func<Message, CancellationToken, Task> handler, …)

There might be lot of exceptions that are raised for which we will not be able to update them. For example, CommunicationException, or say LockLostException etc.

I’m not sure about ServiceBusCommunicationException, but the message is not abandoned if the exception is a MessageLockLostException:

https://github.com/Azure/azure-service-bus-dotnet/blob/4e6ca73c0f661c944a5685d60deb59bcd2222edc/src/Microsoft.Azure.ServiceBus/MessageReceivePump.cs#L153-L157

By the way, I wonder how a MessageLockLostException could be thrown from the user callback. 🤔

There are lot of places where we call the exception handler. What is the expectation from the handler code in those cases?

I have documented this: the returned dictionary is ignored. Maybe this documentation needs improvement? https://github.com/Azure/azure-service-bus-dotnet/blob/4e6ca73c0f661c944a5685d60deb59bcd2222edc/src/Microsoft.Azure.ServiceBus/MessageHandlerOptions.cs#L43-L44

How will it distinguish between exception thrown from the message handler, vs exception thrown from doing auto complete?

I’m not sure I fully understand your concern. I think it doesn’t matter since the returned dictionary is ignored when RaiseExceptionReceived is called when auto completion fails: https://github.com/Azure/azure-service-bus-dotnet/blob/4e6ca73c0f661c944a5685d60deb59bcd2222edc/src/Microsoft.Azure.ServiceBus/MessageReceivePump.cs#L210-L224

If you meant from the user of the library point of view, the ExceptionReceivedEventArgs passed to the exception handler has a ExceptionReceivedContext.Action which describes the action, i.e. Complete, Abandon, UserCallback, Receive, RenewLock, AcceptMessageSession or CloseMessageSession.

0xced commented 5 years ago

@nemakam Any thoughts on this?