Merge pull request #24275 from BrandenXia/curl-local-write-errors

curl: stop retrying local download write failures

Library/Homebrew/exceptions.rb

@@ -803,6 +803,9 @@ class DownloadError < RuntimeError
   end
 end
 
+# Raised when curl cannot write a download to the local filesystem.
+class DownloadWriteError < RuntimeError; end
+
 # Raised in {CurlDownloadStrategy#fetch}.
 class CurlDownloadStrategyError < RuntimeError
   sig { params(url: String, details: T.nilable(String)).void }

Library/Homebrew/test/download_strategies/curl_spec.rb

@@ -110,6 +110,17 @@ RSpec.describe CurlDownloadStrategy do
       FileUtils.touch strategy.temporary_path
     end
 
+    context "when the destination cannot be written" do
+      let(:specs) { { mirrors: ["https://mirror.example.com/foo.tar.gz"] } }
+
+      it "does not try a mirror after a local write failure" do
+        expect(strategy).to receive(:curl).once
+                                          .and_raise(ErrorDuringExecution.new(["curl", url], status: 23))
+
+        expect { strategy.fetch }.to raise_error(DownloadWriteError)
+      end
+    end
+
     it "calls curl with default arguments" do
       expect(strategy).to receive(:curl).with(
         "--remote-time",

Library/Homebrew/test/retryable_download_spec.rb

@@ -0,0 +1,22 @@
+# typed: true
+# frozen_string_literal: true
+
+require "retryable_download"
+
+RSpec.describe Homebrew::RetryableDownload do
+  subject(:download) { described_class.new(resource, tries: 3) }
+
+  let(:resource) do
+    Resource.new("foo").tap do |resource|
+      resource.url("https://example.com/foo.tar.gz")
+    end
+  end
+
+  it "does not retry a local write failure" do
+    allow(resource.downloader).to receive(:curl_headers).and_return({ responses: [] })
+    expect(resource.downloader).to receive(:curl).once
+                                                 .and_raise(ErrorDuringExecution.new(["curl"], status: 23))
+
+    expect { download.fetch(quiet: true) }.to raise_error(DownloadWriteError)
+  end
+end

Library/Homebrew/test/utils/curl_spec.rb

@@ -853,6 +853,53 @@ RSpec.describe "Utils::Curl" do
     end
   end
 
+  describe "::curl_download" do
+    let(:url) { "https://example.com/download.zip" }
+    let(:destination) { mktmpdir/"download.zip.incomplete" }
+    let(:write_error) do
+      ErrorDuringExecution.new(["curl", url], status: 23,
+                                              output: [[:stderr, "curl: (23) Failed writing received data"]])
+    end
+
+    before do
+      destination.write("partial")
+      allow(self).to receive(:curl).and_raise(write_error)
+    end
+
+    it "identifies the destination and suggests checking disk space and permissions" do
+      expect { curl_download(url, to: destination) }.to raise_error(DownloadWriteError, <<~EOS)
+        Could not write download to #{destination}
+        Check that the destination has enough free disk space and is writable.
+        curl: (23) Failed writing received data
+      EOS
+    end
+
+    it "preserves the partial download after a write failure" do
+      begin
+        curl_download(url, to: destination)
+      rescue RuntimeError
+        nil
+      end
+
+      expect(destination.read).to eq("partial")
+    end
+
+    it "preserves the original error as the cause" do
+      expect { curl_download(url, to: destination) }.to raise_error(DownloadWriteError) { |error|
+        expect(error.cause).to eq(write_error)
+      }
+    end
+
+    it "propagates network failures unchanged" do
+      network_error = ErrorDuringExecution.new(["curl", url], status: 7)
+      allow(self).to receive(:curl).and_raise(network_error)
+
+      expect { curl_download(url, to: destination) }.to raise_error { |error|
+        expect(error).to equal(network_error)
+      }
+    end
+  end
+
   describe "curl request deadlines" do
     let(:url) { "https://example.com/download.zip" }
     let(:start_time) { Time.at(1_700_000_000) }

Library/Homebrew/utils/curl.rb

@@ -349,6 +349,14 @@ module Utils
       args = ["--remote-time", "--output", destination.to_s, *args]
 
       curl(*args, **options, timeout: Utils::Timer.remaining!(end_time))
+    rescue ErrorDuringExecution => e
+      raise if e.exitstatus != 23 # CURLE_WRITE_ERROR
+
+      raise DownloadWriteError, <<~EOS
+        Could not write download to #{destination}
+        Check that the destination has enough free disk space and is writable.
+        #{strip_progress_bar(Tty.collapse_carriage_returns(e.stderr)).strip}
+      EOS
     end
 
     # Run after `Tty.collapse_carriage_returns`; a bar-only line becomes empty.