Skip to content

Commit

Permalink
(FACT-3427) Improve custom fact logging
Browse files Browse the repository at this point in the history
Prior to this commit, the define_resolution method was incorrectly
passing two arguments to Facter::Log#log_exception when raising, causing
and ArgumentError.

This commit updates define_resolution's error handling to provide more
information including the fact name and backtrace information (if
available).

Additionally, this commit enables a test that was pending.
  • Loading branch information
mhashizume committed Sep 26, 2023
1 parent 5f615d8 commit 666c50f
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 7 deletions.
4 changes: 3 additions & 1 deletion lib/facter/custom_facts/util/fact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ def define_resolution(resolution_name, options = {}, &block)

resolve
rescue StandardError => e
log.log_exception("Unable to add resolve #{resolution_name.inspect} for fact #{@name}: #{e.message}")
msg = "Unable to add resolve #{resolution_name.inspect} for fact '#{@name}': #{e.message}"
msg += "\n" + e.backtrace.join("\n") if Options[:trace]
log.error(msg, true)
nil
end

Expand Down
15 changes: 9 additions & 6 deletions spec/custom_facts/util/fact_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,15 @@
expect(fact.resolution('named')).to be_a_kind_of Facter::Core::Aggregate
end

# it "raises an error if there is an existing resolution with a different type" do
# pending "We need to stop rescuing all errors when instantiating resolutions"
# fact.define_resolution('named')
# expect(fact.define_resolution('named', :type => :aggregate))
# .to raise_error(ArgumentError, /Cannot return resolution.*already defined as simple/)
# end
it 'raises an error if there is an existing resolution with a different type' do
expect(logger).to receive(:error).with(
"Unable to add resolve \"named\" for fact 'yay': Cannot return resolution named with type aggregate; already "\
'defined as simple', true
)

fact.define_resolution('named', type: :simple)
fact.define_resolution('named', type: :aggregate)
end

it 'returns existing resolutions by name' do
allow(Facter::Util::Resolution).to receive(:new).once.with('named', fact).and_return(res)
Expand Down

0 comments on commit 666c50f

Please sign in to comment.