Handle empty response body case

lib/td/client/api.rb

@@ -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)
           }
@@ -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 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,

lib/td/client/api/job.rb

@@ -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
 
@@ -297,43 +313,31 @@ 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
 
+      # for the case response body is empty
+      # Note that webmock returns response.body as "" instead of nil
+      validate_response_status(response, 0) if response.body.to_s.empty?
+
       # 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}-"

spec/spec_helper.rb

@@ -34,6 +34,15 @@
 
 include TreasureData
 
+RSpec.configure do |config|
+  # This allows you to limit a spec run to individual examples or groups
+  # you care about by tagging them with `:focus` metadata. When nothing
+  # is tagged with `:focus`, all examples get run. RSpec also provides
+  # aliases for `it`, `describe`, and `context` that include `:focus`
+  # metadata: `fit`, `fdescribe` and `fcontext`, respectively.
+  config.filter_run_when_matching :focus
+end
+
 shared_context 'common helper' do
   let :account_id do
     1

spec/td/client/job_api_spec.rb

@@ -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