awslabs / amazon-sqs-java-messaging-lib

This Amazon SQS Java Messaging Library holds the Java Message Service compatible classes, that are used for communicating with Amazon Simple Queue Service.
http://aws.amazon.com/sqs
Apache License 2.0
167 stars 146 forks source link

Custom message attribute types cause JMS Exception... #53

Open jwindberg opened 6 years ago

jwindberg commented 6 years ago

We had a bunch of messages in an SQS queue where the messageAttribute data types were not recognized by the code with SQSMessage. private static Object getObjectValue(String value, String type) throws JMSException Now, I will agree that a bad value type is partially to blame. I would expect something better from the library code, all that was happening is that the message would get rejected, and returned to the queue and then re-read, infinite loop.

Perhaps default to String? or skip the bad value type? Something other than throwing a JMSException.

jwindberg commented 6 years ago

I would alternatively propose that the getObjectValue method be modified along the lines of: valueType.startsWith("String") Thus it would still work if someone entered a value in the custom data type, resulting in a data type of "String.customType"

kuba-aws commented 6 years ago

Actually the best way to deal with all cases of "message cannot be successfully consumed for some reason" is to set up a dead letter queue and alarm on this queue not being empty. This allows you to quickly inspect the contents of the failing messages and redrive them for processing after fixing them (or the code) up.

jwindberg commented 6 years ago

I don't think you understand. The message was not wrong in any way. It was perfectly fine. The code, here in this project, was unable to parse the message attributes when they have a custom type specified. Take a close look at the getObjectValue method specified in the nested class on SQSMessage. There is no way it would ever be able to parse a value coming in with data type of "String.custom". It really does need to be able to parse those custom data types. I don't want the perfectly valid message to get sent to a dead letter queue, I want it processed.

jwindberg commented 6 years ago

Try sending a message to an SQS queue with a custom data type message attribute, and see what happens. It will throw a JMS exception and get returned.

"Number.int" is a valid data type "Number.int.custom" is also a valid data type. "String" is a valid data type. "String.custom" is also a valid data type.

jwindberg commented 6 years ago

This library won't allow perfectly valid sqs messages to get parsed, if they have a custom data type set for any message attribute.

kuba-aws commented 6 years ago

Yeah, sorry, I missed your point.

To confirm, this is more of an interoperability issue, where one side is using the JMS library to talk to SQS (here, the consumer) and the other does not (here, the producer). This would cause the extra attributes placed by the producer to fail to be properly deserialized by the consumer.

Please confirm that this issue does or does not affect cases where both producer and consumer are using the JMS library.

jwindberg commented 6 years ago

Never used the jms library to send messages. We just consume them. I would expect that if this library uses the same rules for creating message attributes as it does for consuming them, it ought work just fine. However, I find that the current handling of a parse failure on a message, to effectively silently return the message to be troublesome. I would much rather have the client code receive an SQSMessage object with issues than never get one at all. Perhaps leave the attribute parsing until external code actually calls getIntProperty or some such. That way the consumer can see the failure rather than have it cause messages to return to the queue without knowing why.

sebast26 commented 4 years ago

Recently I also was hit by this issue but fortunately there is a fix for this in version 1.0.7+ (https://github.com/awslabs/amazon-sqs-java-messaging-lib/releases/tag/1.0.7)