When we hit a syntax error in any face, a whole bunch of unrelated face things would blow up in horrible ways. Stack traces for all...
Now, instead, we catch that fault but specifically only in the face file and report it through our error logs, then quietly ignore the face. Reviewed-By: Nick Lewis <[email protected]> --- lib/puppet/interface/face_collection.rb | 21 ++++++++------------- spec/fixtures/faulty_face/puppet/face/syntax.rb | 8 ++++++++ spec/unit/interface/face_collection_spec.rb | 19 ++++++++++++++++++- 3 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 spec/fixtures/faulty_face/puppet/face/syntax.rb diff --git a/lib/puppet/interface/face_collection.rb b/lib/puppet/interface/face_collection.rb index 6e6afc5..baa4246 100644 --- a/lib/puppet/interface/face_collection.rb +++ b/lib/puppet/interface/face_collection.rb @@ -10,21 +10,12 @@ module Puppet::Interface::FaceCollection unless @loaded @loaded = true $LOAD_PATH.each do |dir| - next unless FileTest.directory?(dir) - Dir.chdir(dir) do - Dir.glob("puppet/face/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file| - iname = file.sub(/\.rb/, '') - begin - require iname - rescue Exception => detail - puts detail.backtrace if Puppet[:trace] - raise "Could not load #{iname} from #{dir}/#{file}: #{detail}" - end - end - end + Dir.glob("#{dir}/puppet/face/*.rb"). + collect {|f| File.basename(f, '.rb') }. + each {|name| self[name, :current] } end end - return @faces.keys.select {|name| @faces[name].length > 0 } + @faces.keys.select {|name| @faces[name].length > 0 } end def self.validate_version(version) @@ -124,6 +115,10 @@ module Puppet::Interface::FaceCollection rescue LoadError => e raise unless e.message =~ %r{-- puppet/face/#{name}$} # ...guess we didn't find the file; return a much better problem. + rescue SyntaxError => e + raise unless e.message =~ %r{puppet/face/#{name}\.rb:\d+: } + Puppet.err "Failed to load face #{name}:\n#{e}" + # ...but we just carry on after complaining. end return get_face(name, version) diff --git a/spec/fixtures/faulty_face/puppet/face/syntax.rb b/spec/fixtures/faulty_face/puppet/face/syntax.rb new file mode 100644 index 0000000..3b1e36c --- /dev/null +++ b/spec/fixtures/faulty_face/puppet/face/syntax.rb @@ -0,0 +1,8 @@ +Puppet::Face.define(:syntax, '1.0.0') do + action :foo do + when_invoked do |whom| + "hello, #{whom}" + end + # This 'end' is deliberately omitted, to induce a syntax error. + # Please don't fix that, as it is used for testing. --daniel 2011-05-02 +end diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb index 4358ef4..4ad8787 100755 --- a/spec/unit/interface/face_collection_spec.rb +++ b/spec/unit/interface/face_collection_spec.rb @@ -22,7 +22,7 @@ describe Puppet::Interface::FaceCollection do after :each do subject.instance_variable_set(:@faces, @original_faces) - $".clear ; @original_required.each do |item| $" << item end + @original_required.each {|f| $".push f unless $".include? f } end describe "::prefix_match?" do @@ -160,4 +160,21 @@ describe Puppet::Interface::FaceCollection do end end end + + context "faulty faces" do + before :each do + $:.unshift "#{PuppetSpec::FIXTURE_DIR}/faulty_face" + end + + after :each do + $:.delete_if {|x| x == "#{PuppetSpec::FIXTURE_DIR}/faulty_face"} + end + + it "should not die if a face has a syntax error" do + subject.faces.should be_include :help + subject.faces.should_not be_include :syntax + @logs.should_not be_empty + @logs.first.message.should =~ /syntax error/ + end + end end -- 1.7.5 -- 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.
