Merge pull request #14491 from MikeMcQuaid/api_non_fatal_errors

api: warn rather than fail if we've got a cached version.
Homebrew · Feb 3, 2023 · 0518958 · 0518958
2 parents b009077 + 561e520
commit 0518958
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 9 deletions.
diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb
@@ -20,14 +20,16 @@ module API
 
     API_DOMAIN = "https://formulae.brew.sh/api"
     HOMEBREW_CACHE_API = (HOMEBREW_CACHE/"api").freeze
-    MAX_RETRIES = 3
+
+    # Set a longer timeout just for large(r) files.
+    JSON_API_MAX_TIME = 10
 
     sig { params(endpoint: String).returns(Hash) }
     def fetch(endpoint)
       return cache[endpoint] if cache.present? && cache.key?(endpoint)
 
       api_url = "#{API_DOMAIN}/#{endpoint}"
-      output = Utils::Curl.curl_output("--fail", api_url, max_time: 5)
+      output = Utils::Curl.curl_output("--fail", api_url)
       raise ArgumentError, "No file found at #{Tty.underline}#{api_url}#{Tty.reset}" unless output.success?
 
       cache[endpoint] = JSON.parse(output.stdout)
@@ -38,18 +40,24 @@ def fetch(endpoint)
     sig { params(endpoint: String, target: Pathname).returns(Hash) }
     def fetch_json_api_file(endpoint, target:)
       retry_count = 0
-
       url = "#{API_DOMAIN}/#{endpoint}"
+      curl_args = %W[--compressed --silent #{url}]
+      curl_args.prepend("--time-cond", target) if target.exist? && !target.empty?
+
       begin
-        curl_args = %W[--compressed --silent #{url}]
-        curl_args.prepend("--time-cond", target) if target.exist? && !target.empty?
-        Utils::Curl.curl_download(*curl_args, to: target, max_time: 5)
+        # Disable retries here, we handle them ourselves below.
+        Utils::Curl.curl_download(*curl_args, to: target, max_time: JSON_API_MAX_TIME, retries: 0)
 
         JSON.parse(target.read)
+      rescue ErrorDuringExecution
+        raise unless target.exist?
+        raise if target.empty?
+
+        opoo "#{target.basename}: update failed, falling back to cached version."
       rescue JSON::ParserError
         target.unlink
         retry_count += 1
-        odie "Cannot download non-corrupt #{url}!" if retry_count > MAX_RETRIES
+        odie "Cannot download non-corrupt #{url}!" if retry_count > Homebrew::EnvConfig.curl_retries.to_i
 
         retry
       end
@@ -63,7 +71,7 @@ def fetch_file_source(filepath, repo:, git_head: nil)
 
       raw_url = "https://raw.githubusercontent.com/#{repo}/#{endpoint}"
       puts "Fetching #{raw_url}..."
-      output = Utils::Curl.curl_output("--fail", raw_url, max_time: 5)
+      output = Utils::Curl.curl_output("--fail", raw_url)
       raise ArgumentError, "No file found at #{Tty.underline}#{raw_url}#{Tty.reset}" unless output.success?
 
       cache[endpoint] = output.stdout

diff --git a/Library/Homebrew/test/api/cask_spec.rb b/Library/Homebrew/test/api/cask_spec.rb
@@ -46,7 +46,7 @@ def mock_curl_download(stdout:)
     it "fetches the source of a cask (defaulting to master when no `git_head` is passed)" do
       curl_output = OpenStruct.new(stdout: "foo", success?: true)
       expect(Utils::Curl).to receive(:curl_output)
-        .with("--fail", "https://raw.githubusercontent.com/Homebrew/homebrew-cask/master/Casks/foo.rb", max_time: 5)
+        .with("--fail", "https://raw.githubusercontent.com/Homebrew/homebrew-cask/master/Casks/foo.rb")
         .and_return(curl_output)
       described_class.fetch_source("foo", git_head: nil)
     end