aws / aws/aws-dotnet-messaging

HandlerInvoker occasionally fails to invoke MessageHandlers (with same `SubscriberMapping.MessageType`)

Open
#284 1 comment 0 reactions 0 assignees View on GitHub
bug p1 queued
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.