Skip to content

Commit

Permalink
No longer swallowing errors from block passed to execute
Browse files Browse the repository at this point in the history
  • Loading branch information
Tony Novak committed Nov 20, 2015
1 parent 8b296b1 commit 9535ffa
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
20 changes: 13 additions & 7 deletions lib/google/apis/core/http_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ def initialize(method, url, body: nil)
# @raise [Google::Apis::AuthorizationError] Authorization is required
def execute(client)
prepare!
proc = block_given? ? Proc.new : nil
begin
Retriable.retriable tries: options.retries + 1,
base_interval: 1,
Expand All @@ -104,11 +103,19 @@ def execute(client)
Retriable.retriable tries: auth_tries,
on: [Google::Apis::AuthorizationError],
on_retry: proc { |*| refresh_authorization } do
return execute_once(client, &proc)
execute_once(client).tap do |result|
if block_given?
yield result, nil
end
end
end
end
rescue => e
raise e if proc.nil?
if block_given?
yield nil, e
else
raise e
end
end
ensure
release!
Expand Down Expand Up @@ -244,12 +251,11 @@ def error(err, rethrow: false, &block)
# @private
# @param [Hurley::Client] client
# HTTP client
# @yield [result, err] Result or error if block supplied
# @return [Object]
# @raise [Google::Apis::ServerError] An error occurred on the server and the request can be retried
# @raise [Google::Apis::ClientError] The request is invalid and should not be retried without modification
# @raise [Google::Apis::AuthorizationError] Authorization is required
def execute_once(client, &block)
def execute_once(client)
body.rewind if body.respond_to?(:rewind)
begin
logger.debug { sprintf('Sending HTTP %s %s', method, url) }
Expand All @@ -264,10 +270,10 @@ def execute_once(client, &block)
logger.debug { response.status_code }
logger.debug { response.inspect }
response = process_response(response.status_code, response.header, response.body)
success(response, &block)
success(response)
rescue => e
logger.debug { sprintf('Caught error %s', e) }
error(e, rethrow: true, &block)
error(e, rethrow: true)
end
end

Expand Down
9 changes: 5 additions & 4 deletions spec/google/apis/core/http_command_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,7 @@

context('with callbacks') do
it 'should return the response body after retries' do
expect { |b| command.execute(client, &b) }.to yield_successive_args(
[nil, an_instance_of(Google::Apis::ServerError)],
[nil, an_instance_of(Google::Apis::ServerError)],
['Hello world', nil])
expect { |b| command.execute(client, &b) }.to yield_with_args('Hello world', nil)
end
end
end
Expand Down Expand Up @@ -254,6 +251,10 @@
it 'should call block if present' do
expect { |b| command.execute(client, &b) }.to yield_with_args(nil, an_instance_of(Google::Apis::ClientError))
end

it 'should not swallow errors raised in block' do
expect { command.execute(client) { raise "Potatoes detected in tailpipe" } }.to raise_error("Potatoes detected in tailpipe")
end
end

it 'should send repeated query parameters' do
Expand Down

0 comments on commit 9535ffa

Please sign in to comment.