Cleanup and document http_get_sock

Correctly fail when we get non-200 HTTP responses and retry on
the next URI.

2 files changed, 45 insertions, 4 deletions

diff --git a/lib/mogilefs/mogilefs.rb b/lib/mogilefs/mogilefs.rb
index 6a070fd..95d44f6 100644
--- a/lib/mogilefs/mogilefs.rb
+++ b/lib/mogilefs/mogilefs.rb
@@ -69,7 +69,7 @@ class MogileFS::MogileFS < MogileFS::Client
           sock = http_get_sock(URI.parse(path))
           return block_given? ? yield(sock) : sock.read
         rescue MogileFS::Timeout, Errno::ECONNREFUSED,
-               EOFError, SystemCallError
+               EOFError, SystemCallError, MogileFS::InvalidResponseError
           next
         end
       else
@@ -256,15 +256,20 @@ class MogileFS::MogileFS < MogileFS::Client
 
   protected
 
+    # given a URI, this returns a readable socket with ready data from the
+    # body of the response.
     def http_get_sock(uri)
       sock = Socket.mogilefs_new_request(uri.host, uri.port,
                                     "GET #{uri.request_uri} HTTP/1.0\r\n\r\n",
                                     @get_file_data_timeout)
       buf = sock.recv(4096, Socket::MSG_PEEK)
       head, body = buf.split(/\r\n\r\n/, 2)
-      head = sock.recv(head.size + 4, 0)
-
-      sock
+      if head =~ %r{\AHTTP/\d+\.\d+\s+200\s+}
+        sock.recv(head.size + 4, 0)
+        return sock
+      end
+      raise MogileFS::InvalidResponseError,
+            "GET on #{uri} returned: #{head.inspect}"
     end # def http_get_sock
 
 end
diff --git a/test/test_mogilefs.rb b/test/test_mogilefs.rb
index 93de323..2fd7a46 100644
--- a/test/test_mogilefs.rb
+++ b/test/test_mogilefs.rb
@@ -44,6 +44,42 @@ class TestMogileFS__MogileFS < TestMogileFS
       TempServer.destroy_all!
   end
 
+  def test_get_file_data_http_not_found_failover
+    accept_nr = 0
+    svr1 = Proc.new do |serv, port|
+      client, client_addr = serv.accept
+      client.sync = true
+      readed = client.recv(4096, 0)
+      assert(readed =~ \
+            %r{\AGET /dev1/0/000/000/0000000062\.fid HTTP/1.[01]\r\n\r\n\Z})
+      client.send("HTTP/1.0 404 Not Found\r\n\r\ndata!", 0)
+      accept_nr += 1
+      client.close
+    end
+
+    svr2 = Proc.new do |serv, port|
+      client, client_addr = serv.accept
+      client.sync = true
+      readed = client.recv(4096, 0)
+      assert(readed =~ \
+            %r{\AGET /dev2/0/000/000/0000000062\.fid HTTP/1.[01]\r\n\r\n\Z})
+      client.send("HTTP/1.0 200 OK\r\nContent-Length: 5\r\n\r\ndata!", 0)
+      accept_nr += 1
+      client.close
+    end
+
+    t1 = TempServer.new(svr1)
+    t2 = TempServer.new(svr2)
+    path1 = "http://127.0.0.1:#{t1.port}/dev1/0/000/000/0000000062.fid"
+    path2 = "http://127.0.0.1:#{t2.port}/dev2/0/000/000/0000000062.fid"
+    @backend.get_paths = { 'paths' => 2, 'path1' => path1, 'path2' => path2 }
+
+    assert_equal 'data!', @client.get_file_data('key')
+    assert_equal 2, accept_nr
+    ensure
+      TempServer.destroy_all!
+  end
+
   def test_get_file_data_http_block
     tmpfp = Tempfile.new('test_mogilefs.open_data')
     nr = nr_chunks