diff --git a/gems/aws-sdk-s3/CHANGELOG.md b/gems/aws-sdk-s3/CHANGELOG.md index 26cbd801bc4..d3b31d6ebdc 100644 --- a/gems/aws-sdk-s3/CHANGELOG.md +++ b/gems/aws-sdk-s3/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Rewind the underlying file on a streaming retry that is not a truncated body (#2692). + 1.113.0 (2022-02-24) ------------------ diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/streaming_retry.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/streaming_retry.rb index 8141e0434b7..fd5fd6ec099 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/streaming_retry.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/streaming_retry.rb @@ -36,6 +36,17 @@ def truncate(_integer); end def rewind; end end + class NonRetryableStreamingError < StandardError + + def initialize(error) + super('Unable to retry request - retry could result in processing duplicated chunks.') + set_backtrace(error.backtrace) + @original_error = error + end + + attr_reader :original_error + end + # This handler works with the ResponseTarget plugin to provide smart # retries of S3 streaming operations that support the range parameter # (currently only: get_object). When a 200 OK with a TruncatedBodyError @@ -84,8 +95,19 @@ def add_event_listeners(context, target) end context.http_response.on_error do |error| - if retryable_body?(context) && truncated_body?(error) - context.http_request.headers[:range] = "bytes=#{context.http_response.body.size}-" + puts context.http_response.body + if retryable_body?(context) + if truncated_body?(error) + context.http_request.headers[:range] = "bytes=#{context.http_response.body.size}-" + else + case context.http_response.body + when RetryableManagedFile + # call rewind on the underlying file + context.http_response.body.instance_variable_get(:@file).rewind + else + raise NonRetryableStreamingError, error + end + end end end end diff --git a/gems/aws-sdk-s3/spec/plugins/streaming_retry_spec.rb b/gems/aws-sdk-s3/spec/plugins/streaming_retry_spec.rb index 60f30e9c90e..762974921b7 100644 --- a/gems/aws-sdk-s3/spec/plugins/streaming_retry_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/streaming_retry_spec.rb @@ -5,6 +5,7 @@ module Aws module S3 module Plugins + describe StreamingRetry do let(:creds) { Aws::Credentials.new('akid', 'secret') } let(:client) { S3::Client.new(region: 'us-east-1', credentials: creds, retry_base_delay: 0.001) } @@ -146,6 +147,33 @@ def stream_to_output output.rewind expect(File.read(tempfile.path)).to eq(parts[0]) end + + it 'rewinds the underlying file on non-truncated errors' do + parts = %w[data_part_1 part2 longer_data_at_end_part3] + full_test_data = parts.join + + first_call = true + allow_any_instance_of(Seahorse::Client::NetHttp::Handler).to receive(:transmit) do |s, config, req, resp| + if first_call + + resp.signal_headers(200, {}) + + underlying_file = resp.body.instance_variable_get(:@file) + expect(underlying_file).to receive(:rewind).and_call_original + + resp.signal_data(parts[0]) + resp.signal_error(Seahorse::Client::NetworkingError.new(SocketError.new)) + first_call = false + else + resp.signal_headers(200, {}) + resp.signal_data(full_test_data) + resp.signal_done + end + end + + client.get_object(request) + expect(File.read(tempfile.path)).to eq(full_test_data) + end end end end