Skip to content

Commit

Permalink
Retry streaming S3 object downloads
Browse files Browse the repository at this point in the history
  • Loading branch information
janko committed Sep 19, 2017
1 parent 16e5b83 commit 07f1f5e
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 33 deletions.
8 changes: 1 addition & 7 deletions gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb
Expand Up @@ -118,7 +118,6 @@ def retry_request(context, error)
delay_retry(context)
context.retries += 1
context.config.credentials.refresh! if error.expired_credentials?
context.http_request.body.rewind
context.http_response.reset
call(context)
end
Expand All @@ -129,8 +128,7 @@ def delay_retry(context)

def should_retry?(context, error)
retryable?(context, error) and
context.retries < retry_limit(context) and
response_truncatable?(context)
context.retries < retry_limit(context)
end

def retryable?(context, error)
Expand All @@ -149,10 +147,6 @@ def retry_limit(context)
context.config.retry_limit
end

def response_truncatable?(context)
context.http_response.body.respond_to?(:truncate)
end

end

def add_handlers(handlers, config)
Expand Down
1 change: 0 additions & 1 deletion gems/aws-sdk-core/lib/seahorse/client/http/response.rb
Expand Up @@ -152,7 +152,6 @@ def on_error(&callback)
def reset
@status_code = 0
@headers.clear
@body.truncate(0)
@error = nil
end

Expand Down
19 changes: 16 additions & 3 deletions gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb
Expand Up @@ -44,7 +44,7 @@ class InvalidHttpVerbError < StandardError; end
# @param [RequestContext] context
# @return [Response]
def call(context)
transmit(context.config, context.http_request, context.http_response)
transmit(context.config, context.http_request, context.http_response, context)
Response.new(context: context)
end

Expand All @@ -69,18 +69,31 @@ def error_message(req, error)
# @param [Http::Request] req
# @param [Http::Response] resp
# @return [void]
def transmit(config, req, resp)
def transmit(config, req, resp, context)
session(config, req) do |http|
http.request(build_net_request(req)) do |net_resp|

status_code = net_resp.code.to_i
headers = extract_headers(net_resp)

bytes_received = 0
context.bytes_written ||= 0
resp.signal_headers(status_code, headers)
net_resp.read_body do |chunk|
if bytes_received < context.bytes_written
chunk_index = context.bytes_written - bytes_received
next_chunk = chunk.byteslice(chunk_index..-1)
else
next_chunk = chunk
end

if next_chunk
resp.signal_data(next_chunk)

context.bytes_written += next_chunk.bytesize
end

bytes_received += chunk.bytesize
resp.signal_data(chunk)
end
complete_response(req, resp, bytes_received)

Expand Down
3 changes: 3 additions & 0 deletions gems/aws-sdk-core/lib/seahorse/client/request_context.rb
Expand Up @@ -47,6 +47,9 @@ def initialize(options = {})
# @return [Integer]
attr_accessor :retries

# @return [Integer]
attr_accessor :bytes_written

# @return [Hash]
attr_reader :metadata

Expand Down
22 changes: 0 additions & 22 deletions gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb
Expand Up @@ -217,28 +217,6 @@ def handle(send_handler = nil, &block)
expect(resp.context.retries).to eq(3)
end

it 'rewinds the request body before each retry attempt' do
body = resp.context.http_request.body
expect(body).to receive(:rewind).exactly(3).times
resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil)
handle { |context| resp }
end

it 'truncates the response body before each retry attempt' do
body = double('truncatable-body', pos: 100, truncate: 0)
resp.context.http_response.body = body
expect(body).to receive(:truncate).with(0).exactly(3).times
resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil)
handle { |context| resp }
end

it 'skips retry if un-truncatable response body has received data' do
resp.context.http_response.body = double('write-once-body', pos: 100)
resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil)
handle { |context| resp }
expect(resp.context.retries).to eq(0)
end

it 'retries if creds expire and are refreshable' do
expect(credentials).to receive(:refresh!).exactly(3).times
resp.error = RetryErrorsSvc::Errors::AuthFailure.new(nil,nil)
Expand Down
16 changes: 16 additions & 0 deletions gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb
Expand Up @@ -251,6 +251,22 @@ def endpoint
expect(resp_body.read).to eq('response-body')
end

it 'populates part of response body that has not been written yet' do
context.bytes_written = 9
stub_request(:any, endpoint).to_return(body: 'response-body')
resp_body = make_request.context.http_response.body
resp_body.rewind
expect(resp_body.read).to eq('body')
end

it 'does not populate part of response body that has already been written' do
context.bytes_written = 15
stub_request(:any, endpoint).to_return(body: 'response-body')
resp_body = make_request.context.http_response.body
resp_body.rewind
expect(resp_body.read).to eq('')
end

it 'wraps errors with a NetworkingError' do
stub_request(:any, endpoint).to_raise(EOFError)
resp = make_request
Expand Down

0 comments on commit 07f1f5e

Please sign in to comment.