aws / aws-lambda-dotnet

Libraries, samples and tools to help .NET Core developers develop AWS Lambda functions.
Apache License 2.0
1.58k stars 477 forks source link

LambdaBootstrap no longer throws if initialization fails when not using SnapStart #1881

Closed martincostello closed 2 days ago

martincostello commented 4 days ago

Describe the bug

In my MartinCostello.Testing.AwsLambdaTestServer library I have a test that verifies that an exception is thrown if a function fails to startup.

With the changes to enable SnapStart in #1876 this appears to no longer occur an instead RunAsync() just runs to completion with no obvious sign of failure.

Is this an intentional behaviour change? It seems like an oversight to me as with SnapStart enabled the code will terminate the process:

https://github.com/aws/aws-lambda-dotnet/blob/7ca5c8c2fdc879a872f52422369b2d26da4b2349/Libraries/src/Amazon.Lambda.RuntimeSupport/Bootstrap/LambdaBootstrap.cs#L216-L221

But without, it will return false and then just return:

https://github.com/aws/aws-lambda-dotnet/blob/7ca5c8c2fdc879a872f52422369b2d26da4b2349/Libraries/src/Amazon.Lambda.RuntimeSupport/Bootstrap/LambdaBootstrap.cs#L223

https://github.com/aws/aws-lambda-dotnet/blob/7ca5c8c2fdc879a872f52422369b2d26da4b2349/Libraries/src/Amazon.Lambda.RuntimeSupport/Bootstrap/LambdaBootstrap.cs#L156-L159

As an aside, Environment.Exit(1) isn't great from a testability perspective (IMHO) as if IsInitTypeSnapstart was true in my test, it would terminate the test process.

Regression Issue

Expected Behavior

An exception is thrown.

Current Behavior

LambdaBootstrap.RunAsync() returns with no error without invoking the lambda function.

Reproduction Steps

  1. Clone https://github.com/martincostello/lambda-test-server/pull/815/commits/b71e148808c13111aa86250265bb3660f557e7cf
  2. Run build.ps1 from the root of the cloned repository.

Possible Solution

Preserve the original behaviour by throwing in the catch block if IsInitTypeSnapstart is false.

Additional Information/Context

No response

AWS .NET SDK and/or Package version used

Amazon.Lambda.RuntimeSupport 1.12.0

Targeted .NET Platform

.NET 9

Operating System and version

Any

normj commented 3 days ago

This was intentional change for when this was running in SnapStart. Due to an upstream issue we needed to force an exit quickly otherwise we weren't getting the logs correctly sent to the Lambda service. We are hoping for an upstream change so we can get rid of the Environment.Exit call. I don't like calling Environment.Exit either.

The non SnapStart behavior change was a side effect of diagnosing the log issue. I'll revert the behavior so that in non SnapStart case if there was an error to let the exception bubble up.

normj commented 2 days ago

Version 1.12.1 of Amazon.Lambda.RuntimeSupport was released that reverts back the behavior for non SnapStart situation.

martincostello commented 2 days ago

Thanks Norm - that went straight through with no issues: https://github.com/martincostello/lambda-test-server/pull/823

normj commented 2 days ago

Great! Thanks for letting us know the change was successful.

github-actions[bot] commented 2 days ago

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.