Skip to content

Commit

Permalink
Ensuring non http invocation responses are not processed by IHttpProx…
Browse files Browse the repository at this point in the history
…yService (#9984) (#10085)

* Fixing bug where non http invocation responses were going through IHttpProxyService.EnsureSuccessfulForwardingAsync

* Test cleanup

* Release notes and patch version bump.
  • Loading branch information
kshyju committed Apr 30, 2024
1 parent 01c899e commit a9ce4db
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 1 deletion.
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<!-- Please add your release notes in the following format:
- My change description (#PR)
-->
- 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)
Expand Down
2 changes: 1 addition & 1 deletion src/WebJobs.Script.Grpc/Channel/GrpcWorkerChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ internal async Task InvokeResponse(InvocationResponse invokeResponse)

try
{
if (IsHttpProxyingWorker)
if (IsHttpProxyingWorker && context.FunctionMetadata.IsHttpTriggerFunction())
{
await _httpProxyService.EnsureSuccessfulForwardingAsync(context);
}
Expand Down
73 changes: 73 additions & 0 deletions test/WebJobs.Script.Tests/Workers/Rpc/GrpcWorkerChannelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>() { { RpcWorkerConstants.HttpUri, "http://localhost:1234" } });

var httpInvocationId = Guid.NewGuid();
ScriptInvocationContext httpInvocationContext = GetTestScriptInvocationContext(httpInvocationId, new TaskCompletionSource<ScriptInvocationResult>(), logger: _logger);
httpInvocationContext.FunctionMetadata = BuildFunctionMetadataForHttpTrigger("httpTrigger");

var timerInvocationId = Guid.NewGuid();
ScriptInvocationContext timerInvocationContext = GetTestScriptInvocationContext(timerInvocationId, new TaskCompletionSource<ScriptInvocationResult>(), 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<ScriptInvocationContext>()), Times.Once);
}

[Fact]
public async Task Log_And_InvocationResult_OrderedCorrectly()
{
Expand Down Expand Up @@ -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;
}
}
}

0 comments on commit a9ce4db

Please sign in to comment.