Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot consume messages if they are not published using this library #141

Open
zRosenthal opened this issue May 16, 2024 · 3 comments
Open
Labels
bug Something isn't working p2 This is a standard priority issue queued

Comments

@zRosenthal
Copy link

zRosenthal commented May 16, 2024

Describe the bug

Love the idea of this library as it drastically simplifies consuming from SQS. However in testing I am unable to get it to work unless the messages are published using this library.
This makes this library unusable in most enterprise environments where we consume events from a wide variety of sources many of which are not directly controlled by the team/app that is publishing those events.

Expected Behavior

Able to consume any json message

Current Behavior

Quick test routing events from AWS EventBridge

fail: AWS.Messaging.Serialization.EnvelopeSerializer[0]
MessageEnvelope instance is not valid
Id cannot be null or empty.
Source cannot be null.
Version cannot be null or empty.
MessageTypeIdentifier cannot be null or empty.
TimeStamp is not set.Message cannot be null.

Whats interesting about this is that the message from EventBridge actually does include Id, Source, Version however my understanding is that because that data is not contained in the detail node it is ignored.

Reproduction Steps

Publish a generic json object to an SQS queue without utilizing this library.

Possible Solution

Why are all of these fields required? Do we truly need them or can they be optional?

Can we allow custom mappers for the fields that are absolutely required? Could even ship with a prebuilt EventBridge mapper to utilize the existing properties in event.

Additional Information/Context

No response

AWS.Messaging (or related) package versions

AWS.Messaging 0.9.1

Targeted .NET Platform

.NET 8

Operating System and version

OSX Sonoma 14.1

@zRosenthal zRosenthal added bug Something isn't working needs-triage This issue or PR still needs to be triaged. labels May 16, 2024
@ashishdhingra ashishdhingra added needs-review and removed needs-triage This issue or PR still needs to be triaged. labels May 16, 2024
@normj
Copy link
Contributor

normj commented May 16, 2024

Thanks for the feedback @zRosenthal. This library is designed to by default expect messages to be in the CloudEvents specification. The publishers in this library publish in this specification. We also need someway to understand the type of message so we can map it to .NET type for serialization and mapping to handler. CloudEvents gave us placeholders for that information.

We did want to make the library extensible for non CloudEvents scenarios. You can inject your own implementation of the IEnvelopeSerializer and IMessageSerializer interfaces into the DI container. This allows you to define your own format of the messages. I would love to hear your feedback on adding your own serialization format to the library.

@ashishdhingra ashishdhingra added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-review labels May 24, 2024
@AndrewTziAnChan
Copy link

At the innermost layer, the data attribute inside the CloudEvent JSON object contains a JSON serialization of the .NET object that was sent or received as the message.

For this framework, the data payload in the CloudEvent should just be a string (an escaped string at that), right? Is it a general AWS recommendation to serialize the data field this way when routing messages through the event bus? The CloudEvent spec seems to be open about the content type and even in an earlier blog post, the payload is a JSON object.

If some other microservice (maybe a lamdba or non-.Net app) tried to publish a message to be consumed up by a .NET service that utilizes this messaging framework, that message payload should ideally be stringified for smooth deserialization by the message framework. Would that be a misinterpretation of opinion and intent from this framework? A stronger opinion seems beneficial if the CloudEvent spec is going to be adopted because it wouldn't be ideal for applications that do not have access to this messaging framework to run into integration issues just because the format of the data happens to be slightly different.

e.g.

If the .NET application received this message (note the format of the data field):

{
  "version": "0",
  "id": "dbc1c73a-c51d-0c0e-ca61-ab9278974c58",
  "account": "1234567890",
  "time": "2023-05-23T12:38:46Z",
  "region": "us-east-1",
  "detail-type": "OrderPlaced",
  "source": "myapp.orders-service",
  "resources": [],
  "detail": {
    "specversion": "1.0",
    "id": "bba4379f-b764-4d90-9fb2-9f572b2b0b61",
    "source": "myapp.orders-service",
    "type": "OrderPlaced",
    "data": {
      "order_id": "c172a984-3ae5-43dc-8c3f-be080141845a",
      "customer_id": "dda98122-b511-4aaf-9465-77ca4a115ee6",
      "order_total": "120.00"
    },
    "time": "2024-01-01T17:31:00Z",
    "dataschema": "https://us-west-2.console.aws.amazon.com/events/home?region=us-west-2#/registries/discovered-schemas/schemas/myapp.orders-service%40OrderPlaced",
    "correlationid": "dddd9340-135a-c8c6-95c2-41fb8f492222",
    "domain": "ORDERS"
  }

Is the expectation that a non-serialized object (JSON object) in the payload is unexpected and needs to be explicitly handled?

@ashishdhingra ashishdhingra added needs-review p2 This is a standard priority issue and removed response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jun 17, 2024
@Tridus
Copy link

Tridus commented Nov 29, 2024

I ran into identical problems starting out with this. I've got a queue that receives events from EventBridge when a PutObject is completed in an S3 bucket. This library has no idea how to understand those messages, apparently because it expects the CloudEvents spec while EventBridge isn't publishing the event in that spec.

While I can inject new serializers in to get around that, it feels like its undermining a lot of the headline selling point of the library in the first place (that it makes working with SQS easy so you can focus on business logic, which is a great idea). Having a built in method to say "deserliaze this message into this class definition I've provided and decorated with [JsonPropertyName]" would alleviate the issue entirely.

That's something I definitely can do, but it feels like something a library like this should have a means to handle out of the box given that it's going to be a common usage scenario.

Thanks. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2 This is a standard priority issue queued
Projects
None yet
Development

No branches or pull requests

5 participants