Skip to content

Fix SQS message attribute decoding - #140

Open
kokobd wants to merge 1 commit into
Nike-Inc:masterfrom
bellroy:fix-sqs-message-attribute
Open

kokobd wants to merge 1 commit into
Nike-Inc:masterfrom
bellroy:fix-sqs-message-attribute

Conversation

@kokobd

@kokobd kokobd commented Mar 17, 2026 •

Copy link
Copy Markdown
Contributor

SQS message attributes arrive as structured JSON objects, so Map Text Text rejects events containing them. Decode attributes into MessageAttribute, retaining custom type labels and representing values as String Text, Number Scientific, or Binary ByteString.

The decoder selects the value field required by dataType, preserves numeric precision and custom suffixes, and rejects unknown types or missing values. The reserved list fields are ignored. Standard queue events without a message group ID and events with empty message attributes remain supported.

This changes the public SQS attribute type and is intended for 1.2. Bellroy's monorepo still uses its existing fork pin; adopting this API there requires a separate caller migration.

@kokobd
kokobd requested a review from IamfromSpace as a code owner March 17, 2026 22:08

@JackKelly-Bellroy JackKelly-Bellroy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Beware copying from Amazonka too closely, it's constrained by what botocore says about data types when its generator emits the Haskell record definitions. We had to write amazonka-dynamodb-attributevalue to provide an actual sum type to make DynamoDB more ergonomic to use. Would it be worth doing similar here?

Comment thread src/AWS/Lambda/Events/SQS.hs Outdated
binaryValue :: Maybe ByteString,
stringListValues :: [Text],
binaryListValues :: [ByteString],
dataType :: Text

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this actually a sum type and should it be modeled as such?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, but dataType is slightly more complex than a closed enum. It can be String, Number, Binary, or one of the three plus .custom-label, for example String.hello-world. We could have something like:

data LabeledMessageAttributeValue = LableledMessageAttributeValue
  { dataTypeLabel :: Text,
    value :: MessageAttributeValue
  }

data MessageAttributeValue = String Text | Binary ByteString | Number Scientific

Do you have any suggestion about how to name these things?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From a discussion @kokobd and I had over voice: we decided on the call it's probably best to keep this mapping as close to what SQS invokes, so library users can see how to use it. But now, looking at the snippet you've posted above, I think something like that would be quite workable, perhaps with customTypeLabel :: Maybe Text:

data MessageAttribute = MessageAttribute
  { customTypeLabel :: Maybe Text
  , value :: MessageAttributeValue
  }

data MessageAttributeValue
  = Binary ByteString
  | Number Text
  | String Text

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree that we should make types ADT/Haskell-like where they fit. It's unfortunate that this is so often made so difficult :)

From here:

Number – Number attributes can store positive or negative numerical values. A number can have up to 38 digits of precision, and it can be between 10^-128 and 10^+126.

So... kind of odd. More precision than an f64, less exponent range than an f128.

Since we can't match the domain exactly, we'll need to make it larger. I think my lean would be to use a Number128 or a Scientific. Both are strictly larger, but not quite so large as Text. Other than that, this last recommendation looks good to me, and I'm not too attached to any specific number type approach.

@IamfromSpace

Copy link
Copy Markdown
Collaborator

Ideally, I think we'll want to get this into 1.2 as well, since that will bundle the breaking changes in this area.

Also, I think our best example of a safe default is the Rust SDK: https://docs.rs/aws_lambda_events/latest/aws_lambda_events/event/sqs/struct.SqsEvent.html since that's actually owned by AWS. There should be no need to type any more loosely than this. We may be able to type more strictly, but we'll want to do so carefully.

@kokobd kokobd changed the title Fix sqs message attribute Fix SQS message attribute decoding Sep 29, 2026
@kokobd
kokobd force-pushed the fix-sqs-message-attribute branch 2 times, most recently from 1021f16 to fbff269 Compare September 29, 2026 03:46
@Tristano8
Tristano8 force-pushed the fix-sqs-message-attribute branch from fbff269 to 45c2434 Compare October 30, 2026 07:33
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