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

Add NullListener to suppress nil exceptions when listener lookup is unsu... #14

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

peakxu
Copy link

@peakxu peakxu commented Oct 6, 2014

...ccessful

@bfulton, please review.

Not sure where CI for this is meant to run. Travis? Ran specs locally and they passed.

Motivation: a recent build failure https://jenkins.swipely.com/view/app/job/app.service_performance_pipeline/lastFailedBuild/console

Relevant stack trace:
12:22:32.645 git-notes plugin: returning results of run: {:out=>"commit dcdf54d62b1aa1a56403e121f35f7a1da347b014\nMerge: 61f5d44 2d5d0c3\nAuthor: Peak Xu [email protected]\nDate: Mon Oct 6 12:20:13 2014 -0400\n\n Merge pull request #33 from dotswipely/deterministic_server_name\n \n Merge strings in aggregating hash in deterministic order\n\n", :err=>"", :val=>0}
12:22:33.398 ERROR: undefined method info' for nil:NilClass (NoMethodError) 12:22:33.399 /var/lib/jenkins/plugins/git-notes/WEB-INF/classes/lib/build_participant.rb:29:ininfo'
12:22:33.399 /var/lib/jenkins/plugins/git-notes/WEB-INF/classes/lib/build_participant.rb:58:in run' 12:22:33.399 /var/lib/jenkins/plugins/git-notes/WEB-INF/classes/lib/git_updater.rb:37:inpush_notes'
12:22:33.399 /var/lib/jenkins/plugins/git-notes/WEB-INF/classes/lib/git_updater.rb:14:in update!' 12:22:33.399 /var/lib/jenkins/plugins/git-notes/WEB-INF/classes/models/git_notes_publisher.rb:50:inupdate_git_notes'

@bfulton
Copy link
Contributor

bfulton commented Oct 6, 2014

It's disturbing that the listener is sometimes not available. We need to be able to rely on our logs for debugging, so I never want to silently not log. How about falling back to puts?

@peakxu
Copy link
Author

peakxu commented Oct 7, 2014

Very good point. Now prints to stderr.

BTW, agreed on this being disturbing. Curious about what the underlying issue is. This PR is more of a quick bandaid.

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