Custom Handler Proxying #11035

satvu · 2025-04-25T22:46:37Z

Issue describing the changes in this PR

resolves #11025

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

Backporting to the in-proc branch is not required
- Otherwise: Link to backporting PR
My changes do not require documentation changes
- Otherwise: Documentation issue linked to PR - TODO
My changes should not be added to the release notes for the next release
- Otherwise: I've added my notes to release_notes.md
My changes do not need to be backported to a previous version
- Otherwise: Backport tracked by issue/PR #issue_or_pr
My changes do not require diagnostic events changes
- Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
I have added all required tests (Unit tests, E2E tests)

src/WebJobs.Script/Workers/Http/Configuration/HttpWorkerOptions.cs

src/WebJobs.Script/Workers/Http/Configuration/HttpWorkerOptionsSetup.cs

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

kshyju · 2025-04-29T15:55:09Z

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

+                _httpProxyService.StartForwarding(scriptInvocationContext, _destinationPrefix);
+
+                await _httpProxyService.EnsureSuccessfulForwardingAsync(scriptInvocationContext);
+                scriptInvocationContext.ResultSource.SetResult(_successfulInvocationResult);


Does the downstream code updates this _successfulInvocationResult instance? I see we use a shared instance for all invocations.

No, the downstream code does not. This value is set to allow the function invocation to continue, but its value is never used (similar to how http proxying flows). We ignore the function invocation result at this step so that the actual response just flows through.

src/WebJobs.Script/ScriptHostBuilderExtensions.cs

src/WebJobs.Script/Workers/Http/Configuration/HttpWorkerOptions.cs

src/WebJobs.Script/HttpProxyService/HttpForwardingException.cs

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

liliankasem · 2025-05-16T01:07:23Z

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

+            };
+
+            _destinationPrefix = new UriBuilder(WorkerConstants.HttpScheme, WorkerConstants.HostName, _httpWorkerOptions.Port).Uri;
+            _userAgentString = $"{HttpWorkerConstants.UserAgentHeaderValue}/{ScriptHost.Version}";


Does this ever get set outside of the ctor? Could this be a const? It doesnt look like its made up of anything that would require it to be init in here no?

Doesn't get set outside of the ctor, but ScriptHost.Version isn't a constant so this can't be init as a constant outside of the ctor at the top of the file.

src/WebJobs.Script/HttpProxyService/DefaultHttpProxyService.cs

src/WebJobs.Script/HttpProxyService/IHttpProxyService.cs

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

…zure/azure-functions-host into satvu/custom-handler-streaming

kshyju · 2025-06-10T22:24:51Z

src/WebJobs.Script/Workers/Http/Configuration/HttpWorkerOptions.cs

@@ -15,8 +15,16 @@ public class HttpWorkerOptions

        public int Port { get; set; }

+        /// <summary>
+        /// Gets or sets a value indicating whether the host will rebuild the initial invocation HTTP Request and send the copy to the worker process.
+        /// </summary>
        public bool EnableForwardingHttpRequest { get; set; }


A bit unclear about the property name and the summary comment. From the name, it sounds like the request is simply being forwarded as it is. But the comment suggests that a new copy of the request is being rebuilt from the original and that is sent to the worker process. Should we consider updating the property name to reflect that behavior?

This is an existing property (see schema store) and we cannot change this without having breaking changes. It's also already documented.

This feature currently works by handling http requests here and calling this extension method.

It's a really unfortunate name - I believe the plan is to phase this setting out in favor of the new one (for streaming). I added the comments to try to make clear the difference between the two very similar property names.

kshyju · 2025-06-10T22:29:51Z

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs

@@ -47,6 +54,9 @@ internal DefaultHttpWorkerService(HttpClient httpClient, IOptions<HttpWorkerOpti
                // Set 1 minute greater than FunctionTimeout to ensure invoction failure due to timeout is raised before httpClient raises operation cancelled exception
                _httpClient.Timeout = scriptHostOptions.Value.FunctionTimeout.Value.Add(TimeSpan.FromMinutes(1));
            }
+
+            _destinationPrefix = new UriBuilder(WorkerConstants.HttpScheme, WorkerConstants.HostName, _httpWorkerOptions.Port).Uri;


Is it a valid scenario where a customer configures their web app to only accept HTTPS traffic, but this code affects request forwarding because it explicitly uses HTTP? Should we allow the customer to specify or override the scheme?

I think your scenario could be valid - @fabiocav, is this an issue customers run into today? I borrowed from the existing pattern (for the EnableForwardingHttpRequest prop) in DefaultHttpWorkerService (see here) which only uses HTTP.

satvu marked this pull request as ready for review April 28, 2025 20:17

satvu requested a review from a team as a code owner April 28, 2025 20:17

satvu requested a review from fabiocav April 28, 2025 20:17

kshyju reviewed Apr 29, 2025

View reviewed changes

liliankasem reviewed Apr 29, 2025

View reviewed changes

satvu requested review from kshyju and liliankasem April 29, 2025 23:19

satvu added 10 commits May 13, 2025 17:44

wip

555c0fa

refactor, add headers logic

6eac80d

working e2e

165f2d2

cleanup, additional yarp comments

7b0a7c5

cleanup and tests

4133b2a

spacing fix

ef032c5

cleanup summary

436c6ed

resolve comments

944e1f5

update namespace

2256ee4

fix missing refs

a05018b

satvu force-pushed the satvu/custom-handler-streaming branch from 6043724 to a05018b Compare May 14, 2025 00:46

satvu added 2 commits May 14, 2025 10:40

cleanup

73b561c

cleanup add proxying headers

d0ad9a9

liliankasem reviewed May 16, 2025

View reviewed changes

satvu added 7 commits May 19, 2025 13:44

cleanup service registration

82f1226

using cleanup

8bb0e1b

revert grpc-registration changes

e4e5a9a

update import

52ffeff

rm unnecessary registration

9948c70

cleanup

ee2116a

update release notes

e45e5a3

satvu requested a review from liliankasem June 9, 2025 22:51

Merge branch 'dev' into satvu/custom-handler-streaming

cd5cdbf

fabiocav reviewed Jun 10, 2025

View reviewed changes

src/WebJobs.Script/HttpProxyService/DefaultHttpProxyService.cs Outdated Show resolved Hide resolved

src/WebJobs.Script/HttpProxyService/IHttpProxyService.cs Outdated Show resolved Hide resolved

src/WebJobs.Script/Workers/Http/DefaultHttpWorkerService.cs Outdated Show resolved Hide resolved

satvu added 2 commits June 10, 2025 14:00

update namespace, cleanup successful invocation result

5ffbb3c

Merge branch 'satvu/custom-handler-streaming' of https://github.com/A…

fe3cbd3

…zure/azure-functions-host into satvu/custom-handler-streaming

satvu requested a review from fabiocav June 10, 2025 21:30

kshyju reviewed Jun 10, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Custom Handler Proxying #11035

Custom Handler Proxying #11035

satvu commented Apr 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kshyju Apr 29, 2025

Uh oh!

satvu Apr 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liliankasem May 16, 2025

Uh oh!

satvu May 19, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kshyju Jun 10, 2025

Uh oh!

satvu Jun 11, 2025 •

edited

Loading

Uh oh!

kshyju Jun 10, 2025

Uh oh!

satvu Jun 11, 2025

Uh oh!

Uh oh!

Custom Handler Proxying #11035

Are you sure you want to change the base?

Custom Handler Proxying #11035

Conversation

satvu commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue describing the changes in this PR

Pull request checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kshyju Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

satvu Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liliankasem May 16, 2025

Choose a reason for hiding this comment

Uh oh!

satvu May 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kshyju Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

satvu Jun 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kshyju Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

satvu Jun 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

satvu commented Apr 25, 2025 •

edited

Loading

satvu Jun 11, 2025 •

edited

Loading