[BUG FIX] HTTP Trigger Functions with Multiple Output Bindings (#2322)

extensions/Worker.Extensions.Http.AspNetCore/release_notes.md

@@ -9,4 +9,4 @@
 - Improvements to context coordination/synchronization handling and observability
   - Failure to receive any of the expected context synchronization calls will now result in a `TimeoutException` thrown with the appropriate exception information. Previously this would block indefinitely and failures here were difficult to diagnose.
   - Debug logs are now emitted in the context coordination calls, improving observability.
-
+- Introduces fix to properly handle multiple output binding scenarios (#2322).

extensions/Worker.Extensions.Http.AspNetCore/src/FunctionsMiddleware/FunctionsHttpProxyingMiddleware.cs

@@ -1,6 +1,7 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
+using System;
 using System.Collections.Concurrent;
 using System.Linq;
 using System.Threading.Tasks;
@@ -9,14 +10,17 @@
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Azure.Functions.Worker.Extensions.Http.Converters;
+using Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore.Infrastructure;
 using Microsoft.Azure.Functions.Worker.Http;
 using Microsoft.Azure.Functions.Worker.Middleware;
+using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.Azure.Functions.Worker.Extensions.Http.AspNetCore
 {
     internal class FunctionsHttpProxyingMiddleware : IFunctionsWorkerMiddleware
     {
         private const string HttpTrigger = "httpTrigger";
+        private const string HttpBindingType = "http";
 
         private readonly IHttpCoordinator _coordinator;
         private readonly ConcurrentDictionary<string, bool> _isHttpTrigger = new();
@@ -47,40 +51,67 @@ public async Task Invoke(FunctionContext context, FunctionExecutionDelegate next
 
             await next(context);
 
-            var invocationResult = context.GetInvocationResult();
-
-            if (invocationResult?.Value is IActionResult actionResult)
+            var responseHandled = await TryHandleHttpResult(context.GetInvocationResult().Value, context, httpContext, true)
+                || await TryHandleOutputBindingsHttpResult(context, httpContext);
+
+            if (!responseHandled)
             {
-                ActionContext actionContext = new ActionContext(httpContext, httpContext.GetRouteData(), new ActionDescriptor());
-
-                await actionResult.ExecuteResultAsync(actionContext);
+                var logger = context.InstanceServices.GetRequiredService<ExtensionTrace>();
+                logger.NoHttpResponseReturned(context.FunctionDefinition.Name, context.InvocationId);
             }
-            else if (invocationResult?.Value is AspNetCoreHttpResponseData)
+
+            // Allow ASP.NET Core middleware to continue
+            _coordinator.CompleteFunctionInvocation(invocationId);
+        }
+
+        private static async Task<bool> TryHandleHttpResult(object? result, FunctionContext context, HttpContext httpContext, bool isInvocationResult = false)
+        {
+            switch (result)
             {
-                // The AspNetCoreHttpResponseData implementation is
-                // simply a wrapper over the underlying HttpResponse and
-                // all APIs manipulate the request.
-                // There's no need to return this result as no additional
-                // processing is required.
-                invocationResult.Value = null;
+                case IActionResult actionResult:
+                    ActionContext actionContext = new ActionContext(httpContext, httpContext.GetRouteData(), new ActionDescriptor());
+                    await actionResult.ExecuteResultAsync(actionContext);
+                    break;
+                case AspNetCoreHttpRequestData when isInvocationResult:
+                    // The AspNetCoreHttpResponseData implementation is
+                    // simply a wrapper over the underlying HttpResponse and
+                    // all APIs manipulate the request.
+                    // There's no need to return this result as no additional
+                    // processing is required.
+                    context.GetInvocationResult().Value = null;
+                    break;
+                case IResult iResult:
+                    await iResult.ExecuteAsync(httpContext);
+                    break;
+                default:
+                    return false;
             }
 
-            // allows asp.net middleware to continue
-            _coordinator.CompleteFunctionInvocation(invocationId);
+            return true;
+        }
+
+        private static Task<bool> TryHandleOutputBindingsHttpResult(FunctionContext context, HttpContext httpContext)
+        {
+            var httpOutputBinding = context.GetOutputBindings<object>()
+                .FirstOrDefault(a => string.Equals(a.BindingType, HttpBindingType, StringComparison.OrdinalIgnoreCase));
+
+            return httpOutputBinding is null
+                ? Task.FromResult(false)
+                : TryHandleHttpResult(httpOutputBinding.Value, context, httpContext);
         }
 
         private static void AddHttpContextToFunctionContext(FunctionContext funcContext, HttpContext httpContext)
         {
             funcContext.Items.Add(Constants.HttpContextKey, httpContext);
 
-            // add asp net version of httprequestdata feature
+            // Add ASP.NET Core integration version of IHttpRequestDataFeature
             funcContext.Features.Set<IHttpRequestDataFeature>(AspNetCoreHttpRequestDataFeature.Instance);
         }
 
         private static bool IsHttpTriggerFunction(FunctionContext funcContext)
         {
             return funcContext.FunctionDefinition.InputBindings
-                .Any(p => p.Value.Type.Equals(HttpTrigger, System.StringComparison.OrdinalIgnoreCase));
+                .Any(p => p.Value.Type.Equals(HttpTrigger, StringComparison.OrdinalIgnoreCase));
         }
     }
 }

extensions/Worker.Extensions.Http.AspNetCore/src/Infrastructure/ExtensionTrace.General.cs

@@ -18,13 +18,21 @@ public void FunctionContextSet(string invocationId)
             GeneralLog.FunctionContextSet(_defaultLogger, invocationId);
         }
 
+        public void NoHttpResponseReturned(string functionName, string invocationId)
+        {
+            GeneralLog.NoHttpResponseReturned(_defaultLogger, functionName, invocationId);
+        }
+
         private static partial class GeneralLog
         {
             [LoggerMessage(1, LogLevel.Debug, @"HttpContext set for invocation ""{InvocationId}"", Request id ""{RequestId}"".", EventName = nameof(HttpContextSet))]
             public static partial void HttpContextSet(ILogger logger, string invocationId, string requestId);
 
             [LoggerMessage(2, LogLevel.Debug, @"FunctionContext set for invocation ""{InvocationId}"".", EventName = nameof(FunctionContextSet))]
             public static partial void FunctionContextSet(ILogger logger, string invocationId);
+
+            [LoggerMessage(3, LogLevel.Trace, @"No HTTP response returned from function '{FunctionName}', invocation {InvocationId}.", EventName = nameof(NoHttpResponseReturned))]
+            public static partial void NoHttpResponseReturned(ILogger logger, string functionName, string invocationId);
         }
     }
 }

extensions/Worker.Extensions.Http/release_notes.md

@@ -4,7 +4,8 @@
 - My change description (#PR/#issue)
 -->
 
-### Microsoft.Azure.Functions.Worker.Extensions.Http <version>
+### Microsoft.Azure.Functions.Worker.Extensions.Http 3.2.0
 
 - Added ability to bind a POCO parameter to the request body using `FromBodyAttribute`
   - Special thanks to @njqdev for the contributions and collaboration on this feature
+- Introduces `HttpResultAttribute`, which should be used to label the parameter associated with the HTTP result in multiple output binding scenarios (#2322).

extensions/Worker.Extensions.Http/src/HttpResultAttribute.cs

@@ -0,0 +1,15 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+using System;
+
+namespace Microsoft.Azure.Functions.Worker
+{
+    /// <summary>
+    /// Attribute used to mark an HTTP Response on an HTTP Trigger function with multiple output bindings.
+    /// </summary>
+    [AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
+    public sealed class HttpResultAttribute : Attribute
+    {
+    }
+}

extensions/Worker.Extensions.Http/src/Worker.Extensions.Http.csproj

@@ -6,7 +6,7 @@
     <Description>HTTP extensions for .NET isolated functions</Description>
 
     <!--Version information-->
-    <VersionPrefix>3.1.0</VersionPrefix>
+    <VersionPrefix>3.2.0</VersionPrefix>
   </PropertyGroup>
 
   <Import Project="..\..\..\build\Extensions.props" />

sdk/Sdk.Generators/Constants.cs

@@ -41,7 +41,8 @@ internal static class Types
             internal const string BindingPropertyNameAttribute = "Microsoft.Azure.Functions.Worker.Extensions.Abstractions.BindingPropertyNameAttribute";
             internal const string DefaultValue = "Microsoft.Azure.Functions.Worker.Extensions.Abstractions.DefaultValueAttribute";
 
-            internal const string HttpResponse = "Microsoft.Azure.Functions.Worker.Http.HttpResponseData";
+            internal const string HttpResponseData = "Microsoft.Azure.Functions.Worker.Http.HttpResponseData";
+            internal const string HttpResultAttribute = "Microsoft.Azure.Functions.Worker.HttpResultAttribute";
             internal const string HttpTriggerBinding = "Microsoft.Azure.Functions.Worker.HttpTriggerAttribute";
 
             internal const string BindingCapabilitiesAttribute = "Microsoft.Azure.Functions.Worker.Extensions.Abstractions.BindingCapabilitiesAttribute";

sdk/Sdk.Generators/DiagnosticDescriptors.cs

@@ -45,7 +45,7 @@ private static DiagnosticDescriptor Create(string id, string title, string messa
         public static DiagnosticDescriptor MultipleHttpResponseTypes { get; }
                   = Create(id: "AZFW0007",
                     title: "Symbol could not be found in user compilation.",
-                    messageFormat: "Found multiple public properties of type HttpResponseData defined in return type '{0}'. Only one HTTP response binding type is supported in your return type definition.",
+                    messageFormat: "Found multiple HTTP Response types (properties with HttpResultAttribute or properties of type HttpResponseData) defined in return type '{0}'. Only one HTTP response binding type is supported in your return type definition.",
                     category: "FunctionMetadataGeneration",
                     severity: DiagnosticSeverity.Error);
 

sdk/Sdk.Generators/FunctionMetadataProviderGenerator/FunctionMetadataProviderGenerator.Parser.cs

@@ -164,15 +164,15 @@ private bool TryGetMethodOutputBinding(IMethodSymbol method, out bool hasMethodO
 
                 if (outputBindingAttribute != null)
                 {
-                    if (!TryCreateBindingDict(outputBindingAttribute, Constants.FunctionMetadataBindingProps.ReturnBindingName, Location.None, out IDictionary<string, object>? bindingDict))
+                    if (!TryCreateBindingDictionary(outputBindingAttribute, Constants.FunctionMetadataBindingProps.ReturnBindingName, Location.None, out IDictionary<string, object>? bindings))
                     {
                         bindingsList = null;
                         return false;
                     }
 
                     bindingsList = new List<IDictionary<string, object>>(capacity: 1)
                     {
-                        bindingDict!
+                        bindings!
                     };
 
                     return true;
@@ -288,22 +288,22 @@ private bool TryGetParameterInputAndTriggerBindings(IMethodSymbol method, out bo
 
                             string bindingName = parameter.Name;
 
-                            if (!TryCreateBindingDict(attribute, bindingName, Location.None, out IDictionary<string, object>? bindingDict, supportsDeferredBinding))
+                            if (!TryCreateBindingDictionary(attribute, bindingName, Location.None, out IDictionary<string, object>? bindings, supportsDeferredBinding))
                             {
-                                bindingsList = null;
+                                bindings = null;
                                 return false;
                             }
 
                             // If cardinality is supported and validated, but was not found in named args, constructor args, or default value attributes
                             // default to Cardinality: One to stay in sync with legacy generator.
-                            if (cardinalityValidated && !bindingDict!.Keys.Contains("cardinality"))
+                            if (cardinalityValidated && !bindings!.Keys.Contains("cardinality"))
                             {
-                                bindingDict!.Add("cardinality", "One");
+                                bindings!.Add("cardinality", "One");
                             }
 
                             if (dataType is not DataType.Undefined)
                             {
-                                bindingDict!.Add("dataType", Enum.GetName(typeof(DataType), dataType));
+                                bindings!.Add("dataType", Enum.GetName(typeof(DataType), dataType));
                             }
 
                             // check for binding capabilities
@@ -318,7 +318,7 @@ private bool TryGetParameterInputAndTriggerBindings(IMethodSymbol method, out bo
                                 }
                             }
 
-                            bindingsList.Add(bindingDict!);
+                            bindingsList.Add(bindings!);
                         }
                     }
                 }
@@ -434,7 +434,7 @@ private bool TryGetReturnTypeBindings(IMethodSymbol method, bool hasHttpTrigger,
                         }
                     }
 
-                    if (SymbolEqualityComparer.Default.Equals(returnTypeSymbol, _knownFunctionMetadataTypes.HttpResponse)) // If return type is HttpResponseData
+                    if (SymbolEqualityComparer.Default.Equals(returnTypeSymbol, _knownFunctionMetadataTypes.HttpResponseData))
                     {
                         bindingsList.Add(GetHttpReturnBinding(Constants.FunctionMetadataBindingProps.ReturnBindingName));
                     }
@@ -467,8 +467,9 @@ private bool TryGetReturnTypePropertyBindings(ITypeSymbol returnTypeSymbol, bool
                         return false;
                     }
 
-                    // Check if this attribute is an HttpResponseData type attribute
-                    if (prop is IPropertySymbol property && SymbolEqualityComparer.Default.Equals(property.Type, _knownFunctionMetadataTypes.HttpResponse))
+                    // Check for HttpResponseData type for legacy apps
+                    if (prop is IPropertySymbol property
+                             && (SymbolEqualityComparer.Default.Equals(property.Type, _knownFunctionMetadataTypes.HttpResponseData)))
                     {
                         if (foundHttpOutput)
                         {
@@ -479,34 +480,46 @@ private bool TryGetReturnTypePropertyBindings(ITypeSymbol returnTypeSymbol, bool
 
                         foundHttpOutput = true;
                         bindingsList.Add(GetHttpReturnBinding(prop.Name));
+                        continue;
                     }
-                    else if (prop.GetAttributes().Length > 0) // check if this property has any attributes
+
+                    var propAttributes = prop.GetAttributes();
+
+                    if (propAttributes.Length > 0)
                     {
-                        var foundPropertyOutputAttr = false;
+                        var bindingAttributes = propAttributes.Where(p => p.AttributeClass!.IsOrDerivedFrom(_knownFunctionMetadataTypes.BindingAttribute));
+
+                        if (bindingAttributes.Count() > 1)
+                        {
+                            _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.MultipleBindingsGroupedTogether, Location.None, new object[] { "Property", prop.Name.ToString() }));
+                            bindingsList = null;
+                            return false;
+                        }
 
-                        foreach (var attr in prop.GetAttributes()) // now loop through and check if any of the attributes are Binding attributes
+                        // Check if this property has an HttpResultAttribute on it
+                        if (HasHttpResultAttribute(prop))
                         {
-                            if (SymbolEqualityComparer.Default.Equals(attr.AttributeClass?.BaseType, _knownFunctionMetadataTypes.OutputBindingAttribute))
+                            if (foundHttpOutput)
                             {
-                                // validate that there's only one binding attribute per property
-                                if (foundPropertyOutputAttr)
-                                {
-                                    _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.MultipleBindingsGroupedTogether, Location.None, new object[] { "Property", prop.Name.ToString() }));
-                                    bindingsList = null;
-                                    return false;
-                                }
+                                _context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.MultipleHttpResponseTypes, Location.None, new object[] { returnTypeSymbol.Name }));
+                                bindingsList = null;
+                                return false;
+                            }
 
-                                if (!TryCreateBindingDict(attr, prop.Name, prop.Locations.FirstOrDefault(), out IDictionary<string, object>? bindingDict))
-                                {
-                                    bindingsList = null;
-                                    return false;
-                                }
+                            foundHttpOutput = true;
+                            bindingsList.Add(GetHttpReturnBinding(prop.Name));
+                        }
+                        else
+                        {
+                            if (!TryCreateBindingDictionary(bindingAttributes.FirstOrDefault(), prop.Name, prop.Locations.FirstOrDefault(), out IDictionary<string, object>? bindings))
+                            {
+                                bindingsList = null;
+                                return false;
+                            }
 
-                                bindingsList.Add(bindingDict!);
+                            bindingsList.Add(bindings!);
 
-                                returnTypeHasOutputBindings = true;
-                                foundPropertyOutputAttr = true;
-                            }
+                            returnTypeHasOutputBindings = true;
                         }
                     }
                 }
@@ -519,6 +532,21 @@ private bool TryGetReturnTypePropertyBindings(ITypeSymbol returnTypeSymbol, bool
                 return true;
             }
 
+            private bool HasHttpResultAttribute(ISymbol prop)
+            {
+                var attributes = prop.GetAttributes();
+                foreach (var attribute in attributes)
+                {
+                    if (attribute.AttributeClass is not null && 
+                        attribute.AttributeClass.IsOrDerivedFrom(_knownFunctionMetadataTypes.HttpResultAttribute))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
+
             private IDictionary<string, object> GetHttpReturnBinding(string returnBindingName)
             {
                 var httpBinding = new Dictionary<string, object>
@@ -531,7 +559,7 @@ private IDictionary<string, object> GetHttpReturnBinding(string returnBindingNam
                 return httpBinding;
             }
 
-            private bool TryCreateBindingDict(AttributeData bindingAttrData, string bindingName, Location? bindingLocation, out IDictionary<string, object>? bindings, bool supportsDeferredBinding = false)
+            private bool TryCreateBindingDictionary(AttributeData bindingAttrData, string bindingName, Location? bindingLocation, out IDictionary<string, object>? bindings, bool supportsDeferredBinding = false)
             {
                 // Get binding info as a dictionary with keys as the property name and value as the property value
                 if (!TryGetAttributeProperties(bindingAttrData, bindingLocation, out IDictionary<string, object?>? attributeProperties))