From 561e52046708b749b8ffbe8837dd55b913e8b40c Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Fri, 3 Feb 2023 10:22:50 +0000 Subject: [PATCH] api: warn rather than fail if we've got a cached version. Rather than failing every time we fail to update (breaking most usage while offline) instead only fail if we cannot obtain the JSON and we don't have a valid local file we can use. Also tweak the timeout and retry logic and values to make a bit more sense in this context and not be so noisy when offline. --- Library/Homebrew/api.rb | 24 ++++++++++++++++-------- Library/Homebrew/test/api/cask_spec.rb | 2 +- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index 49bb507b1df49..88cb7140cad5b 100644 --- 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 index b5dd7ec1f2296..5fb66577dc866 100644 --- 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