diff --git a/Gemfile.lock b/Gemfile.lock index dc7495b058b8f9f17c67cb38dacee3d98e659a5d..c19fb8df343fe71c21f8023cc2bdcd3446570c9b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -28,6 +28,7 @@ PATH specs: gitlab-http (0.1.0) activesupport (~> 7) + concurrent-ruby (~> 1.2) httparty (~> 0.21.0) ipaddress (~> 0.8.3) nokogiri (~> 1.15.4) diff --git a/gems/gitlab-http/Gemfile.lock b/gems/gitlab-http/Gemfile.lock index 1023e12efd605be53c1cbf5c1d99eace015c8224..1f4910d1d576be09a391763feb8be768463c2765 100644 --- a/gems/gitlab-http/Gemfile.lock +++ b/gems/gitlab-http/Gemfile.lock @@ -20,6 +20,7 @@ PATH specs: gitlab-http (0.1.0) activesupport (~> 7) + concurrent-ruby (~> 1.2) httparty (~> 0.21.0) ipaddress (~> 0.8.3) nokogiri (~> 1.15.4) diff --git a/gems/gitlab-http/README.md b/gems/gitlab-http/README.md index 13ff330bb19da6b306e624dff27b02d07631ed71..e717afbdb2c856733198edcc3ce60404246ae31d 100644 --- a/gems/gitlab-http/README.md +++ b/gems/gitlab-http/README.md @@ -24,16 +24,27 @@ end ### Actions -Basic examples; +Basic examples: ```ruby Gitlab::HTTP_V2.post(uri, body: body) Gitlab::HTTP_V2.try_get(uri, params) -response = Gitlab::HTTP_V2.head(project_url, verify: true) +response = Gitlab::HTTP_V2.head(project_url, verify: true) # returns an HTTParty::Response object -Gitlab::HTTP_V2.post(path, base_uri: base_uri, **params) +Gitlab::HTTP_V2.post(path, base_uri: base_uri, **params) # returns an HTTParty::Response object +``` + +Async usage examples: + +```ruby +lazy_response = Gitlab::HTTP_V2.get(location, async: true) + +lazy_response.execute # starts the request and returns the same LazyResponse object +lazy_response.wait # waits for the request to finish and returns the same LazyResponse object + +response = lazy_response.value # returns an HTTParty::Response object ``` ## Development diff --git a/gems/gitlab-http/gitlab-http.gemspec b/gems/gitlab-http/gitlab-http.gemspec index 6146ba7f78be7de3239268043c8a0e7da630f998..0033f17447b511b6f88defe50c9e95fada4aea53 100644 --- a/gems/gitlab-http/gitlab-http.gemspec +++ b/gems/gitlab-http/gitlab-http.gemspec @@ -20,6 +20,7 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_runtime_dependency 'activesupport', '~> 7' + spec.add_runtime_dependency 'concurrent-ruby', '~> 1.2' spec.add_runtime_dependency 'httparty', '~> 0.21.0' spec.add_runtime_dependency 'ipaddress', '~> 0.8.3' spec.add_runtime_dependency 'nokogiri', '~> 1.15.4' diff --git a/gems/gitlab-http/lib/gitlab/http_v2/client.rb b/gems/gitlab-http/lib/gitlab/http_v2/client.rb index c10197e03854d42e147e7d07c71475df11904ff5..52c9ab897f5c6ee60101b326ef3e50988eaa6a48 100644 --- a/gems/gitlab-http/lib/gitlab/http_v2/client.rb +++ b/gems/gitlab-http/lib/gitlab/http_v2/client.rb @@ -4,7 +4,8 @@ require 'net/http' require 'active_support/all' require_relative 'new_connection_adapter' -require_relative "exceptions" +require_relative 'exceptions' +require_relative 'lazy_response' module Gitlab module HTTP_V2 @@ -45,9 +46,12 @@ def configuration # TODO: This overwrites a method implemented by `HTTPParty` # The calls to `get/...` will call this method instead of `httparty_perform_request` def perform_request(http_method, path, options, &block) + raise_if_options_are_invalid(options) raise_if_blocked_by_silent_mode(http_method) if options.delete(:silent_mode_enabled) log_info = options.delete(:extra_log_info) + async = options.delete(:async) + options_with_timeouts = if !options.has_key?(:timeout) options.with_defaults(DEFAULT_TIMEOUT_OPTIONS) @@ -57,29 +61,57 @@ def perform_request(http_method, path, options, &block) if options[:stream_body] httparty_perform_request(http_method, path, options_with_timeouts, &block) + elsif async + async_perform_request(http_method, path, options, options_with_timeouts, log_info, &block) else - begin - start_time = nil - read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT) - - httparty_perform_request(http_method, path, options_with_timeouts) do |fragment| - start_time ||= system_monotonic_time - elapsed = system_monotonic_time - start_time - - raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout - - yield fragment if block - end - rescue HTTParty::RedirectionTooDeep - raise RedirectionTooDeep - rescue *HTTP_ERRORS => e - extra_info = log_info || {} - extra_info = log_info.call(e, path, options) if log_info.respond_to?(:call) - configuration.log_exception(e, extra_info) - - raise e + sync_perform_request(http_method, path, options, options_with_timeouts, log_info, &block) + end + end + + def async_perform_request(http_method, path, options, options_with_timeouts, log_info, &block) + start_time = nil + read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT) + + promise = Concurrent::Promise.new do + httparty_perform_request(http_method, path, options_with_timeouts) do |fragment| + start_time ||= system_monotonic_time + elapsed = system_monotonic_time - start_time + + raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout + + yield fragment if block end end + + LazyResponse.new(promise, path, options, log_info) + end + + def sync_perform_request(http_method, path, options, options_with_timeouts, log_info, &block) + start_time = nil + read_total_timeout = options.fetch(:timeout, DEFAULT_READ_TOTAL_TIMEOUT) + + httparty_perform_request(http_method, path, options_with_timeouts) do |fragment| + start_time ||= system_monotonic_time + elapsed = system_monotonic_time - start_time + + raise ReadTotalTimeout, "Request timed out after #{elapsed} seconds" if elapsed > read_total_timeout + + yield fragment if block + end + rescue HTTParty::RedirectionTooDeep + raise RedirectionTooDeep + rescue *HTTP_ERRORS => e + extra_info = log_info || {} + extra_info = log_info.call(e, path, options) if log_info.respond_to?(:call) + configuration.log_exception(e, extra_info) + + raise e + end + + def raise_if_options_are_invalid(options) + return unless options[:async] && (options[:stream_body] || options[:silent_mode_enabled]) + + raise ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`' end def raise_if_blocked_by_silent_mode(http_method) diff --git a/gems/gitlab-http/lib/gitlab/http_v2/lazy_response.rb b/gems/gitlab-http/lib/gitlab/http_v2/lazy_response.rb new file mode 100644 index 0000000000000000000000000000000000000000..65d1ab966445dbdae9a8489356b0cb3645ecd9ac --- /dev/null +++ b/gems/gitlab-http/lib/gitlab/http_v2/lazy_response.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + module HTTP_V2 + class LazyResponse + NotExecutedError = Class.new(StandardError) + + attr_reader :promise + + delegate :state, to: :promise + + def initialize(promise, path, options, log_info) + @promise = promise + @path = path + @options = options + @log_info = log_info + end + + def execute + @promise.execute + self + end + + def wait + @promise.wait + self + end + + def value + raise NotExecutedError, '`execute` must be called before `value`' if @promise.unscheduled? + + wait # wait for the promise to be completed + + raise @promise.reason if @promise.rejected? + + @promise.value + rescue HTTParty::RedirectionTooDeep + raise HTTP_V2::RedirectionTooDeep + rescue *HTTP_V2::HTTP_ERRORS => e + extra_info = @log_info || {} + extra_info = @log_info.call(e, @path, @options) if @log_info.respond_to?(:call) + Gitlab::HTTP_V2.configuration.log_exception(e, extra_info) + + raise e + end + end + end +end diff --git a/gems/gitlab-http/spec/gitlab/http_v2_spec.rb b/gems/gitlab-http/spec/gitlab/http_v2_spec.rb index bfa1dcd263395221743f277de5ace96c495ca1e2..3151761d375fe085ea3ab91999f21ada1a0a996c 100644 --- a/gems/gitlab-http/spec/gitlab/http_v2_spec.rb +++ b/gems/gitlab-http/spec/gitlab/http_v2_spec.rb @@ -450,4 +450,101 @@ def read_body(*) end end end + + context 'when options[:async] is true' do + context 'when it is a valid request' do + before do + stub_full_request('http://example.org', method: :any).to_return(status: 200, body: 'hello world') + end + + it 'returns a LazyResponse' do + result = described_class.get('http://example.org', async: true) + + expect(result).to be_a(Gitlab::HTTP_V2::LazyResponse) + expect(result.state).to eq(:unscheduled) + + expect(result.execute).to be_a(Gitlab::HTTP_V2::LazyResponse) + expect(result.wait).to be_a(Gitlab::HTTP_V2::LazyResponse) + + expect(result.value).to be_a(HTTParty::Response) + expect(result.value.body).to eq('hello world') + end + end + + context 'when the URL is denied' do + let(:url) { 'http://localhost:3003' } + let(:error_class) { Gitlab::HTTP_V2::BlockedUrlError } + let(:opts) { {} } + + let(:result) do + described_class.get(url, allow_local_requests: false, async: true, **opts) + end + + it 'returns a LazyResponse with error value' do + expect(result).to be_a(Gitlab::HTTP_V2::LazyResponse) + + expect { result.execute.value }.to raise_error(error_class) + end + + it 'logs the exception' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(error_class), {}) + + expect { result.execute.value }.to raise_error(error_class) + end + + context 'with extra_log_info as hash' do + let(:opts) { { extra_log_info: { a: :b } } } + + it 'handles the request' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(error_class), { a: :b }) + + expect { result.execute.value }.to raise_error(error_class) + end + end + + context 'with extra_log_info as proc' do + let(:extra_log_info) do + proc do |error, url, options| + { klass: error.class, url: url, options: options } + end + end + + let(:opts) { { extra_log_info: extra_log_info } } + + it 'handles the request' do + expect(described_class.configuration) + .to receive(:log_exception) + .with(instance_of(error_class), { url: url, klass: error_class, options: { allow_local_requests: false } }) + + expect { result.execute.value }.to raise_error(error_class) + end + end + end + end + + context 'when options[:async] and options[:stream_body] are true' do + before do + stub_full_request('http://example.org', method: :any) + end + + it 'raises an ArgumentError' do + expect { described_class.get('http://example.org', async: true, stream_body: true) } + .to raise_error(ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`') + end + end + + context 'when options[:async] and options[:silent_mode_enabled] are true' do + before do + stub_full_request('http://example.org', method: :any) + end + + it 'raises an ArgumentError' do + expect { described_class.get('http://example.org', async: true, silent_mode_enabled: true) } + .to raise_error(ArgumentError, '`async` cannot be used with `stream_body` or `silent_mode_enabled`') + end + end end