From 844e820b63d46ec8e84aec4d5fdbfa8681a69d88 Mon Sep 17 00:00:00 2001 From: Shyju Krishnankutty Date: Wed, 10 Apr 2024 10:21:15 -0700 Subject: [PATCH] Ensuring non http invocation responses are not processed by IHttpProxyService (#9984) * Fixing bug where non http invocation responses were going through IHttpProxyService.EnsureSuccessfulForwardingAsync * Test cleanup * Release notes and patch version bump. --- release_notes.md | 1 + .../Channel/GrpcWorkerChannel.cs | 2 +- .../Workers/Rpc/GrpcWorkerChannelTests.cs | 73 +++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/release_notes.md b/release_notes.md index 38f9257fa5..168f0924df 100644 --- a/release_notes.md +++ b/release_notes.md @@ -3,6 +3,7 @@ +- Fixed a bug where non HTTP invocation responses were processed by `IHttpProxyService` when HTTP proxying capability is enabled (#9984) - Update Python Worker Version to [4.28.1](https://github.com/Azure/azure-functions-python-worker/releases/tag/4.28.1) - Fixed an issue causing sporadic HTTP request failures when worker listeners were not fully initialized on first request #9954 - Update Node.js Worker Version to [3.10.0](https://github.com/Azure/azure-functions-nodejs-worker/releases/tag/v3.10.0) (#9999) diff --git a/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs b/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs index 00f94cbf97..1778269eda 100644 --- a/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs +++ b/src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs @@ -1094,7 +1094,7 @@ internal async Task InvokeResponse(InvocationResponse invokeResponse) try { - if (IsHttpProxyingWorker) + if (IsHttpProxyingWorker && context.FunctionMetadata.IsHttpTriggerFunction()) { await _httpProxyService.EnsureSuccessfulForwardingAsync(context); } diff --git a/test/WebJobs.Script.Tests/Workers/Rpc/GrpcWorkerChannelTests.cs b/test/WebJobs.Script.Tests/Workers/Rpc/GrpcWorkerChannelTests.cs index ec7ed11127..fa3d0760c9 100644 --- a/test/WebJobs.Script.Tests/Workers/Rpc/GrpcWorkerChannelTests.cs +++ b/test/WebJobs.Script.Tests/Workers/Rpc/GrpcWorkerChannelTests.cs @@ -1396,6 +1396,35 @@ public async Task GetFunctionMetadata_MultipleCalls_ReturnSameTask() Assert.Same(functionsTask1, functionsTask2); } + [Fact] + public async Task Ensure_SuccessfulForwardingAsync_Is_Invoked_OnlyFor_HttpInvocationResponses() + { + await CreateDefaultWorkerChannel(capabilities: new Dictionary() { { RpcWorkerConstants.HttpUri, "http://localhost:1234" } }); + + var httpInvocationId = Guid.NewGuid(); + ScriptInvocationContext httpInvocationContext = GetTestScriptInvocationContext(httpInvocationId, new TaskCompletionSource(), logger: _logger); + httpInvocationContext.FunctionMetadata = BuildFunctionMetadataForHttpTrigger("httpTrigger"); + + var timerInvocationId = Guid.NewGuid(); + ScriptInvocationContext timerInvocationContext = GetTestScriptInvocationContext(timerInvocationId, new TaskCompletionSource(), logger: _logger); + timerInvocationContext.FunctionMetadata = BuildFunctionMetadataForTimerTrigger("timerTrigger"); + + // Send http trigger and timer trigger invocation invocation requests. + await _workerChannel.SendInvocationRequest(httpInvocationContext); + await _workerChannel.SendInvocationRequest(timerInvocationContext); + + // Send http trigger and timer trigger invocation responses. + await _workerChannel.InvokeResponse(BuildSuccessfulInvocationResponse(timerInvocationId.ToString())); + await _workerChannel.InvokeResponse(BuildSuccessfulInvocationResponse(httpInvocationId.ToString())); + + var logs = _logger.GetLogMessages().ToArray(); + Assert.Single(logs.Where(m => m.FormattedMessage.Contains($"InvocationResponse received for invocation: '{timerInvocationId}'"))); + Assert.Single(logs.Where(m => m.FormattedMessage.Contains($"InvocationResponse received for invocation: '{httpInvocationId}'"))); + + // IHttpProxyService.EnsureSuccessfulForwardingAsync method should be invoked only for http invocation response. + _mockHttpProxyService.Verify(m => m.EnsureSuccessfulForwardingAsync(It.IsAny()), Times.Once); + } + [Fact] public async Task Log_And_InvocationResult_OrderedCorrectly() { @@ -1574,5 +1603,49 @@ private Task CreateSharedMemoryEnabledWorkerChannel(bool setEnvironmentVariable // Send worker init request and enable the capabilities return CreateDefaultWorkerChannel(capabilities: capabilities); } + + private static InvocationResponse BuildSuccessfulInvocationResponse(string invocationId) + { + return new InvocationResponse + { + InvocationId = invocationId, + Result = new StatusResult + { + Status = StatusResult.Types.Status.Success + }, + }; + } + + private static FunctionMetadata BuildFunctionMetadataForHttpTrigger(string name, string language = null) + { + var functionMetadata = new FunctionMetadata() { Name = name, Language = language }; + functionMetadata.Bindings.Add(new BindingMetadata() + { + Type = "httpTrigger", + Direction = BindingDirection.In, + Name = "req" + }); + functionMetadata.Bindings.Add(new BindingMetadata() + { + Type = "http", + Direction = BindingDirection.Out, + Name = "$return" + }); + + return functionMetadata; + } + + private static FunctionMetadata BuildFunctionMetadataForTimerTrigger(string name, string language = null) + { + var functionMetadata = new FunctionMetadata() { Name = name, Language = language }; + functionMetadata.Bindings.Add(new BindingMetadata() + { + Type = "timerTrigger", + Direction = BindingDirection.In, + Name = "myTimer" + }); + + return functionMetadata; + } } }