diff options
| -rwxr-xr-x | bin/puppet | 6 | ||||
| -rw-r--r-- | lib/puppet/error.rb | 7 | ||||
| -rw-r--r-- | lib/puppet/network/handler/configuration.rb | 17 | ||||
| -rw-r--r-- | lib/puppet/network/handler/master.rb | 7 | ||||
| -rw-r--r-- | lib/puppet/parser/interpreter.rb | 19 | ||||
| -rwxr-xr-x | spec/unit/parser/interpreter.rb | 51 |
6 files changed, 49 insertions, 58 deletions
diff --git a/bin/puppet b/bin/puppet index deeb65c42..a87a07619 100755 --- a/bin/puppet +++ b/bin/puppet @@ -197,13 +197,13 @@ begin config = client.getconfig config.apply rescue => detail + if Puppet[:trace] + puts detail.backtrace + end if detail.is_a?(XMLRPC::FaultException) $stderr.puts detail.message else $stderr.puts detail end - if Puppet[:trace] - puts detail.backtrace - end exit(1) end diff --git a/lib/puppet/error.rb b/lib/puppet/error.rb index 55fbf2e2f..971b31116 100644 --- a/lib/puppet/error.rb +++ b/lib/puppet/error.rb @@ -41,12 +41,5 @@ module Puppet # :nodoc: # An error class for when I don't know what happened. Automatically # prints a stack trace when in debug mode. class DevError < Puppet::Error - # XXX This is probably the wrong way to do this, but... - def set_backtrace(trace) - if Puppet[:trace] - puts trace - end - super - end end end diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb index 2df1b3ab4..353693bdc 100644 --- a/lib/puppet/network/handler/configuration.rb +++ b/lib/puppet/network/handler/configuration.rb @@ -107,16 +107,13 @@ class Puppet::Network::Handler benchmark(level, "Compiled configuration for %s" % node.name) do begin config = interpreter.compile(node) - rescue Puppet::Error => detail - if Puppet[:trace] - puts detail.backtrace - end - unless local? - Puppet.err detail.to_s - end - raise XMLRPC::FaultException.new( - 1, detail.to_s - ) + rescue => detail + # If we're local, then we leave it to the local system + # to handle error reporting, but otherwise we do it here + # so the interpreter doesn't need to know if the parser + # is local or not. + Puppet.err(detail.to_s) unless local? + raise end end diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index 030950c61..25c4318b8 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -77,12 +77,7 @@ class Puppet::Network::Handler Puppet::Node::Facts.new(client, facts).save # And get the configuration from the config handler - begin - config = config_handler.configuration(client) - rescue => detail - puts detail.backtrace - raise - end + config = config_handler.configuration(client) return translate(config.extract) end diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 81867c84b..0b8c035ba 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -25,7 +25,8 @@ class Puppet::Parser::Interpreter # evaluate our whole tree def compile(node) - return Puppet::Parser::Compile.new(node, parser(node.environment), :ast_nodes => usenodes?).compile + raise Puppet::ParseError, "Could not parse configuration; cannot compile" unless env_parser = parser(node.environment) + return Puppet::Parser::Compile.new(node, env_parser, :ast_nodes => usenodes?).compile end # create our interpreter @@ -74,15 +75,14 @@ class Puppet::Parser::Interpreter parser.parse return parser rescue => detail - if Puppet[:trace] - puts detail.backtrace - end msg = "Could not parse" if environment and environment != "" msg += " for environment %s" % environment end - msg += ": %s" % detail - raise Puppet::Error, detail + msg += ": %s" % detail.to_s + error = Puppet::Error.new(msg) + error.set_backtrace(detail.backtrace) + raise error end end @@ -96,8 +96,11 @@ class Puppet::Parser::Interpreter tmp = create_parser(environment) @parsers[environment].clear if @parsers[environment] @parsers[environment] = tmp - rescue - # Nothing, yo. + rescue => detail + # If a parser already exists, than assume that we logged the + # exception elsewhere and reuse the parser. If one doesn't + # exist, then reraise. + raise detail unless @parsers[environment] end end @parsers[environment] diff --git a/spec/unit/parser/interpreter.rb b/spec/unit/parser/interpreter.rb index a79267b52..b4aa2a2d1 100755 --- a/spec/unit/parser/interpreter.rb +++ b/spec/unit/parser/interpreter.rb @@ -100,12 +100,7 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do @parser = mock('parser') end - it "it should an exception when nothing is there and nil is returned" do - @interp.expects(:create_parser).with(:myenv).returns(nil) - @interp.send(:parser, :myenv).should be_nil - end - - it "should create and return a new parser and use the same parser when the parser does not need reparsing" do + it "should use the same parser when the parser does not need reparsing" do @interp.expects(:create_parser).with(:myenv).returns(@parser) @interp.send(:parser, :myenv).should equal(@parser) @@ -125,7 +120,12 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do @interp.send(:parser, :myenv).should equal(newparser) end - it "should keep the old parser if create_parser doesn't return anything." do + it "should fail intelligently if a parser cannot be created and one does not already exist" do + @interp.expects(:create_parser).with(:myenv).raises(ArgumentError) + proc { @interp.send(:parser, :myenv) }.should raise_error(ArgumentError) + end + + it "should keep the old parser if a new parser cannot be created" do # Get the first parser in the hash. @interp.expects(:create_parser).with(:myenv).returns(@parser) @interp.send(:parser, :myenv).should equal(@parser) @@ -134,7 +134,7 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do @parser.expects(:reparse?).returns(true) # But fail to create a new parser - @interp.expects(:create_parser).with(:myenv).returns(nil) + @interp.expects(:create_parser).with(:myenv).raises(ArgumentError) # And make sure we still get the old valid parser @interp.send(:parser, :myenv).should equal(@parser) @@ -154,27 +154,30 @@ end describe Puppet::Parser::Interpreter, " when compiling configurations" do before do @interp = Puppet::Parser::Interpreter.new + @node = stub 'node', :environment => :myenv + @compile = mock 'compile' + @parser = mock 'parser' end - it "should create a configuration with the node, parser, and whether to use ast nodes" do - node = mock('node') - node.expects(:environment).returns(:myenv) - compile = mock 'compile' - compile.expects(:compile).returns(:config) - parser = mock 'parser' - @interp.expects(:parser).with(:myenv).returns(parser) + it "should create a compile with the node, parser, and whether to use ast nodes when ast nodes is true" do + @compile.expects(:compile).returns(:config) + @interp.expects(:parser).with(:myenv).returns(@parser) @interp.expects(:usenodes?).returns(true) - Puppet::Parser::Compile.expects(:new).with(node, parser, :ast_nodes => true).returns(compile) - @interp.compile(node) + Puppet::Parser::Compile.expects(:new).with(@node, @parser, :ast_nodes => true).returns(@compile) + @interp.compile(@node) + end - # Now try it when usenodes is true - @interp = Puppet::Parser::Interpreter.new :UseNodes => false - node.expects(:environment).returns(:myenv) - compile.expects(:compile).returns(:config) - @interp.expects(:parser).with(:myenv).returns(parser) + it "should create a compile with the node, parser, and whether to use ast nodes when ast nodes is false" do + @compile.expects(:compile).returns(:config) + @interp.expects(:parser).with(:myenv).returns(@parser) @interp.expects(:usenodes?).returns(false) - Puppet::Parser::Compile.expects(:new).with(node, parser, :ast_nodes => false).returns(compile) - @interp.compile(node).should equal(:config) + Puppet::Parser::Compile.expects(:new).with(@node, @parser, :ast_nodes => false).returns(@compile) + @interp.compile(@node).should equal(:config) + end + + it "should fail intelligently when no parser can be found" do + @interp.expects(:parser).with(:myenv).returns(nil) + proc { @interp.compile(@node) }.should raise_error(Puppet::ParseError) end end |