search

Azure / azure-functions-dotnet-worker

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

Binding metadata consumption breaks deferred binding #1969

Open mattchenderson opened 1 year ago

mattchenderson commented 1 year ago

Trigger metadata (not payload data), if consumed in an output binding, for example, will cause consumption of the trigger payload.

This will not work:

        [Function(nameof(CreateThumbnail))]
        [BlobOutput("thumbnails/{name}-thumbnail.jpg", Connection = "AzureWebJobsStorage")]
        public async Task<byte[]> Run([BlobTrigger("images/{name}", Connection = "AzureWebJobsStorage")] Stream stream, string name)

stream will have already been consumed and have failed to be parsed by JsonPocoConverter. Here's the exception:

Exception: Microsoft.Azure.Functions.Worker.FunctionInputConverterException: Error converting 1 input parameters for Function 'CreateThumbnail': Cannot convert input parameter 'stream' to type 'System.IO.Stream' from type 'System.ReadOnlyMemory`1[[System.Byte, System.Private.CoreLib, Version=6.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]]'. Error:System.Text.Json.JsonException: '0x89' is an invalid start of a value. Path: $ | LineNumber: 0 | BytePositionInLine: 0.

If you remove the binding data reference, it will work as expected, though:

        [Function(nameof(CreateThumbnail))]
        [BlobOutput("thumbnails/no-binding-metadata-thumbnail.jpg", Connection = "AzureWebJobsStorage")]
        public async Task<byte[]> Run([BlobTrigger("images/{name}", Connection = "AzureWebJobsStorage")] Stream stream, string name)
liliankasem commented 1 year ago

I'm not 100% sure if this is a bug. This was an intentional implementation on the host side to "SkipDeferredBinding" if expressions are used in input or output bindings (code).

The bigger issue/bug is that expressions do not work with deferred binding period, and the workaround was to skip deferred binding for scenarios like this. I thought we had another issue tracking that, but if we do not, we can use this.

⚠️ Edit: Adding more context

During the SDK-binding feature work, we found that expressions were not working as expected, further investigation found that there is a bug in the WebJobs SDK that is behind this. The WebJobs SDK bug needs to be addressed for expressions to work with SDK-type bindings, however it is a breaking change fix and would therefore need to be part of the next major host (v5?). This has been identified as a parity blocker.

The bug is being tracked here: https://github.com/Azure/azure-webjobs-sdk/issues/2930

The decision to skip deferred binding for this specific scenario was an intentional, and temporary, decision to unblock the release of the SDK-binding feature.

Related context:

Specific use case where things break

In general, expressions are supported with SDK-types except for this one specific scenario:

If an input (or output) binding uses an expression, the trigger binding will not use ParameterBindingData (the input binding will continue to use ParameterBindingData).

In other words: the issue only occurs when a trigger binding is used with an input or output binding that uses an expression. We currently only support SDK-types for trigger bindings if none of the function's input or output bindings contain an expression.

There is a purple note about this in the public docs, but maybe it is not clear enough (cc: @mattchenderson)

Note: The broken scenarios are only for extensions that use deferred binding/SDK-types.

❌ This is broken :(

    [BlobTrigger("containerA/{name}")] Stream myBlob,
    [BlobInput("containerB/{name}")] Stream myOtherBlob, string name

⚠️ This is also broken (cannot use property of POCO as parameter for input binding) but only for extensions that use deferred binding for POCO types too i.e. Blob and Cosmos

    [BlobTrigger("containerA/{name}")] Book myBook,
    [BlobInput("containerB/{Book.Property}")] Stream myOtherBlob, string name

✅ This is OK

    [BlobTrigger("containerA/{name}")] string myBlob, // string, byte[] or POCO supported
    [BlobInput("containerB/{name}")] Stream myOtherBlob, string name

✅ This is OK

    [BlobTrigger("containerA/{name}")] Stream myBlob, string name,
    [BlobInput("containerB")] Stream[] blobs

✅ This is OK

    [HttpTrigger(AuthorizationLevel.Anonymous, "get", "post")] HttpRequest req,
    [BlobInput("containerA/{Query.name}")] Stream myBlob, string name

⭐️ As mentioned above, we will keep this issue open to track that this is fixed as part of the next major host.

domv3 commented 8 months ago

I was getting this error as well in a .NET 8 Isolated Blob Trigger. Cannot convert input parameter 'stream' to type 'System.IO.Stream' My issue was that even though I had added my user secrets for my blob connection string, it was not defined in my program.cs. You can also put the blob storage connection string in the local.settings.json. Here is what I added to my program.cs. .ConfigureAppConfiguration(config => { config.AddUserSecrets<YourFunctionHere>(optional: true, reloadOnChange: false); })

duggaraju commented 3 months ago

Even binding to BlobClient is broken. So this doesn't work too

[BlobTrigger("containerA")] BlobClient input
fabiocav commented 2 months ago

This requires some additional validation to understand whether the output binding logic is currently honoring the binding data defined by the trigger expression.

Flagging this for follow up as investigation is required.

surgupta-msft commented 1 week ago

Adding additional context here -

The distinction between payload and binding data (aka trigger metadata) is important here. To make sure that the terminology is clear, in the below example - data is payload and name is trigger metadata. 

        [Function(nameof(CreateThumbnail))]
        [BlobOutput("thumbnails/{name}-thumbnail.jpg", Connection = "AzureWebJobsStorage")]
        public async Task<byte[]> Run([BlobTrigger("images/{name}", Connection = "AzureWebJobsStorage")] Stream data, string name)

This code skips deferred binding if there is any expression in Input/Output binding. The restriction makes sense for payload - if we defer it to the worker, then we can't consume it in the host. However, there shouldn't be a need for binding data to prompt consumption of the resource data.

Hence, the first ask here is to investigate if we can allow expressions for the scenario where trigger metadata is used in output binding expressions.