Azure / azure-functions-dotnet-worker

Azure Functions out-of-process .NET language worker
MIT License
412 stars 176 forks source link

Service Bus message settlement from middleware when using Poco types in the functions #2157

Open nordvall opened 8 months ago

nordvall commented 8 months ago

Scenario

We have 100's of C# functions that consumes messages from Azure Service Bus. When there is an exception in the function logic we want to add some properties to the message.

In the in-process world we do it like this:

public class OurMessageProcessor : MessageProcessor
{
    protected override Task CompleteProcessingMessageAsync(ServiceBusMessageActions actions, ServiceBusReceivedMessage message, FunctionResult result, CancellationToken cancellationToken)
    { 
    // removed code...

        if (!result.Succeeded)
        {
            Exception exception = result.Exception.InnerException ?? result.Exception;

            var properties = new Dictionary<string, object>
            {
                { "ExceptionMessage", exception.Message }
                // some more properties
            };

            actions.AbandonMessageAsync(message, properties);
        }

        // removed code
    }
}

The logic above is packaged as an Azure Functions Extension library that all in-process function apps can consume.

Problem

We are looking into migrating all of our Function apps to the new isolated worker. According to https://github.com/Azure/azure-functions-dotnet-worker/issues/226#issuecomment-1771932718 we can now get a reference to ServiceBusMessageActions in the isolated world. But since we have lots of functions we want to do it via some middleware. According to https://github.com/Azure/azure-functions-dotnet-worker/issues/1824#issuecomment-1785960034 there are some hacks that let you access both ServiceBusMessageActions and ServiceBusReceivedMessage from middleware.

However, this part:

BindingMetadata meta = context.FunctionDefinition.InputBindings.FirstOrDefault(b => b.Value.Type == "serviceBusTrigger").Value;
var input = await context.BindInputAsync<ServiceBusReceivedMessage>(meta);

...only works if your function actually consumes the message as ServiceBusReceivedMessage. In our case we consume the messages as POCO types in the function signatures. When I try to run the code above from middleware I get this exception:

Unable to cast object of type 'a.b.c.OurPocoType' to type 'Azure.Messaging.ServiceBus.ServiceBusReceivedMessage'

Proposed solution

Alternative 1 The obvious solution would be to make it possible to retrieve ServiceBusReceivedMessage from middleware even if the called function consumed the message as a Poco type. Then we could use ServiceBusReceivedMessage in combination with ServiceBusMessageActions and call actions.AbandonMessageAsync(message, properties) just like before.

Alternative 2 According to the source code for ServiceBusMessageActions.AbandonMessageAsync, it basically does this:

public virtual async Task AbandonMessageAsync(ServiceBusReceivedMessage message, IDictionary<string, object>? propertiesToModify = default, CancellationToken cancellationToken = default)
{
    var request = new AbandonRequest()
    {
        Locktoken = message.LockToken,
    };

    if (propertiesToModify != null)
    {
        request.PropertiesToModify = ConvertToByteString(propertiesToModify);
    }

    await _settlement.AbandonAsync(request, cancellationToken: cancellationToken);
}

All that is needed from the ServiceBusReceivedMessage seem to be the LockToken. And the LockToken is already available in middleware via context.BindingContext.BindingData["LockToken"]

So maybe we are "crossing the river to get water" (as we say in Sweden) when we are struggling to inflate a ServiceBusReceivedMessage just to get something that we already have. Maybe an easier approach would be an overload for ServiceBusMessageActions.AbandonMessageAsync like this:

async Task AbandonMessageAsync(string lockToken, IDictionary<string, object>? propertiesToModify = default,
            CancellationToken cancellationToken = default)

Feel free to choose the approach, but we hesitate to migrate to the isolated worker until this has a (hopefully not-so-hacky) solution.

jehrenzweig-pi commented 8 months ago

It would be helpful to hear what other folks think about this issue & the solutions proposed in the OP. It would address a potential & (AFAIK) undocumented "gotcha", when a developer chooses to use ServiceBusReceivedMessage or a POCO class.

SeanFeldman commented 8 months ago

This one ties into #1824.

The whole story about working with ASB messages from the middleware is not good today.

cc @fabiocav @Matthew

nordvall commented 3 months ago

Could you add the tag extension:servicebus to this issue, so it shows up together with other ServiceBus-related issues? To ensure it is not forgotten...

RobARichardson commented 3 months ago

It would be helpful to hear what other folks think about this issue & the solutions proposed in the OP. It would address a potential & (AFAIK) undocumented "gotcha", when a developer chooses to use ServiceBusReceivedMessage or a POCO class.

I would also like to be able to access ServiceBusReceivedMessage and ServiceBusMessageActions from middleware even when the Function is using a POCO as input.

Nehmiabm commented 2 months ago

I had also opened a similar request here https://github.com/Azure/azure-functions-dotnet-worker/issues/1824#issuecomment-2158859670. In my case the eventual goal is to be able to schedule the message in a function middleware but since 'AbandonMessageAsync' doesn't accept a timeout (Tracked in separate issue here https://github.com/Azure/azure-service-bus/issues/454), I won't be able to use @nordvall 's workaround.