Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WorkerExtensionStartup registers Middleware before ConfigureFunctionsWebApplication when using Http.AspNetCore #2506

Open
swettstein opened this issue Jun 3, 2024 · 5 comments
Labels
enhancement New feature or request needs-discussion

Comments

@swettstein
Copy link

Description

I have an extension library that automatically registers some middleware for my users. However, these middleware are dependent on the HttpContext existing in the FunctionContext.Items dictionary (put there by FunctionsHttpProxyingMiddleware). When using WorkerExtensionStartup to register these middleware, it appears they are added before ConfigureFunctionsWebApplication adds the proxying middleware and thus will not have access to the HttpContext.

Steps to reproduce

Extension Library Startup Class

[assembly: WorkerExtensionStartup(typeof(HttpStartup))]
namespace AzureFunctions.Extensions.Isolated.Http;

public class HttpStartup : WorkerExtensionStartup
{

    public override void Configure(IFunctionsWorkerApplicationBuilder applicationBuilder)
    {
        applicationBuilder.UseMiddleware(MyMiddleware);
    }

    private async Task MyMiddleware(FunctionContext context, Func<Task> next)
    {
        var httpContext = context.GetHttpContext();
        // do something with the HttpContext but it's null
        await next();
    }
}

Client Application Program.cs

var host = new HostBuilder()
    .ConfigureFunctionsWebApplication()
    .ConfigureServices(services =>
    {
        // register some services
    })
    .Build();

await host.RunAsync();

There is a workaround: create an extension method so that users have to register the middleware inside of ConfigureFunctionsWebApplication.
Extension Method:

public static IFunctionsWorkerApplicationBuilder AddCustomHttpMiddleware(this IFunctionsWorkerApplicationBuilder applicationBuilder)
{
    return applicationBuilder.UseMiddleware(MyMiddleware);
}

Program.cs:

var host = new HostBuilder()
    .ConfigureFunctionsWebApplication(b => b.AddCustomHttpMiddleware())
    .ConfigureServices(services =>
    {
        // register some services
    })
    .Build();

await host.RunAsync();
@swettstein swettstein added the potential-bug Items opened using the bug report template, not yet triaged and confirmed as a bug label Jun 3, 2024
@jviau jviau added needs-discussion enhancement New feature or request and removed potential-bug Items opened using the bug report template, not yet triaged and confirmed as a bug labels Jun 4, 2024
@jviau
Copy link
Contributor

jviau commented Jun 4, 2024

@swettstein AspNetCore registration is also an extension using the same hook as your example. We offer no ordering control for this way of registering middleware, so this would be a new feature to offer that ability via the attribute route.

For now, if you need to control the middleware timing, users will need to register it during ConfigureFunctionsWorkerDefaults callback - as that will happen after extension middleware is automatically registered.

@swettstein
Copy link
Author

controlling the order of extension middleware would be a nice feature but there is some complexity there and probably not necessary in this case.

the AspNetCore extension doesn't use WorkerExtensionStartup but rather depends on users correctly applying an extension method from FunctionsHostBuilderExtensions. none of the other built-in extensions operate this way so why is this one different?

why couldn't the AspNetCore extension be internally smarter about how it gets registered by ensuring that its middleware gets inserted first?

@fabiocav
Copy link
Member

Keeping this item in the backlog, as it is a valid request/gap, but this is low priority and there are no plans to have this worked on at the moment.

@jviau
Copy link
Contributor

jviau commented Jun 12, 2024

the AspNetCore extension doesn't use WorkerExtensionStartup but rather depends on users correctly applying an extension method from FunctionsHostBuilderExtensions. none of the other built-in extensions operate this way so why is this one different?

The reason for this comes down to needing access to the IHostBuilder, which WorkerExtensionStartup does not offer.

why couldn't the AspNetCore extension be internally smarter about how it gets registered by ensuring that its middleware gets inserted first?

For the same reason other extensions do not control order of middleware: there is no API for that. The extension only accesses public APIs from our worker assemblies, so it has no more privilege at setting a middleware first than any other extension does.

Addressing both of these points would require a non-trivial overhaul of some of our public APIs - which is not a priority at this point.

@swettstein
Copy link
Author

The reason for this comes down to needing access to the IHostBuilder, which WorkerExtensionStartup does not offer.

Some of it needs access to IHostBuilder but adding the FunctionsHttpProxyingMiddleware does not. It happens in an extension of IFunctionsWorkerApplicationBuilder which is accessible from WorkerExtensionStartup. regardless, with no guaranteed ordering of loading Extension Startups, the only way to guarantee my custom middleware is after the proxying middleware is to register it in ConfigureFunctionsWebApplication like i've already done for the workaround above.

i agree setting middleware priority would be non-trivial, especially when some of them are anonymous delegates. but what may be easier to do is priority of WorkerExtensionStartup through additional properties on the WorkerExtensionStartupAttribute. either way it's not high on your priority list.

ultimately if i'm creating an extension that's dependent on another extension, i would like to control the timing/ordering within my extension rather than depending on end users to get it right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs-discussion
Projects
None yet
Development

No branches or pull requests

3 participants