From 90c5d9bbc3f2e4a59b99e225f90aa75681b2e1c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Janko=20Marohni=C4=87?= Date: Tue, 19 Sep 2017 13:44:02 +0200 Subject: [PATCH 1/5] Retry streaming S3 object downloads --- .../lib/aws-sdk-core/plugins/retry_errors.rb | 9 +++++++-- .../lib/seahorse/client/http/response.rb | 4 ++-- .../lib/seahorse/client/net_http/handler.rb | 10 +++++++++- .../spec/aws/plugins/retry_errors_spec.rb | 16 +++++++++++++--- .../seahorse/client/net_http/handler_spec.rb | 16 ++++++++++++++++ .../lib/aws-sdk-s3/encryption/io_decrypter.rb | 4 ++++ 6 files changed, 51 insertions(+), 8 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index 16b1ca5b765..ee81f6aa0a0 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -117,6 +117,11 @@ def networking? NETWORKING_ERRORS.include?(@name) end + def invalid_bytes? + @error.is_a?(Seahorse::Client::NetworkingError) && + @error.original_error.is_a?(Seahorse::Client::NetHttp::Handler::TruncatedBodyError) + end + def server? (500..599).include?(@http_status_code) end @@ -198,7 +203,7 @@ def retry_request(context, error) context.retries += 1 context.config.credentials.refresh! if error.expired_credentials? context.http_request.body.rewind - context.http_response.reset + context.http_response.reset(truncate: error.invalid_bytes?) call(context) end @@ -209,7 +214,7 @@ def delay_retry(context) def should_retry?(context, error) error.retryable?(context) and context.retries < retry_limit(context) and - response_truncatable?(context) + (response_truncatable?(context) or !error.invalid_bytes?) end def retry_limit(context) diff --git a/gems/aws-sdk-core/lib/seahorse/client/http/response.rb b/gems/aws-sdk-core/lib/seahorse/client/http/response.rb index e39a68e1ffa..4f6927cc269 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/http/response.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/http/response.rb @@ -149,10 +149,10 @@ def on_error(&callback) end end - def reset + def reset(options = {}) @status_code = 0 @headers.clear - @body.truncate(0) + @body.truncate(0) if options[:truncate] @error = nil end diff --git a/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb b/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb index 49bf5d75b41..45db2a34bd1 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb @@ -79,8 +79,16 @@ def transmit(config, req, resp) bytes_received = 0 resp.signal_headers(status_code, headers) net_resp.read_body do |chunk| + if bytes_received < resp.body.size + chunk_index = resp.body.size - bytes_received + next_chunk = chunk.byteslice(chunk_index..-1) + else + next_chunk = chunk + end + + resp.signal_data(next_chunk) if next_chunk + bytes_received += chunk.bytesize - resp.signal_data(chunk) end complete_response(req, resp, bytes_received) diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb index a43bd7ea408..aeecbfbead9 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb @@ -351,17 +351,27 @@ def handle(send_handler = nil, &block) handle { |context| resp } end - it 'truncates the response body before each retry attempt' do + it 'truncates the response body before retry attempt of invalid bytes' 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 = Seahorse::Client::NetworkingError.new( + Seahorse::Client::NetHttp::Handler::TruncatedBodyError.new(11, 10)) + handle { |context| resp } + end + + it 'does not truncate response body before retry attempt of other errors' do + body = double('truncatable-body', pos: 100, truncate: 0) + resp.context.http_response.body = body + expect(body).to receive(:truncate).never resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil) handle { |context| resp } end - it 'skips retry if un-truncatable response body has received data' do + it 'skips retry on invalid bytes if response body is un-truncatable' do resp.context.http_response.body = double('write-once-body', pos: 100) - resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil) + resp.error = Seahorse::Client::NetworkingError.new( + Seahorse::Client::NetHttp::Handler::TruncatedBodyError.new(11, 10)) handle { |context| resp } expect(resp.context.retries).to eq(0) end diff --git a/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb b/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb index 8431ca50c8d..2fb09ce8ad7 100644 --- a/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb +++ b/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb @@ -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.http_response.body.write('response') + 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('response-body') + end + + it 'skips part of response body that has already been written' do + context.http_response.body.write('response-body') + 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('response-body') + end + it 'wraps errors with a NetworkingError' do stub_request(:any, endpoint).to_raise(EOFError) resp = make_request diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb index 68273738a90..9c3f518e08e 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb @@ -23,6 +23,10 @@ def finalize @io.write(@cipher.final) end + def size + @io.size + end + end end end From bc3af36f81629f429f5eb0116c7fda4009849c87 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Mon, 8 Jun 2020 16:34:42 -0700 Subject: [PATCH 2/5] Rest IO in IODecrypter on retries and re-use BlockIO on retries. --- .../lib/seahorse/client/block_io.rb | 1 + .../client/plugins/response_target.rb | 23 +++++++++++++------ .../lib/aws-sdk-s3/encryption/io_decrypter.rb | 3 ++- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb index 75af80c8689..9430ae6e99a 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb @@ -11,6 +11,7 @@ def initialize(&block) # @return [Integer] def write(chunk) @block.call(chunk) + ensure chunk.bytesize.tap { |chunk_size| @size += chunk_size } end diff --git a/gems/aws-sdk-core/lib/seahorse/client/plugins/response_target.rb b/gems/aws-sdk-core/lib/seahorse/client/plugins/response_target.rb index 9e003b74e36..64de371250f 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/plugins/response_target.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/plugins/response_target.rb @@ -28,7 +28,13 @@ def call(context) def add_event_listeners(context, target) handler = self context.http_response.on_headers(200..299) do - context.http_response.body = handler.send(:io, target) + # In a fresh response body will be a StringIO + # However, when a request is retried we may have + # an existing ManagedFile or BlockIO and those + # should be reused. + if context.http_response.body.is_a? StringIO + context.http_response.body = handler.send(:io, target) + end end context.http_response.on_success(200..299) do @@ -40,15 +46,18 @@ def add_event_listeners(context, target) context.http_response.on_error do body = context.http_response.body - File.unlink(body) if ManagedFile === body + + # When using response_target of file we do not want to write + # error messages to the file. So set the body to a new StringIO + if body.is_a? ManagedFile + File.unlink(body) + context.http_response.body = StringIO.new + end + # Aws::S3::Encryption::DecryptHandler (with lower priority) # has callbacks registered after ResponseTarget::Handler, # where http_response.body is an IODecrypter - # and has error callbacks handling for it. - # Thus avoid early remove of IODecrypter at ResponseTarget::Handler - unless context.http_response.body.respond_to?(:io) - context.http_response.body = StringIO.new - end + # and has error callbacks handling for it so no action is required here end end diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb index 9c3f518e08e..48e0ec26d50 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb @@ -8,7 +8,8 @@ class IODecrypter # @param [IO#write] io An IO-like object that responds to `#write`. def initialize(cipher, io) @cipher = cipher.clone - @io = io + # Ensure that IO is reset between retries + @io = io.tap { |io| io.truncate(0) if io.respond_to?(:truncate) } end # @return [#write] From 57e250bfd996488dcd4e3f2588287b8f57887cf9 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 10 Jun 2020 16:05:15 -0700 Subject: [PATCH 3/5] Remove generic retry of streaming responses after getting a 2XX. Keep resets of IO setting of IO on error --- .../lib/aws-sdk-core/plugins/retry_errors.rb | 15 +++-------- .../lib/seahorse/client/block_io.rb | 1 - .../lib/seahorse/client/http/response.rb | 4 +-- .../lib/seahorse/client/net_http/handler.rb | 10 +------- .../spec/aws/plugins/retry_errors_spec.rb | 25 ------------------- .../seahorse/client/net_http/handler_spec.rb | 16 ------------ gems/aws-sdk-personalize/CHANGELOG.md | 2 +- gems/aws-sdk-personalizeruntime/CHANGELOG.md | 2 +- 8 files changed, 9 insertions(+), 66 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb index f189251f2c1..b6adb19c6b3 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/retry_errors.rb @@ -243,13 +243,6 @@ def call(context) config.clock_skew.update_clock_correction(context) end - def invalid_bytes? - @error.is_a?(Seahorse::Client::NetworkingError) && - @error.original_error.is_a?(Seahorse::Client::NetHttp::Handler::TruncatedBodyError) - end - - def server? - (500..599).include?(@http_status_code) # Estimated skew needs to be updated on every request config.clock_skew.update_estimated_skew(context) @@ -383,7 +376,7 @@ def retry_request(context, error) context.retries += 1 context.config.credentials.refresh! if error.expired_credentials? context.http_request.body.rewind - context.http_response.reset(truncate: error.invalid_bytes?) + context.http_response.reset call(context) end @@ -392,9 +385,9 @@ def delay_retry(context) end def should_retry?(context, error) - error.retryable?(context) and - context.retries < retry_limit(context) and - (response_truncatable?(context) or !error.invalid_bytes?) + error.retryable?(context) && + context.retries < retry_limit(context) && + response_truncatable?(context) end def retry_limit(context) diff --git a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb index 9430ae6e99a..75af80c8689 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb @@ -11,7 +11,6 @@ def initialize(&block) # @return [Integer] def write(chunk) @block.call(chunk) - ensure chunk.bytesize.tap { |chunk_size| @size += chunk_size } end diff --git a/gems/aws-sdk-core/lib/seahorse/client/http/response.rb b/gems/aws-sdk-core/lib/seahorse/client/http/response.rb index 466b6b3fa74..bf0e071f544 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/http/response.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/http/response.rb @@ -154,10 +154,10 @@ def on_error(&callback) end end - def reset(options = {}) + def reset @status_code = 0 @headers.clear - @body.truncate(0) if options[:truncate] + @body.truncate(0) @error = nil end diff --git a/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb b/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb index 8e540e14f7b..3801f6000b3 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/net_http/handler.rb @@ -79,16 +79,8 @@ def transmit(config, req, resp) bytes_received = 0 resp.signal_headers(status_code, headers) net_resp.read_body do |chunk| - if bytes_received < resp.body.size - chunk_index = resp.body.size - bytes_received - next_chunk = chunk.byteslice(chunk_index..-1) - else - next_chunk = chunk - end - - resp.signal_data(next_chunk) if next_chunk - bytes_received += chunk.bytesize + resp.signal_data(chunk) end complete_response(req, resp, bytes_received) diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb index 463a16722aa..c5693750ed9 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb @@ -320,31 +320,6 @@ module Plugins handle_with_retry(test_case_def) end - - it 'truncates the response body before retry attempt of invalid bytes' 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 = Seahorse::Client::NetworkingError.new( - Seahorse::Client::NetHttp::Handler::TruncatedBodyError.new(11, 10)) - handle { |context| resp } - end - - it 'does not truncate response body before retry attempt of other errors' do - body = double('truncatable-body', pos: 100, truncate: 0) - resp.context.http_response.body = body - expect(body).to receive(:truncate).never - resp.error = RetryErrorsSvc::Errors::RequestLimitExceeded.new(nil,nil) - handle { |context| resp } - end - - it 'skips retry on invalid bytes if response body is un-truncatable' do - resp.context.http_response.body = double('write-once-body', pos: 100) - resp.error = Seahorse::Client::NetworkingError.new( - Seahorse::Client::NetHttp::Handler::TruncatedBodyError.new(11, 10)) - handle { |context| resp } - expect(resp.context.retries).to eq(0) - end end diff --git a/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb b/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb index 9b7388d1133..b7eeeaa0502 100644 --- a/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb +++ b/gems/aws-sdk-core/spec/seahorse/client/net_http/handler_spec.rb @@ -273,22 +273,6 @@ 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.http_response.body.write('response') - 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('response-body') - end - - it 'skips part of response body that has already been written' do - context.http_response.body.write('response-body') - 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('response-body') - end - it 'wraps errors with a NetworkingError' do stub_request(:any, endpoint).to_raise(EOFError) resp = make_request diff --git a/gems/aws-sdk-personalize/CHANGELOG.md b/gems/aws-sdk-personalize/CHANGELOG.md index 611b5a820b9..864e678a1d0 100644 --- a/gems/aws-sdk-personalize/CHANGELOG.md +++ b/gems/aws-sdk-personalize/CHANGELOG.md @@ -4,7 +4,7 @@ Unreleased Changes 1.13.0 (2020-06-05) ------------------ -* Feature - [Personalize] Adds ability to create and apply filters. +* Feature - Adds ability to create and apply filters. 1.12.0 (2020-05-28) ------------------ diff --git a/gems/aws-sdk-personalizeruntime/CHANGELOG.md b/gems/aws-sdk-personalizeruntime/CHANGELOG.md index 7760e9f5770..caf2025fcb1 100644 --- a/gems/aws-sdk-personalizeruntime/CHANGELOG.md +++ b/gems/aws-sdk-personalizeruntime/CHANGELOG.md @@ -4,7 +4,7 @@ Unreleased Changes 1.11.0 (2020-06-05) ------------------ -* Feature - [Personalize] Adds ability to apply filter to real-time recommendations +* Feature - Adds ability to apply filter to real-time recommendations 1.10.0 (2020-05-28) ------------------ From fd3fe7f5c3a3c1845b44d86014df3109eb5b7d1f Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 10 Jun 2020 16:44:03 -0700 Subject: [PATCH 4/5] Ensure block_io records the chunk_size --- gems/aws-sdk-core/lib/seahorse/client/block_io.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb index 75af80c8689..9430ae6e99a 100644 --- a/gems/aws-sdk-core/lib/seahorse/client/block_io.rb +++ b/gems/aws-sdk-core/lib/seahorse/client/block_io.rb @@ -11,6 +11,7 @@ def initialize(&block) # @return [Integer] def write(chunk) @block.call(chunk) + ensure chunk.bytesize.tap { |chunk_size| @size += chunk_size } end From 2e6446494b34af994db58641585df0d40b3b64cf Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Thu, 11 Jun 2020 14:20:41 -0700 Subject: [PATCH 5/5] Add changelog entry --- gems/aws-sdk-core/CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 05e3131d8f3..4b2d0f04e4a 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Don't retry streaming requests with blocks (#2311) + 3.99.1 (2020-06-11) ------------------