Please review pull request #619: (#13584) master swallows errors during startup opened by (cprice-puppet)
Description:
Our main "exit_on_fail" method was only catching
and logging a limited subset of all possible errors;
this was preventing some types of errors (e.g.
File Access errors) from being logged, which makes
it much more difficult to troubleshoot failures
on CI and other locations.
- Opened: Tue Apr 03 01:11:37 UTC 2012
- Based on: puppetlabs:master (ba112e94619e6d35ca1d2d89975f787e537fe1e3)
- Requested merge: cprice-puppet:bug/master/13584-master-swallows-errors (1e249cd3ab2d112492694136cd702b2e2cc82ff1)
Diff follows:
diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index b97f2c3..90f1947 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -26,9 +26,9 @@ def daemonize
Process.setsid
Dir.chdir("/")
begin
- $stdin.reopen "/dev/null"
- $stdout.reopen "/dev/null", "a"
- $stderr.reopen $stdout
+ $stdin = File.open("/dev/null")
+ $stdout = File.open("/dev/null", "a")
+ $stderr = $stdout
Puppet::Util::Log.reopen
rescue => detail
Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 9d1e563..61bbaf6 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -476,8 +476,20 @@ def replace_file(file, default_mode, &block)
# @yield
def exit_on_fail(message, code = 1)
yield
- rescue ArgumentError, RuntimeError, NotImplementedError => detail
- Puppet.log_exception(detail, "Could not #{message}: #{detail}")
+ # First, we need to check and see if we are catching a SystemExit error. These will be raised
+ # when we daemonize/fork, and they do not necessarily indicate a failure case.
+ rescue SystemExit => err
+ if (err.status != 0)
+ # If we have an unexpected exit code, let's log the exception
+ Puppet.log_exception(err, "Could not #{message}: #{err}")
+ end
+ # Whatever the exit code, we respect it.
+ exit(err.status)
+
+ # Now we need to catch *any* other kind of exception, because we may be calling third-party
+ # code (e.g. webrick), and we have no idea what they might throw.
+ rescue Exception => err
+ Puppet.log_exception(err, "Could not #{message}: #{err}")
exit(code)
end
module_function :exit_on_fail
diff --git a/lib/puppet/util/log.rb b/lib/puppet/util/log.rb
index 41839b1..fc6a819 100644
--- a/lib/puppet/util/log.rb
+++ b/lib/puppet/util/log.rb
@@ -135,10 +135,10 @@ def Log.newdestination(dest)
flushqueue
@destinations[dest]
rescue => detail
- Puppet.log_exception(detail)
-
# If this was our only destination, then add the console back in.
newdestination(:console) if @destinations.empty? and (dest != :console and dest != "console")
+
+ Puppet.log_exception(detail)
end
end
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 3647a6d..cc84f5e 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -93,16 +93,22 @@ def handle(msg)
require 'puppet/util/colors'
include Puppet::Util::Colors
- def initialize
+ def initialize()
+ if ($stdout.is_a?(File) && $stdout.path == "/dev/null")
+ @out = STDERR
+ else
+ @out = $stdout
+ end
+
# Flush output immediately.
- $stdout.sync = true
+ @out.sync = true
end
def handle(msg)
if msg.source == "Puppet"
- puts colorize(msg.level, "#{msg.level}: #{msg}")
+ @out.puts colorize(msg.level, "#{msg.level}: #{msg}")
else
- puts colorize(msg.level, "#{msg.level}: #{msg.source}: #{msg}")
+ @out.puts colorize(msg.level, "#{msg.level}: #{msg.source}: #{msg}")
end
end
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
