aws / aws/aws-dotnet-messaging
HandlerInvoker occasionally fails to invoke MessageHandlers (with same `SubscriberMapping.MessageType`)
- Dominant language
- C#
- Stars
- 143
- Forks
- 27
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 4
Description
### Describe the bug
`HandlerInvoker` occasionally fails to invoke `MessageHandler`s (with same `SubscriberMapping.MessageType`):
```cs
System.Reflection.TargetException: Object does not match target type.
at System.Reflection.MethodInvokerCommon.ValidateInvokeTarget(Object target, MethodBase method)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at AWS.Messaging.Services.HandlerInvoker.InvokeAsync(MessageEnvelope messageEnvelope, SubscriberMapping subscriberMapping, CancellationToken token)
```
I'm assuming it's code: https://github.com/aws/aws-dotnet-messaging/blob/f53b53dfab4d57396793702780060d58b0d8369f/src/AWS.Messaging/Services/HandlerInvoker.cs#L71-L86
Note I'm using a custom `EnvelopeSerializer` and hence `SubscriberMapping` (added context below).
### Regression Issue
- [ ] Select this option if this issue appears to be a regression.
### Expected Behavior
The MessageHandlers are invoked successfully for every message and every retry attempt.
### Current Behavior
The MessageHandlers are sometimes not invoked and produce the error shown above.
This could happen on the first attempt, a subsequent retry, or for all retry attempts of a message.
### Reproduction Steps
This seems to only happen under load.
Register a custom `IEnvelopeSerializer` which creates `SubscriberMapping` with the same `MessageType`, but different `HandlerType`.
For example:
```cs
SubscriberMapping mapping;
if (s3Metadata.Prefix.StartsWith("inbox/"))
{
mapping = SubscriberMapping.Create("inbox");
}
else if (s3Metadata.Prefix.StartsWith("failed/"))
{
mapping = SubscriberMapping.Create("failed");
}
```
This seems to create a race on the `HandlerInvoker._handlerMethods` cache, where it retrieves the method for _a different handler_: https://github.com/aws/aws-dotnet-messaging/blob/f53b53dfab4d57396793702780060d58b0d8369f/src/AWS.Messaging/Services/HandlerInvoker.cs#L71-L86
### Possible Solution
Instead of `subscriberMapping.MessageType`, use `subscriberMapping.MessageTypeIdentifier` as the cache key.
Alternatively it could cache both the handler and method?
### Additional Information/Context
I'm using this library to consume 2 SQS queues (in the same app). These queues consume S3 events (i.e. produced by AWS).
Given these messages are not in CloudEvents format, I created a custom `IEnvelopeSerializer` as per this issue: https://github.com/aws/aws-dotnet-messaging/issues/141
This custom `IEnvelopeSerializer` inspects the S3 prefix and routes the message to the appropriate handler.
For example:
```cs
public ValueTask ConvertToEnvelopeAsync(Message message)
{
var s3Metadata = ExtractS3Metadata(message);
SubscriberMapping mapping;
if (s3Metadata.Prefix.StartsWith("inbox/"))
{
mapping = SubscriberMapping.Create("inbox");
}
else if (s3Metadata.Prefix.StartsWith("failed/"))
{
mapping = SubscriberMapping.Create("failed");
}
else
{
throw new AmazonSQSException($"Unexpected prefix: {s3Metadata.Prefix}");
}
var messageEnvelope = new MessageEnvelope
{
Id = message.MessageId,
Message = s3Metadata,
SQSMetadata = ExtractSqsMetadata(message),
};
return ValueTask.FromResult(new ConvertToEnvelopeResult(messageEnvelope, mapping));
}
```
Please let me know if this is incorrect usage of the library 🙏
### AWS.Messaging (or related) package versions
AWS.Messaging 1.0.1
### Targeted .NET Platform
.NET 8
### Operating System and version
Linux (dotnet/aspnet:8.0 container)
Contributor guide
Research direction
Start with src/AWS.Messaging/Services/HandlerInvoker.cs at the referenced cache and invocation lines, then trace how custom EnvelopeSerializer mappings provide MessageType and HandlerType. Reproduce the failure under concurrent load using the inbox/failed mapping example. Done means each mapped MessageHandler is invoked successfully on every message and retry attempt without TargetException.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- aws, csharp
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100