Make AbstractAspNetCoreFunction.Init() async

[damianh]

Make this method async:

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs

Line 189 in b54acef

protected virtual void Init(IWebHostBuilder builder) { }

Describe the Feature

When initializing a webhost it is desirable to be able to load configuration asynchronously. For example, from Secrets Manager or Parameter Store. As this method is Sync, one is forced to use the nasty GetAwaiter().GetResult() when calling async APIs from within this methods.

This has a bit of knock on effect, however the function handler:

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs

Lines 431 to 436 in b54acef

	public virtual async Task<TRESPONSE> FunctionHandlerAsync(TREQUEST request, ILambdaContext lambdaContext)
	{
	if (!IsStarted)
	{
	Start();
	}

... is already async so this change is entirely possible.

Is your Feature Request related to a problem?

More of a quality-of-life improvement.

Proposed Solution

• Add an InitAsync() methods so it's not breaking.
• Call InitAsync() before calling Init() - users should only override one but if they override both things should still work.
• Optional - add [Obsolete("..", false)] attribute to Init()

Describe alternatives you've considered

Currently using GetAwaiter().GetResult() which is not desirable, especially when already in an async context (via the function handler).

Additional Context

Real world usage - loading config from Secrets Manager and Paramater Store.

Marking is as potentially breaking, though making it non-breaking would be preferable.

Environment

• 👋 I may be able to implement this feature request
• ⚠️ This feature might incur a breaking change

---

This is a 🚀 Feature Request

[normj]

This is a valid feature request and the proposed solution does not cause a breaking change. I would be willing to take in a PR for this otherwise I will get to it when I can.

[damianh]

I've taken a look and it appears it won't be possible without a breaking change. The default mechanism to intitialize the aspnet core webhost is calling Start() from a constructor which will always be synchronous.

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs

Lines 103 to 111 in e2bb81a

	protected AbstractAspNetCoreFunction(StartupMode startupMode)
	{
	_startupMode = startupMode;
	if (_startupMode == StartupMode.Constructor)
	{
	Start();
	}
	}

Ultimately Init() is called via here:

aws-lambda-dotnet/Libraries/src/Amazon.Lambda.AspNetCoreServer/HostBuilderExtensions.cs

Line 57 in e2bb81a

configure(webBuilder);

... which is inside a sync action that comes from IHostBuilder.ConfigureWebHostDefaults() of which whose API you don't have control over.

The only thing I can think of is to add nother virtual that invoked only if the StartupMode is set to FirstRequest and invoke it from within:
https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.AspNetCoreServer/AbstractAspNetCoreFunction.cs#L433-L436

Something like:

if (!IsStarted)
{
    await PreInitAsync();
    Start();
}

To be honest, I don't understand why one startup mode would have advantage or disadvantage over the other. In terms of a lambda lifecycle, it's only ever going to be effectively initialized on first request.

[mungojam]

I've hit a similar thing with APIGatewayProxyFunction since I want to do an async operation during the PostMarshallResponseFeature() abstract call but can't do it properly because it isn't async

[damianh]

Hi Norm, sorry dropped the ball the related PR #708. Had a chance to look at over the holidays and on reflecion I think adding yet another virtual member PreInitAsycn doesn't make things simpler for the end user. They'd probably end up writing code like:

private IConfiguration _config;

public overide async Task PreInitAsync()
{
	_config = await LoadConfigAsync();
}

public overide void Init(IWebHostBuilder builder)
{
	builder.UseConfiguration(_config);
}

I think this would preferable:

public overide async InitAsync(IWebHostBuilder builder)
{
	var config = await LoadConfigAsync();
	builder.UseConfiguration(config);
}

However, if people are not doing async work, they'll now be forced to return Task.CompletedTask ugh.

I'm also not happy with the oxymoronic name.

[github-actions]

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

[mungojam]

Still relevant

[ashishdhingra]

Needs review. @normj Please check if this is still relevant.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.