Change OperationFailedError itself to include the body in "message" if it's a 400.

4 files changed, 56 insertions, 32 deletions

diff --git a/lib/chef/chef_fs/file_system.rb b/lib/chef/chef_fs/file_system.rb
index 79ec09c167..a6e14e548c 100644
--- a/lib/chef/chef_fs/file_system.rb
+++ b/lib/chef/chef_fs/file_system.rb
@@ -394,10 +394,7 @@ class Chef
         rescue DefaultEnvironmentCannotBeModifiedError => e
           ui.warn "#{format_path.call(e.entry)} #{e.reason}." if ui
         rescue OperationFailedError => e
-          error_msg = "#{format_path.call(e.entry)} failed to #{e.operation}: #{e.message}"
-          error_cause = get_error_cause(e)
-          error_msg += " cause: #{error_cause}" if error_cause
-          ui.error error_msg if ui
+          ui.error "#{format_path.call(e.entry)} failed to #{e.operation}: #{e.message}" if ui
           error = true
         rescue OperationNotAllowedError => e
           ui.error "#{format_path.call(e.entry)} #{e.reason}." if ui
@@ -406,14 +403,6 @@ class Chef
         error
       end
 
-      def self.get_error_cause(error)
-        if error.respond_to?('cause') && error.cause.instance_of?(Net::HTTPServerException)
-          error.cause.response.body
-        else
-          nil
-        end
-      end
-
       def self.get_or_create_parent(entry, options, ui, format_path)
         parent = entry.parent
         if parent && !parent.exists?
diff --git a/lib/chef/chef_fs/file_system/operation_failed_error.rb b/lib/chef/chef_fs/file_system/operation_failed_error.rb
index 1af2d2dcff..28d170d628 100644
--- a/lib/chef/chef_fs/file_system/operation_failed_error.rb
+++ b/lib/chef/chef_fs/file_system/operation_failed_error.rb
@@ -27,6 +27,14 @@ class Chef
           @operation = operation
         end
 
+        def message
+          if cause && cause.is_a?(Net::HTTPExceptions) && cause.response.code == "400"
+            "#{super} cause: #{cause.response.body}"
+          else
+            super
+          end
+        end
+
         attr_reader :operation
       end
     end
diff --git a/spec/unit/chef_fs/file_system/operation_failed_error_spec.rb b/spec/unit/chef_fs/file_system/operation_failed_error_spec.rb
new file mode 100644
index 0000000000..570246c41f
--- /dev/null
+++ b/spec/unit/chef_fs/file_system/operation_failed_error_spec.rb
@@ -0,0 +1,47 @@
+#
+# Author:: John Keiser (<jkeiser@opscode.com>)
+# Copyright:: Copyright (c) 2012 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'spec_helper'
+require 'chef/chef_fs/file_system/operation_failed_error'
+
+describe Chef::ChefFS::FileSystem::OperationFailedError do
+  context 'message' do
+    let(:error_message) { 'HTTP error writing: 400 "Bad Request"' }
+
+    context 'has a cause attribute and HTTP result code is 400' do
+      it 'include error cause' do
+        allow_message_expectations_on_nil
+        response_body = '{"error":["Invalid key test in request body"]}'
+        @response.stub(:code).and_return("400")
+        @response.stub(:body).and_return(response_body)
+        exception = Net::HTTPServerException.new("(exception) unauthorized", @response)
+        proc {
+          raise Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self, exception), error_message
+        }.should raise_error(Chef::ChefFS::FileSystem::OperationFailedError, "#{error_message} cause: #{response_body}")
+      end
+    end
+
+    context 'does not have a cause attribute' do
+      it 'does not include error cause' do
+        proc {
+          raise Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self), error_message
+        }.should raise_error(Chef::ChefFS::FileSystem::OperationFailedError, error_message)
+      end
+    end
+  end
+end
diff --git a/spec/unit/chef_fs/file_system_spec.rb b/spec/unit/chef_fs/file_system_spec.rb
index b223dc084d..383a2c81ab 100644
--- a/spec/unit/chef_fs/file_system_spec.rb
+++ b/spec/unit/chef_fs/file_system_spec.rb
@@ -131,25 +131,5 @@ describe Chef::ChefFS::FileSystem do
         Chef::ChefFS::FileSystem.resolve_path(fs, '/y/x/w').path.should == '/y/x/w'
       end
     end
-
-    context 'get_error_cause' do
-      context 'error has a cause attribute and cause attribute is instance of Net::HTTPServerException' do
-        it 'return error cause' do
-          allow_message_expectations_on_nil
-          response_body = '{"error":["Invalid key test in request body"]}'
-          @response.stub(:body).and_return(response_body)
-          exception = Net::HTTPServerException.new("(exception) unauthorized", @response)
-          error = Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self, exception)
-          Chef::ChefFS::FileSystem.get_error_cause(error).should == response_body
-        end
-      end
-
-      context 'error has not a cause property' do
-        it 'return nil' do
-          error = Chef::ChefFS::FileSystem::OperationFailedError.new(:write, self)
-          Chef::ChefFS::FileSystem.get_error_cause(error).should == nil
-        end
-      end
-    end
   end
 end