Skip to content

Commit

Permalink
Don't retry streaming requests with blocks after data has been reciev…
Browse files Browse the repository at this point in the history
…ed (#1617)

Co-authored-by: Alex Woods <alextwoods@outlook.com>
  • Loading branch information
janko and alextwoods committed Jun 11, 2020
1 parent 3bd7d94 commit 9c05213
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 8 deletions.
2 changes: 2 additions & 0 deletions 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)
------------------

Expand Down
1 change: 1 addition & 0 deletions gems/aws-sdk-core/lib/seahorse/client/block_io.rb
Expand Up @@ -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

Expand Down
23 changes: 16 additions & 7 deletions gems/aws-sdk-core/lib/seahorse/client/plugins/response_target.rb
Expand Up @@ -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
Expand All @@ -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

Expand Down
7 changes: 6 additions & 1 deletion gems/aws-sdk-s3/lib/aws-sdk-s3/encryption/io_decrypter.rb
Expand Up @@ -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]
Expand All @@ -23,6 +24,10 @@ def finalize
@io.write(@cipher.final)
end

def size
@io.size
end

end
end
end
Expand Down

0 comments on commit 9c05213

Please sign in to comment.