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

Fix missing key in message sends by Producer issue #700

Closed
wants to merge 1 commit into from

Conversation

twang2218
Copy link

fixes #699

Signed-off-by: Tao Wang twang2218@gmail.com

@@ -134,7 +134,7 @@ BaseProducer.prototype.buildPayloads = function (payloads, topicMetadata) {
if (message instanceof KeyedMessage) {
return message;
}
return new Message(0, 0, '', message);
return new Message(0, 0, p.key || '', message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

what happens if the key is number 0? Would it be better to check if key != null and then use the key otherwise empty string?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Yes, even if I give a pass here, it would still having problem at encodeMessage(). So, I guess the key must be type of String, right? In such case, what if p.key = true? I think the boolean value will cause some problem, I think we should use _.isEmpty(p.key) to check it.

fixes SOHU-Co#699

Signed-off-by: Tao Wang <twang2218@gmail.com>
@twang2218
Copy link
Author

I updated the PR, now _.isEmpty() is used for validating the p.key

@hyperlink
Copy link
Collaborator

Fixed in #704 and published as 2.0.0. Thanks for your help.

@hyperlink hyperlink closed this Jul 20, 2017
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.

Missing 'key' for message with 'key' field sends by Producer
2 participants