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 nil exception to empty headers during metadata assignment #140

Merged

Conversation

kaisecheng
Copy link
Contributor

@kaisecheng kaisecheng commented Feb 15, 2023

Fix: #141

This commit guard nil exception in metadata assignment

@kaisecheng kaisecheng changed the title Fix nil exception to empty headers in metadata during event assignment Fix nil exception to empty headers during metadata assignment Feb 15, 2023
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Tried locally and I think the test doesn't exercise the code, plus a minor suggestion to make it more declarative

@@ -374,6 +374,8 @@ def maybe_set_metadata(event, record)
end
if @metadata_mode.include?(:headers)
record.headers.each do |header|
next if header.nil? || header.value.nil? || header.key.nil?
Copy link
Contributor

@andsel andsel Feb 16, 2023

Choose a reason for hiding this comment

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

I tried to comment this line and ran the test, but was still green. Seems the test doesn't enter the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great that you spot it

@kaisecheng kaisecheng requested a review from andsel February 16, 2023 14:01
Copy link
Contributor

@andsel andsel left a comment

Choose a reason for hiding this comment

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

Once it's green, LGTM!

@kaisecheng kaisecheng merged commit 5f61588 into logstash-plugins:main Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kafka input throws TypeError when record return empty headers
3 participants