diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb index 3c7ada83a2..c4c331fc4e 100644 --- a/Library/Homebrew/exceptions.rb +++ b/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 } diff --git a/Library/Homebrew/test/download_strategies/curl_spec.rb b/Library/Homebrew/test/download_strategies/curl_spec.rb index de36d77f6b..6f8264d745 100644 --- a/Library/Homebrew/test/download_strategies/curl_spec.rb +++ b/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", diff --git a/Library/Homebrew/test/retryable_download_spec.rb b/Library/Homebrew/test/retryable_download_spec.rb new file mode 100644 index 0000000000..95c07f8500 --- /dev/null +++ b/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 diff --git a/Library/Homebrew/test/utils/curl_spec.rb b/Library/Homebrew/test/utils/curl_spec.rb index 20d7fc4578..82b5bd3a24 100644 --- a/Library/Homebrew/test/utils/curl_spec.rb +++ b/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) } diff --git a/Library/Homebrew/utils/curl.rb b/Library/Homebrew/utils/curl.rb index b67712d122..30f8d85f7f 100644 --- a/Library/Homebrew/utils/curl.rb +++ b/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.