Skip to content

Use .nil? for nil check - #29

Open
suhailpatel wants to merge 1 commit into
logstash-plugins:mainfrom
suhailpatel:patch-1
Open

Use .nil? for nil check#29
suhailpatel wants to merge 1 commit into
logstash-plugins:mainfrom
suhailpatel:patch-1

Conversation

@suhailpatel

@suhailpatel suhailpatel commented Nov 15, 2017

Copy link
Copy Markdown

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

suhailpatel commented Nov 16, 2017

Copy link
Copy Markdown
Author

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
Copy Markdown
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
Copy Markdown

@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