Properly handle unavailable packages

Signed-off-by: Tim Smith <tsmith@chef.io>

2 files changed, 32 insertions, 3 deletions

diff --git a/lib/chef/provider/package/homebrew.rb b/lib/chef/provider/package/homebrew.rb
index 1677c1a3dc..2407841255 100644
--- a/lib/chef/provider/package/homebrew.rb
+++ b/lib/chef/provider/package/homebrew.rb
@@ -94,7 +94,11 @@ class Chef
           @brew_info ||= begin
             command_array = ["info", "--json=v1"].concat Array(new_resource.package_name)
             # convert the array of hashes into a hash where the key is the package name
-            Hash[Chef::JSONCompat.from_json(brew_cmd_output(command_array)).collect { |pkg| [pkg["name"], pkg] }]
+
+            cmd_output = brew_cmd_output(command_array, allow_failure: true)
+            return nil if cmd_output.empty? # empty std_out == bad package queried
+
+            Hash[Chef::JSONCompat.from_json(cmd_output).collect { |pkg| [pkg["name"], pkg] }]
           end
         end
 
@@ -106,6 +110,8 @@ class Chef
         # @return [Hash] Package information
         #
         def package_info(package_name)
+          return nil if brew_info.nil? # continue to raise the nil up the chain
+
           # return the package hash if it's in the brew info hash
           return brew_info[package_name] if brew_info[package_name]
 
@@ -129,6 +135,9 @@ class Chef
         def installed_version(i)
           p_data = package_info(i)
 
+          # nil means we couldn't find anything
+          return nil if p_data.nil?
+
           if p_data["keg_only"]
             if p_data["installed"].empty?
               nil
@@ -156,16 +165,26 @@ class Chef
         def available_version(i)
           p_data = package_info(i)
 
+          # nil means we couldn't find anything
+          return nil if p_data.nil?
+
           p_data["versions"]["stable"]
         end
 
-        def brew_cmd_output(*command)
+        def brew_cmd_output(*command, **options)
           homebrew_uid = find_homebrew_uid(new_resource.respond_to?(:homebrew_user) && new_resource.homebrew_user)
           homebrew_user = Etc.getpwuid(homebrew_uid)
 
           logger.trace "Executing 'brew #{command.join(" ")}' as user '#{homebrew_user.name}'"
+
+          # allow the calling method to decide if the cmd should raise or not
+          # brew_info uses this when querying out available package info since a bad
+          # package name will raise and we want to surface a nil available package so that
+          # the package provider can magically handle that
+          shell_out_cmd = options[:allow_failure] ? :shell_out : :shell_out!
+
           # FIXME: this 1800 second default timeout should be deprecated
-          output = shell_out!("brew", *command, timeout: 1800, user: homebrew_uid, environment: { "HOME" => homebrew_user.dir, "RUBYOPT" => nil, "TMPDIR" => nil })
+          output = send(shell_out_cmd, "brew", *command, timeout: 1800, user: homebrew_uid, environment: { "HOME" => homebrew_user.dir, "RUBYOPT" => nil, "TMPDIR" => nil })
           output.stdout.chomp
         end
 
diff --git a/spec/unit/provider/package/homebrew_spec.rb b/spec/unit/provider/package/homebrew_spec.rb
index ef52597bd6..8e4f754d07 100644
--- a/spec/unit/provider/package/homebrew_spec.rb
+++ b/spec/unit/provider/package/homebrew_spec.rb
@@ -237,6 +237,11 @@ describe Chef::Provider::Package::Homebrew do
       allow(provider).to receive(:brew_cmd_output).and_return(brew_cmd_output_data)
       expect(provider.brew_info).to have_key("vim")
     end
+
+    it "returns nil if brew_cmd_output_data returned empty stdout" do
+      allow(provider).to receive(:brew_cmd_output).and_return("")
+      expect(provider.brew_info).to be_nil
+    end
   end
 
   describe "#installed_version" do
@@ -276,6 +281,11 @@ describe Chef::Provider::Package::Homebrew do
       allow(provider).to receive(:brew_info).and_return(brew_info_data)
       expect(provider.available_version("openssl")).to eql("1.1.1f")
     end
+
+    it "returns nil if brew_info returns nil" do
+      allow(provider).to receive(:brew_info).and_return(nil)
+      expect(provider.available_version("foo")).to be_nil
+    end
   end
 
   describe "#brew_cmd_output" do