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

[Bugfix] Cannot publish SNS with MessageAttributes via aws-json #326

Merged

Conversation

kojisaikiAtSony
Copy link
Contributor

@kojisaikiAtSony kojisaikiAtSony commented Oct 15, 2024

We found a bug when upgrade our AWS SDK to use aws-json protocol.
There is a nil access error when request SNS Publish API with MessageAttributes.

Comment on lines +220 to 224
if r.MessageAttributes == nil {
r.MessageAttributes = make(map[string]MessageAttributeValue)
}

r.MessageAttributes[name] = MessageAttributeValue{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This line has not been tested since there is no test case for Publish with MessageAttributes.

@@ -52,6 +56,74 @@ func Test_Publish_sqs_json_raw(t *testing.T) {
assert.Equal(t, message, string(messages[0].MessageBody))
}

func Test_Publish_Sqs_With_Message_Attributes(t *testing.T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the new test case with MessageAttributes

@kojisaikiAtSony
Copy link
Contributor Author

Hi @Admiral-Piett can you review this, please?

Copy link
Owner

@Admiral-Piett Admiral-Piett left a comment

Choose a reason for hiding this comment

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

Nice, thanks for taking care of this!

@Admiral-Piett Admiral-Piett merged commit 4e06d3d into Admiral-Piett:master Oct 22, 2024
2 checks passed
@kojisaikiAtSony kojisaikiAtSony deleted the bugfix/sns-publish-request branch October 22, 2024 23:30
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.

2 participants