Handle empty response body case

[nurse]

No description provided.

[coveralls]

Coverage decreased (-0.04%) to 77.454% when pulling 708557d on handle-empty-response-body-case into e21f4d4 on master.

[uu59]

This is in else for if block, so !block_given? is always true. We don't need to check it.

[uu59]

Original code had redo out of if-else. Now it only exists in else.
Thus don't retry if block given and response code is 5xx. Is this behavior change intentional?

[nurse]

Original code also had redo inside if !block_given?, isn't it?

Why not retry if block given is because if block given the block may be not idempotent.

[uu59]

Ah I meant if-else for if block ... else ...

td-client-ruby/lib/td/client/api.rb

Lines 289 to 314 in e21f4d4

	if block
	current_total_chunk_size = 0
	response = client.get(target, params, header) {|res, chunk|
	current_total_chunk_size += chunk.bytesize
	block.call(res, chunk, current_total_chunk_size)
	}
	# XXX ext/openssl raises EOFError in case where underlying connection causes an error,
	# and msgpack-ruby that used in block handles it as an end of stream == no exception.
	# Therefor, check content size.
	validate_content_length!(response, current_total_chunk_size) if @ssl
	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

if block
  ...
else
  ...
end

if !block_given? && ..
  ...
  redo
end

changed to

if block
  ...
else
  ...
  if !block_given? && ...
    ...
    redo
  end
end

[uu59]

Ah nvm, sorry I got wrong about it. Behavior didn't changed

[uu59]

This line is intended for empty response body, so client.get(...) do ... end block wasn't called but response is assigned, right?
I need some comments for that situation because looks like just validate_response_status is called twice.
Also I think validate_response_status(response, 0) if response.body.nil? would be more explicit.

[nurse]

@uu59 fixed!

[coveralls]

Coverage decreased (-0.04%) to 77.454% when pulling e355ba1 on handle-empty-response-body-case into e21f4d4 on master.

[uu59]

👍