Skip to content

Commit

Permalink
Handle the case HTTPClient doesn't yield block or body is nil
Browse files Browse the repository at this point in the history
If HTTP response body is empty, HTTPClient doesn't yield given block,
and its response object's body attribute is nil.
  • Loading branch information
nurse committed Sep 14, 2017
1 parent 40c36ee commit 708557d
Show file tree
Hide file tree
Showing 3 changed files with 58 additions and 35 deletions.
21 changes: 11 additions & 10 deletions lib/td/client/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ def do_get(url, params=nil, &block)
if block
current_total_chunk_size = 0
response = client.get(target, params, header) {|res, chunk|
break if res.status != 200
current_total_chunk_size += chunk.bytesize
block.call(res, chunk, current_total_chunk_size)
}
Expand All @@ -300,17 +301,17 @@ def do_get(url, params=nil, &block)
else
response = client.get(target, params, header)

validate_content_length!(response, response.body.bytesize) if @ssl
end
status = response.code
# retry if the HTTP error code is 500 or higher and we did not run out of retrying attempts
if !block_given? && status >= 500 && cumul_retry_delay < @max_cumul_retry_delay
$stderr.puts "Error #{status}: #{get_error(response)}. Retrying after #{retry_delay} seconds..."
sleep retry_delay
cumul_retry_delay += retry_delay
retry_delay *= 2
redo # restart from beginning of do-while loop
end

status = response.code
# retry if the HTTP error code is 500 or higher and we did not run out of retrying attempts
if !block_given? && status >= 500 && cumul_retry_delay < @max_cumul_retry_delay
$stderr.puts "Error #{status}: #{get_error(response)}. Retrying after #{retry_delay} seconds..."
sleep retry_delay
cumul_retry_delay += retry_delay
retry_delay *= 2
redo # restart from beginning of do-while loop
validate_content_length!(response, response.body.to_s.bytesize) if @ssl
end
rescue Errno::ECONNREFUSED, Errno::ECONNRESET, Timeout::Error, EOFError,
SystemCallError, OpenSSL::SSL::SSLError, SocketError, HTTPClient::TimeoutError,
Expand Down
51 changes: 26 additions & 25 deletions lib/td/client/api/job.rb
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,22 @@ def validate_content_length_with_range(response, current_total_chunk_size)
end
end

def validate_response_status(res, current_total_chunk_size=0)
case res.status
when 200
if current_total_chunk_size != 0
# try to resume but the server returns 200
raise_error("Get job result failed", res)
end
when 206 # resuming
else
if res.status/100 == 5
raise HTTPServerException
end
raise_error("Get job result failed", res)
end
end

class HTTPServerException < StandardError
end

Expand All @@ -297,43 +313,28 @@ def job_result_download(job_id, format='msgpack', autodecode=true)
current_total_chunk_size = 0
infl = nil
begin # LOOP of Network/Server errors
response = nil
client.get(url, params, header) do |res, chunk|
unless response
case res.status
when 200
if current_total_chunk_size != 0
# try to resume but the server returns 200
raise_error("Get job result failed", res)
end
when 206 # resuming
else
if res.status/100 == 5
raise HTTPServerException
end
raise_error("Get job result failed", res)
end
if infl.nil? && autodecode
case res.header['Content-Encoding'][0].to_s.downcase
when 'gzip'
infl = Zlib::Inflate.new(Zlib::MAX_WBITS + 16)
when 'deflate'
infl = Zlib::Inflate.new
end
response = client.get(url, params, header) do |res, chunk|
validate_response_status(res, current_total_chunk_size)
if infl.nil? && autodecode
case res.header['Content-Encoding'][0].to_s.downcase
when 'gzip'
infl = Zlib::Inflate.new(Zlib::MAX_WBITS + 16)
when 'deflate'
infl = Zlib::Inflate.new
end
end
response = res
current_total_chunk_size += chunk.bytesize
chunk = infl.inflate(chunk) if infl
yield chunk, current_total_chunk_size
end
validate_response_status(response)

# completed?
validate_content_length_with_range(response, current_total_chunk_size)
rescue Errno::ECONNREFUSED, Errno::ECONNRESET, Timeout::Error, EOFError,
SystemCallError, OpenSSL::SSL::SSLError, SocketError, HTTPClient::TimeoutError,
HTTPServerException => e
if response # at least a chunk is downloaded
if current_total_chunk_size > 0
if etag = response.header['ETag'][0]
header['If-Range'] = etag
header['Range'] = "bytes=#{current_total_chunk_size}-"
Expand Down
21 changes: 21 additions & 0 deletions spec/td/client/job_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,27 @@
expect(api.job_result(12345)).to eq ['hello', 'world']
end

it '200 (blank) -> 200 works fine' do
i = 0
stub_api_request(:get, '/v3/job/result/12345').
with(:query => {'format' => 'msgpack'}).
to_return do
i += 1
puts "count : #{i}"
{
:headers => {
'Content-Length' => packed.bytesize,
'Etag' => '"abcdefghijklmn"',
},
:body => (i == 1 ? nil : packed)
}
end
expect(api).to receive(:sleep).once
expect($stderr).to receive(:print)
expect($stderr).to receive(:puts)
expect(api.job_result(12345)).to eq ['hello', 'world']
end

end

describe 'job_result_format' do
Expand Down

0 comments on commit 708557d

Please sign in to comment.