Fixing #807. The exception handling should more closely resemble how it used to be done.

4 files changed, 32 insertions, 11 deletions

diff --git a/lib/puppet/network/handler/configuration.rb b/lib/puppet/network/handler/configuration.rb
index 05c86f22e..2c72d3d2b 100644
--- a/lib/puppet/network/handler/configuration.rb
+++ b/lib/puppet/network/handler/configuration.rb
@@ -111,7 +111,9 @@ class Puppet::Network::Handler
                     if Puppet[:trace]
                         puts detail.backtrace
                     end
-                    Puppet.err detail
+                    unless local?
+                        Puppet.err detail.to_s
+                    end
                     raise XMLRPC::FaultException.new(
                         1, detail.to_s
                     )
diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb
index 93a4bc170..a4ea26572 100644
--- a/lib/puppet/parser/interpreter.rb
+++ b/lib/puppet/parser/interpreter.rb
@@ -77,20 +77,27 @@ class Puppet::Parser::Interpreter
             if Puppet[:trace]
                 puts detail.backtrace
             end
-            Puppet.err "Could not parse for environment %s: %s" % [environment, detail]
-            return nil
+            msg = "Could not parse"
+            if environment and environment != ""
+                msg += " for environment %s" % environment
+            end
+            msg += ": %s" % detail
+            raise Puppet::Error, detail
         end
     end
 
     # Return the parser for a specific environment.
     def parser(environment)
         if ! @parsers[environment] or @parsers[environment].reparse?
-            if tmp = create_parser(environment)
+            # This will throw an exception if it does not succeed.  We only
+            # want to get rid of the old parser if we successfully create a new
+            # one.
+            begin
+                tmp = create_parser(environment)
                 @parsers[environment].clear if @parsers[environment]
                 @parsers[environment] = tmp
-            end
-            unless @parsers[environment]
-                raise Puppet::Error, "Could not parse any configurations"
+            rescue
+                # Nothing, yo.
             end
         end
         @parsers[environment]
diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 90bc56460..4429fe3a3 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -54,6 +54,16 @@ end
 
 describe Puppet::Node::Configuration, " when extracting transobjects" do
 
+    def mkscope
+        @parser = Puppet::Parser::Parser.new :Code => ""
+        @node = Puppet::Node.new("mynode")
+        @compile = Puppet::Parser::Compile.new(@node, @parser)
+
+        # XXX This is ridiculous.
+        @compile.send(:evaluate_main)
+        @scope = @compile.topscope
+    end
+
     def mkresource(type, name)
         Puppet::Parser::Resource.new(:type => type, :title => name, :source => @source, :scope => @scope)
     end
@@ -62,7 +72,7 @@ describe Puppet::Node::Configuration, " when extracting transobjects" do
     it "should transform the resource graph into a tree of TransBuckets and TransObjects" do
         config = Puppet::Node::Configuration.new("mynode")
 
-        @scope = mock 'scope'
+        @scope = mkscope
         @source = mock 'source'
 
         defined = mkresource("class", :main)
@@ -83,7 +93,7 @@ describe Puppet::Node::Configuration, " when extracting transobjects" do
     it "should transform arbitrarily deep graphs into isomorphic trees" do
         config = Puppet::Node::Configuration.new("mynode")
 
-        @scope = mock 'scope'
+        @scope = mkscope
         @scope.stubs(:tags).returns([])
         @source = mock 'source'
 
diff --git a/spec/unit/parser/interpreter.rb b/spec/unit/parser/interpreter.rb
index ebb7d4cbf..c0f9d54b3 100755
--- a/spec/unit/parser/interpreter.rb
+++ b/spec/unit/parser/interpreter.rb
@@ -67,7 +67,7 @@ describe Puppet::Parser::Interpreter, " when creating parser instances" do
 
     it "should return nothing when new parsers fail" do
         Puppet::Parser::Parser.expects(:new).with(:environment => :myenv).raises(ArgumentError)
-        @interp.send(:create_parser, :myenv).should be_nil
+        proc { @interp.send(:create_parser, :myenv) }.should raise_error(Puppet::Error)
     end
 
     it "should create parsers with environment-appropriate manifests" do
@@ -83,11 +83,13 @@ describe Puppet::Parser::Interpreter, " when creating parser instances" do
         parser1 = mock 'parser1'
         Puppet::Parser::Parser.expects(:new).with(:environment => :env1).returns(parser1)
         parser1.expects(:file=).with("/t/env1.pp")
+        parser1.expects(:parse)
         @interp.send(:create_parser, :env1)
 
         parser2 = mock 'parser2'
         Puppet::Parser::Parser.expects(:new).with(:environment => :env2).returns(parser2)
         parser2.expects(:file=).with("/t/env2.pp")
+        parser2.expects(:parse)
         @interp.send(:create_parser, :env2)
     end
 end
@@ -100,7 +102,7 @@ describe Puppet::Parser::Interpreter, " when managing parser instances" do
 
     it "it should an exception when nothing is there and nil is returned" do
         @interp.expects(:create_parser).with(:myenv).returns(nil)
-        lambda { @interp.send(:parser, :myenv) }.should raise_error(Puppet::Error)
+        @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