Merge pull request #23385 from Homebrew/skip-bad-bottles

install: isolate formula and cask fetch errors

Library/Homebrew/install.rb

@@ -359,16 +359,7 @@ module Homebrew
           download_queue.fetch
 
           [:prelude, :enqueue_fetch].each do |step|
-            valid_formula_installers.select! do |fi|
-              fi.public_send(step)
-              true
-            rescue CannotInstallFormulaError => e
-              ofail e.message
-              false
-            rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
-              ofail "#{fi.formula}: #{e}"
-              false
-            end
+            valid_formula_installers = select_formula_installers(valid_formula_installers, step:)
             next if step == :enqueue_fetch && !fetch_after_enqueue
 
             download_queue.fetch
@@ -391,13 +382,20 @@ module Homebrew
           fi.download_queue = download_queue
         end
 
+        select_formula_installers(formula_installers, step: :prelude_fetch)
+      end
+
+      sig {
+        params(formula_installers: T::Array[FormulaInstaller], step: Symbol).returns(T::Array[FormulaInstaller])
+      }
+      def select_formula_installers(formula_installers, step:)
         formula_installers.select do |fi|
-          fi.prelude_fetch
+          fi.public_send(step)
           true
         rescue CannotInstallFormulaError => e
           ofail e.message
           false
-        rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
+        rescue => e
           ofail "#{fi.formula}: #{e}"
           false
         end
@@ -425,16 +423,29 @@ module Homebrew
 
       sig { params(cask_installers: T::Array[T.untyped], download_queue: Homebrew::DownloadQueue).void }
       def enqueue_cask_installers(cask_installers, download_queue:)
-        if cask_installers.any?(&:source_download_requires_pre_fetch?)
-          source_downloads = cask_installers.filter_map(&:prelude_fetch_download)
-          if source_downloads.any?
-            oh1 "Downloading Cask files"
-            source_downloads.each { |source_download| download_queue.enqueue(source_download) }
-            download_queue.fetch
+        source_downloads = []
+        valid_cask_installers = cask_installers.select do |cask_installer|
+          if cask_installer.source_download_requires_pre_fetch? &&
+             (source_download = cask_installer.prelude_fetch_download)
+            source_downloads << source_download
           end
+          true
+        rescue => e
+          ofail "#{cask_installer.cask}: #{e}"
+          false
         end
 
-        cask_installers.each(&:enqueue_downloads)
+        if source_downloads.any?
+          oh1 "Downloading Cask files"
+          source_downloads.each { |source_download| download_queue.enqueue(source_download) }
+          download_queue.fetch
+        end
+
+        valid_cask_installers.each do |cask_installer|
+          cask_installer.enqueue_downloads
+        rescue => e
+          ofail "#{cask_installer.cask}: #{e}"
+        end
       end
 
       sig {

Library/Homebrew/test/install_spec.rb

@@ -1,6 +1,7 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "cask/installer"
 require "install"
 require "dependency"
 require "test/support/fixtures/testball"
@@ -24,6 +25,49 @@ RSpec.describe Homebrew::Install do
     described_class.perform_preinstall_checks
   end
 
+  describe "::fetch_formulae" do
+    it "skips formulae whose fetch steps raise and continues with the rest" do
+      good_fi = FormulaInstaller.new(formula("good-bottle") do
+        T.bind(self, T.class_of(Formula))
+        url "foo-1.0"
+      end)
+      bad_fi = FormulaInstaller.new(formula("bad-bottle") do
+        T.bind(self, T.class_of(Formula))
+        url "foo-1.0"
+      end)
+      [good_fi, bad_fi].each do |fi|
+        allow(fi).to receive(:prelude_fetch)
+        allow(fi).to receive(:prelude)
+      end
+      allow(good_fi).to receive(:enqueue_fetch)
+      allow(bad_fi).to receive(:enqueue_fetch).and_raise("unexpected failure")
+
+      expect do
+        expect(described_class.fetch_formulae([good_fi, bad_fi])).to eq([good_fi])
+      end.to output(/Error: bad-bottle: unexpected failure/).to_stderr
+    end
+  end
+
+  describe "::enqueue_cask_installers" do
+    it "skips casks whose enqueue raises and continues with the rest" do
+      bad_cask = instance_double(Cask::Cask, to_s: "bad-cask")
+      bad_installer = instance_double(Cask::Installer, cask:                                bad_cask,
+                                                       source_download_requires_pre_fetch?: false)
+      allow(bad_installer).to receive(:enqueue_downloads)
+        .and_raise(URI::InvalidURIError, 'bad URI (is not URI?): "https://example.com/bad -cask.dmg"')
+      good_installer = instance_double(Cask::Installer, source_download_requires_pre_fetch?: false)
+      expect(good_installer).to receive(:enqueue_downloads)
+
+      download_queue = Homebrew::DownloadQueue.new(pour: true)
+      begin
+        expect { described_class.enqueue_cask_installers([bad_installer, good_installer], download_queue:) }
+          .to output(/Error: bad-cask: bad URI/).to_stderr
+      ensure
+        download_queue.shutdown
+      end
+    end
+  end
+
   describe "::print_dry_run_dependencies" do
     it "splits fresh installs and upgrades under separate headers" do
       fresh = formula("fresh-dep") do