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

Use .nil? for nil check #29

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

suhailpatel
Copy link

@suhailpatel suhailpatel commented Nov 15, 2017

Currently the metadata nil check uses value == nil, unfortunately this causes issues with time fields and the time comparator with an exception:

[2017-11-15T23:01:51,491][FATAL][logstash.runner          ] An unexpected error occurred! {:error=>#<NoMethodError: undefined method `time' for nil:NilClass>, :backtrace=>["/usr/share/logstash/logstash-core/lib/logstash/timestamp.rb:13:in `<=>'", "org/jruby/RubyComparable.java:150:in `=='", "/usr/share/logstash/vendor/bundle/jruby/2.3.0/gems/logstash-output-gelf-3.1.4/lib/logstash/outputs/gelf.rb:146:in `block in receive'", "org/jruby/RubyHash.java:1343:in `each'", "/usr/share/logstash/vendor/bundle/jruby/2.3.0/gems/logstash-output-gelf-3.1.4/lib/logstash/outputs/gelf.rb:145:in `receive'", "/usr/share/logstash/logstash-core/lib/logstash/outputs/base.rb:92:in `block in multi_receive'", "org/jruby/RubyArray.java:1734:in `each'", "/usr/share/logstash/logstash-core/lib/logstash/outputs/base.rb:92:in `multi_receive'", "/usr/share/logstash/logstash-core/lib/logstash/output_delegator_strategies/legacy.rb:22:in `multi_receive'", "/usr/share/logstash/logstash-core/lib/logstash/output_delegator.rb:49:in `multi_receive'", "/usr/share/logstash/logstash-core/lib/logstash/pipeline.rb:538:in `block in output_batch'", "org/jruby/RubyHash.java:1343:in `each'", "/usr/share/logstash/logstash-core/lib/logstash/pipeline.rb:536:in `output_batch'", "/usr/share/logstash/logstash-core/lib/logstash/pipeline.rb:481:in `worker_loop'", "/usr/share/logstash/logstash-core/lib/logstash/pipeline.rb:439:in `block in start_workers'"]}

This modifies the code so the nil check is not an equality check but rather a Ruby nil? check

@suhailpatel
Copy link
Author

suhailpatel commented Nov 16, 2017

I've signed the CLA but Github hasn't reflected it (https://www.elastic.co/contributor-agreement?license=icla).

Is there any step I need to take before it takes effect? Turns out leaving this comment refreshed the CLA status

@suhailpatel
Copy link
Author

@jakelandis You seem to be the approver of PRs here, mind taking a look at this when you have a chance. I believe the bug is still present in the current versions

@vitkovskii
Copy link

@cwurm Hey :) Or somebody else, please merge this :)

Thanks a lot!

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