Improved error reporting especially around never shallowing exceptions. Debugging helpers should be much easier now #980 [Nicholas Seckar]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@984 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

10 files changed, 55 insertions, 24 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index 9aca63d0bb..105622e7d3 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Improved error reporting especially around never shallowing exceptions. Debugging helpers should be much easier now #980 [Nicholas Seckar]
+
 * Fixed Toggle.display in prototype.js #902 [Lucas Carlson]
 
 
diff --git a/actionpack/lib/action_controller/dependencies.rb b/actionpack/lib/action_controller/dependencies.rb
index 1deaf0ce4b..7fb72417c0 100644
--- a/actionpack/lib/action_controller/dependencies.rb
+++ b/actionpack/lib/action_controller/dependencies.rb
@@ -71,8 +71,8 @@ module ActionController #:nodoc:
           dependencies.flatten.each do |dependency|
             begin
               require_dependency(dependency.to_s)
-            rescue LoadError
-              raise LoadError, "Missing #{layer} #{dependency}.rb"
+            rescue LoadError => e
+              raise LoadError.new("Missing #{layer} #{dependency}.rb").copy_blame!(e)
             rescue Object => exception
               exception.blame_file! "=> #{layer} #{dependency}.rb"
               raise
@@ -83,12 +83,14 @@ module ActionController #:nodoc:
         def inherited(child)
           inherited_without_model(child)
           return if child.controller_name == "application" # otherwise the ApplicationController in Rails will include itself
+          model_name = child.controller_name.singularize
           begin
-            child.model(child.controller_name.singularize)
-          rescue NameError, LoadError
-            # No neither singular or plural model available for this controller
+            require_dependency model_name
+            child.model model_name
+          rescue MissingSourceFile => e
+            raise unless e.path == model_name + '.rb'
           end
         end        
     end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/lib/action_controller/helpers.rb b/actionpack/lib/action_controller/helpers.rb
index fcfd45a507..1db8a3ceba 100644
--- a/actionpack/lib/action_controller/helpers.rb
+++ b/actionpack/lib/action_controller/helpers.rb
@@ -52,12 +52,13 @@ module ActionController #:nodoc:
             when String, Symbol
               file_name  = arg.to_s.underscore + '_helper'
               class_name = file_name.camelize
-
+                
               begin
                 require_dependency(file_name)
               rescue LoadError => load_error
                 requiree = / -- (.*?)(\.rb)?$/.match(load_error).to_a[1]
-                raise LoadError, requiree == file_name ? "Missing helper file helpers/#{file_name}.rb" : "Can't load file: #{requiree}"
+                msg = (requiree == file_name) ? "Missing helper file helpers/#{file_name}.rb" : "Can't load file: #{requiree}"
+                raise LoadError.new(msg).copy_blame!(load_error)
               end
 
               add_template_helper(class_name.constantize)
@@ -90,10 +91,9 @@ module ActionController #:nodoc:
       private 
         def inherited(child)
           inherited_without_helper(child)
-          begin
-            child.helper(child.controller_path)
-          rescue ArgumentError, LoadError
-            # No default helper available for this controller
+          begin child.helper(child.controller_path)
+          rescue MissingSourceFile => e
+            raise unless e.path == "helpers/#{child.controller_path}_helper.rb"
           end
         end        
     end
diff --git a/actionpack/lib/action_controller/routing.rb b/actionpack/lib/action_controller/routing.rb
index 6d5f108484..28d692aa5b 100644
--- a/actionpack/lib/action_controller/routing.rb
+++ b/actionpack/lib/action_controller/routing.rb
@@ -312,7 +312,7 @@ module ActionController
           route_file = defined?(RAILS_ROOT) ? File.join(RAILS_ROOT, 'config', 'routes') : nil
           require_dependency(route_file) if route_file
         rescue LoadError, ScriptError => e
-          raise RoutingError, "Cannot load config/routes.rb:\n    #{e.message}"
+          raise RoutingError.new("Cannot load config/routes.rb:\n    #{e.message}").copy_blame!(e)
         ensure # Ensure that there is at least one route:
           connect(':controller/:action/:id', :action => 'index', :id => nil) if @routes.empty?
         end
diff --git a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
index 30a1a05d1f..23e622a47a 100644
--- a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
+++ b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
@@ -1,6 +1,8 @@
-<% if @exception.blamed_files && !@exception.blamed_files.empty? %>
-  <a href="#" onclick="document.getElementById('blame_trace').style.display='block'; return false;">Show blamed files</a>
-  <pre id="blame_trace" style="display:none"><code><%=h @exception.describe_blame %></code></pre>
+<% unless @exception.blamed_files.blank? %>
+  <% if (hide = @exception.blamed_files.length > 8) %>
+    <a href="#" onclick="document.getElementById('blame_trace').style.display='block'; return false;">Show blamed files</a>
+  <% end %>
+  <pre id="blame_trace" <%='style="display:none"' if hide %>><code><%=h @exception.describe_blame %></code></pre>
 <% end %>
 
 <% if defined?(Breakpoint) %>
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 4aaab2d3a5..28c718c54c 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -21,7 +21,7 @@ module ActionView
       # Example:
       #   link_to "Delete this page", { :action => "destroy", :id => @page.id }, :confirm => "Are you sure?"
       def link_to(name, options = {}, html_options = nil, *parameters_for_method_reference)
-        html_options = (html_options || {}).stringify_keys
+        html_options = (html_options || {}).symbolize_keys
         convert_confirm_option_to_javascript!(html_options)
         if options.is_a?(String)
           content_tag "a", name || options, (html_options || {}).merge("href" => options)
diff --git a/actionwebservice/lib/action_web_service/container/action_controller_container.rb b/actionwebservice/lib/action_web_service/container/action_controller_container.rb
index 4dda93ec11..12a566ef8e 100644
--- a/actionwebservice/lib/action_web_service/container/action_controller_container.rb
+++ b/actionwebservice/lib/action_web_service/container/action_controller_container.rb
@@ -64,7 +64,8 @@ module ActionWebService # :nodoc:
               require_dependency(file_name)
             rescue LoadError => load_error
               requiree = / -- (.*?)(\.rb)?$/.match(load_error).to_a[1]
-              raise LoadError, requiree == file_name ? "Missing API definition file in apis/#{file_name}.rb" : "Can't load file: #{requiree}"
+              msg = requiree == file_name ? "Missing API definition file in apis/#{file_name}.rb" : "Can't load file: #{requiree}"
+              raise LoadError.new(msg).copy_blame!(load_error)
             end
             klass = nil
             class_names.each do |name|
@@ -83,8 +84,10 @@ module ActionWebService # :nodoc:
         private
           def inherited(child)
             inherited_without_api(child)
-            child.web_service_api(child.controller_path)
-          rescue Exception => e
+            begin child.web_service_api(child.controller_path)
+            rescue MissingSourceFile => e
+              raise unless e.path == "apis/#{child.controller_path}_api.rb"
+            end
           end
       end
     end
diff --git a/activesupport/lib/active_support/core_ext/object_and_class.rb b/activesupport/lib/active_support/core_ext/object_and_class.rb
index eb54f79534..be486b0816 100644
--- a/activesupport/lib/active_support/core_ext/object_and_class.rb
+++ b/activesupport/lib/active_support/core_ext/object_and_class.rb
@@ -21,6 +21,13 @@ class Object #:nodoc:
       !self
     end
   end
+    
+  def suppress(*exception_classes)
+    begin yield
+    rescue Exception => e
+      raise unless exception_classes.any? {|cls| e.kind_of? cls}
+    end
+  end
 end
 
 class Class #:nodoc:
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 9518dfab90..a189da189a 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -21,8 +21,6 @@ module Dependencies
         require_or_load(file_name)
       rescue LoadError
         raise unless swallow_load_errors
-      rescue Object => e
-        raise ScriptError, "#{e.message}"
       end
     end
   end
@@ -179,8 +177,8 @@ class Object #:nodoc:
       begin
         require_or_load(class_id.to_s.demodulize.underscore)
         if Object.const_defined?(class_id) then return Object.const_get(class_id) else raise LoadError end
-      rescue LoadError
-        raise NameError, "uninitialized constant #{class_id}"
+      rescue LoadError => e
+        raise NameError.new("uninitialized constant #{class_id}").copy_blame!(e)
       end
     end
   end
@@ -216,4 +214,9 @@ class Exception
     return nil if blamed_files.empty?
     "This error occured while loading the following files:\n   #{blamed_files.join "\n   "}"
   end
+
+  def copy_blame!(exc)
+    @blamed_files = exc.blamed_files.clone
+    self
+  end
 end
diff --git a/activesupport/test/core_ext/object_and_class_ext_test.rb b/activesupport/test/core_ext/object_and_class_ext_test.rb
index 71eeb4bdb8..a0a6243480 100644
--- a/activesupport/test/core_ext/object_and_class_ext_test.rb
+++ b/activesupport/test/core_ext/object_and_class_ext_test.rb
@@ -19,3 +19,15 @@ class ClassExtTest < Test::Unit::TestCase
     assert !defined?(ClassD)
   end
 end
+
+class ObjectTests < Test::Unit::TestCase
+  def test_suppress_re_raises
+    assert_raises(LoadError) { suppress(ArgumentError) {raise LoadError} }
+  end
+  def test_suppress_supresses
+    suppress(ArgumentError) { raise ArgumentError }
+    suppress(LoadError) { raise LoadError }
+    suppress(LoadError, ArgumentError) { raise LoadError }
+    suppress(LoadError, ArgumentError) { raise ArgumentError }
+  end
+end
\ No newline at end of file