Merge pull request #1358 from johntdyer/master

CHEF-5116 - Catch HTTPServerException for 404 in remote_file retry

2 files changed, 59 insertions, 36 deletions

diff --git a/lib/chef/provider/remote_file/content.rb b/lib/chef/provider/remote_file/content.rb
index 7f9e2332a8..3a8514e077 100644
--- a/lib/chef/provider/remote_file/content.rb
+++ b/lib/chef/provider/remote_file/content.rb
@@ -48,7 +48,7 @@ class Chef
           begin
             uri = URI.parse(source)
             raw_file = grab_file_from_uri(uri)
-          rescue SocketError, Errno::ECONNREFUSED, Errno::ENOENT, Errno::EACCES, Timeout::Error, Net::HTTPFatalError, Net::FTPError => e
+          rescue SocketError, Errno::ECONNREFUSED, Errno::ENOENT, Errno::EACCES, Timeout::Error, Net::HTTPServerException, Net::HTTPFatalError, Net::FTPError => e
             Chef::Log.warn("#{@new_resource} cannot be downloaded from #{source}: #{e.to_s}")
             if source = sources.shift
               Chef::Log.info("#{@new_resource} trying to download from another mirror")
diff --git a/spec/unit/provider/remote_file/content_spec.rb b/spec/unit/provider/remote_file/content_spec.rb
index 0f311dc0ec..4ee33aeefb 100644
--- a/spec/unit/provider/remote_file/content_spec.rb
+++ b/spec/unit/provider/remote_file/content_spec.rb
@@ -160,44 +160,68 @@ describe Chef::Provider::RemoteFile::Content do
 
   describe "when there is an array of sources and the first fails" do
 
-    let(:source) { [ "http://opscode.com/seattle.txt", "http://opscode.com/nyc.txt" ] }
-    before do
-      new_resource.stub(:checksum).and_return(nil)
-      current_resource.stub(:checksum).and_return(nil)
-      @uri0 = double("URI0")
-      @uri1 = double("URI1")
-      URI.should_receive(:parse).with(new_resource.source[0]).and_return(@uri0)
-      URI.should_receive(:parse).with(new_resource.source[1]).and_return(@uri1)
-      @http_fetcher_throws_exception = double("Chef::Provider::RemoteFile::HTTP")
-      @http_fetcher_throws_exception.should_receive(:fetch).at_least(:once).and_raise(Errno::ECONNREFUSED)
-      Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri0, new_resource, current_resource).and_return(@http_fetcher_throws_exception)
-    end
-
-    describe "when the second url succeeds" do
-      before do
-        @tempfile = double("Tempfile")
-        mtime = Time.now
-        http_fetcher_works = double("Chef::Provider::RemoteFile::HTTP", :fetch => @tempfile)
-        Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri1, new_resource, current_resource).and_return(http_fetcher_works)
-      end
-
-      it "should return a valid tempfile" do
-        content.tempfile.should == @tempfile
-      end
-
-      it "should not mutate the new_resource" do
-        content.tempfile
-        new_resource.source.length.should == 2
+    # https://github.com/opscode/chef/pull/1358#issuecomment-40853299
+    def create_exception(exception_class)
+      if [ Net::HTTPServerException, Net::HTTPFatalError ].include? exception_class
+        exception_class.new("message", {"something" => 1})
+      else
+        exception_class.new
       end
     end
 
-    describe "when both urls fail" do
-      before do
-        Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri1, new_resource, current_resource).and_return(@http_fetcher_throws_exception)
-      end
+    let(:source) { [ "http://opscode.com/seattle.txt", "http://opscode.com/nyc.txt" ] }
 
-      it "should propagate the error back to the caller" do
-        lambda { content.tempfile }.should raise_error(Errno::ECONNREFUSED)
+    ### Test each exception we care about and make sure they all behave properly
+    [
+      SocketError,
+      Errno::ECONNREFUSED,
+      Errno::ENOENT,
+      Errno::EACCES,
+      Timeout::Error,
+      Net::HTTPServerException,
+      Net::HTTPFatalError,
+      Net::FTPError
+    ].each do |exception|
+      describe "with an exception of #{exception}" do
+        before do
+          new_resource.stub(:checksum).and_return(nil)
+          current_resource.stub(:checksum).and_return(nil)
+          @uri0 = double("URI0")
+          @uri1 = double("URI1")
+          URI.should_receive(:parse).with(new_resource.source[0]).and_return(@uri0)
+          URI.should_receive(:parse).with(new_resource.source[1]).and_return(@uri1)
+          @http_fetcher_throws_exception = double("Chef::Provider::RemoteFile::HTTP")
+          @http_fetcher_throws_exception.should_receive(:fetch).at_least(:once).and_raise(create_exception(exception))
+          Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri0, new_resource, current_resource).and_return(@http_fetcher_throws_exception)
+        end
+
+        describe "the second url should succeed" do
+          before do
+            @tempfile = double("Tempfile")
+            mtime = Time.now
+            http_fetcher_works = double("Chef::Provider::RemoteFile::HTTP", :fetch => @tempfile)
+            Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri1, new_resource, current_resource).and_return(http_fetcher_works)
+          end
+
+          it "should return a valid tempfile" do
+            content.tempfile.should == @tempfile
+          end
+
+          it "should not mutate the new_resource" do
+            content.tempfile
+            new_resource.source.length.should == 2
+          end
+        end
+
+        describe "when both urls fail" do
+          before do
+            Chef::Provider::RemoteFile::Fetcher.should_receive(:for_resource).with(@uri1, new_resource, current_resource).and_return(@http_fetcher_throws_exception)
+          end
+
+          it "should propagate the error back to the caller" do
+            lambda { content.tempfile }.should raise_error(exception)
+          end
+        end
       end
     end
   end
@@ -227,4 +251,3 @@ describe Chef::Provider::RemoteFile::Content do
   end
 
 end
-