awslabs / aws-dotnet-messaging

An AWS-native framework that simplifies the development of .NET message processing applications that use AWS services, such as SQS, SNS, and EventBridge.
https://awslabs.github.io/aws-dotnet-messaging/
Apache License 2.0
81 stars 15 forks source link

Dependency injection fails to create IMessageHandler when the constructor contains additional parameters #166

Open bslatner opened 2 weeks ago

bslatner commented 2 weeks ago

Describe the bug

Starting with a project created with:

dotnet new serverless.Messaging

I registered an IAmazonDynamoDB client with the IServiceCollection passed to ConfigureServices in Startup.cs. I changed the constructor definition in the provided GreetingMessageHandler to receive a registered service as a parameter and store it in an instance field.

I run the app and publish a message to the queue. The log indicates that Functions.Handler() gets called and that it, in turn, invokes _LambdaMessaging.ProcessLambdaEventWithBatchResponseAsync() without error. However, the HandleAsync method of GreetingMessageHandler never gets called.

Some investigation and additional logging shows that GreetingMessageHandler never even gets constructed. Function.Handler() winds up getting called over and over again because the message never leaves the queue.

Regression Issue

Expected Behavior

I would expect the framework to properly construct the IMessageHandler instance as long as the DI framework can resolve all the parameters passed to the constructor.

I would also expect that when ProcessLambdaEventWithBatchResponseAsync is invoked -- and possibly even before that -- if the message handler for the type of the message passed to it cannot be constructed, an exception should be thrown or, at the very least, an entry added to the log with information about why.

Current Behavior

Messages sent to the queue never get processed. The registered Lambda function that handles them gets invoked, but the IMessageHandler for that message type is never constructed or invoked.

Reproduction Steps

Create a new project using dotnet new serverless.Messaging

Open Startup.cs and on the line before services.AddAWSMessageBus add some instance to the DI container. In my case, I called:

services.AddAWSService<IAmazonDynamoDB>(); services.AddSingleton<IDataAccess>(x => { var ddb = x.GetRequiredService<IAmazonDynamoDB>(); return new DynamoDbDataAccess(ddb, Environment.GetEnvironmentVariable("TABLE_NAME")); });

Add a parameter for the service you added to the constructor GreetingMessageHandler . In my case, I added IDataAccess dataAccess.

Publish the stack and invoke the message sender function with a valid message.

Possible Solution

No response

Additional Information/Context

No response

AWS.Messaging (or related) package versions

Include="AWS.Messaging.Lambda" Version="0.10.0"

Targeted .NET Platform

.NET 8

Operating System and version

Windows 11

bslatner commented 1 week ago

I've figured it out. The behavior I've described happens when an exception occurs in the constructor of the service. This test case demonstrates the behavior. The issue, at this point, is that the log has no info about the exception.

The attached sample works normally. To duplicate the behavior I'm talking about, uncomment the throw exception in Startup.cs.

MessagingIssue.zip

ashishdhingra commented 1 week ago

I've figured it out. The behavior I've described happens when an exception occurs in the constructor of the service. This test case demonstrates the behavior. The issue, at this point, is that the log has no info about the exception.

The attached sample works normally. To duplicate the behavior I'm talking about, uncomment the throw exception in Startup.cs.

MessagingIssue.zip

@bslatner Good afternoon. Thanks for sharing the scenario and reproduction code. I'm unsure if this is an issue since exception occurs while resolving dependency in GreetingMessageHandler which is added as a Singleton in Startup class. Is your use case is that even for below test message handler request:

{
  "Records": [
    {
      "messageId": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78",
      "receiptHandle": "MessageReceiptHandle",
      "body": "{\"id\":\"d9b4bfc7-9398-44aa-8049-85c07490fb35\",\"source\":\"/AWSLambda/FunctionName\",\"specversion\":\"1.0\",\"type\":\"DotNETMessagingTest.GreetingMessage\",\"time\":\"2024-03-22T21:01:03.5484607+00:00\",\"data\":\"{\\u0022SenderName\\u0022:\\u0022value2\\u0022,\\u0022Greeting\\u0022:\\u0022value1\\u0022}\"}",
      "attributes": {
        "ApproximateReceiveCount": "1",
        "SentTimestamp": "1523232000000",
        "SenderId": "123456789012",
        "ApproximateFirstReceiveTimestamp": "1523232000001"
      },
      "messageAttributes": {},
      "md5OfBody": "7b270e59b47ff90a553787216d55d91d",
      "eventSource": "aws:sqs",
      "eventSourceARN": "arn:{partition}:sqs:{region}:123456789012:MyQueue",
      "awsRegion": "{region}"
    }
  ]
}

the function still executes successfully and reports the below output:

{
  "batchItemFailures": [
    {
      "itemIdentifier": "19dd0b57-b21e-4ac1-bd88-01bbb068cb78"
    }
  ]
}

And that it should actually throw error since the dependency is not able to be resolved due to exception while resolving it from IServiceCollection. Please confirm.

Thanks, Ashish

bslatner commented 1 week ago

I can confirm what you're seeing.

The problem as I see it, at this point, is that there is no way to diagnose this problem. At least not easily. The exception is completely lost. I'm not even sure how I would go about guarding against it, because at the point of construction there is no way to acquire a context to which you can log.

ashishdhingra commented 1 week ago

I can confirm what you're seeing.

The problem as I see it, at this point, is that there is no way to diagnose this problem. At least not easily. The exception is completely lost. I'm not even sure how I would go about guarding against it, because at the point of construction there is no way to acquire a context to which you can log.

@bslatner Thanks. I will review this with the team.