Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Jan 6, 2025

Issue #, if available: DOTNET-7824

Description of changes:

Problem

The issue is that when an exception occurs during dependency injection, we are swallowing that error and not showing the error to the user when they invoke the function. Specifically the issue can be reproduced with something like this

public void ConfigureServices(IServiceCollection services)
{
    services.AddAWSService<IAmazonDynamoDB>();
    services.AddSingleton<IThingDoer>(x =>
    {
        var ddb = x.GetRequiredService<IAmazonDynamoDB>();
        return new ThingDoer(ddb, Environment.GetEnvironmentVariable("TABLE_NAME"));
    });
    services.AddSingleton<IDependentThing>(x =>
    {
        var thingDoer = x.GetRequiredService<IThingDoer>();
        throw new InvalidOperationException("Blah blah"); <------------------ replicating an error ocurring during DependencyInjection
        return new DependentThing(thingDoer);
    });
    services.AddAWSMessageBus(builder =>
    {
        builder.AddSQSPublisher<GreetingMessage>(Environment.GetEnvironmentVariable("QUEUE_URL"));
        builder.AddLambdaMessageProcessor();
        builder.AddMessageHandler<GreetingMessageHandler, GreetingMessage>();
    });
}

the user would run the messaging tool and see this as the output

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

which does not provide and detail on what went wrong.

Issue

This line

handler = scope.ServiceProvider.GetService(subscriberMapping.HandlerType);

was the one throwing an InvalidOperationException when an issue occurred during DI initialization.

Fix

In order to fix this, i catch the exception when dependency injection fails, and then eventually re-throw it, which then displays on the UI as an error.

The reason for changing GetService to GetRequiredService is because GetRequiredService throws an exception when it cannot get the service (GetService i think is throwing the error too in this specific scenario because its an exception occuring in the constructor of something during DI), which can be caught in one place. Technically, i could have also try/catched the GetService too, but then i would have had some duplicate for when I catch the exception and when handler is null, so I just use GetRequiredService to make things easier.

Note: This change also affects the other scenarios which use the InvalidMessageHandlerSignatureException error. However, i believe these errors should also be treated as fatal, since that is what is listed here

image

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ghost ghost marked this pull request as ready for review January 6, 2025 21:51
@ghost ghost requested review from normj and philasmar January 6, 2025 21:51
{
handler = scope.ServiceProvider.GetRequiredService(subscriberMapping.HandlerType);
}
catch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should include this exception as the inner exception for the InvalidMessageHandlerSignatureException.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've updated it to include the inner exception and have re-tested and verified that the error shows the inner exception as well in the ui. I have updated the PR description with the new screenshot too

@ghost ghost requested a review from normj January 8, 2025 15:34
@ghost ghost merged commit 89b2d75 into dev Jan 9, 2025
7 checks passed
@aws-sdk-dotnet-automation aws-sdk-dotnet-automation deleted the devexception branch July 20, 2025 15:59
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants