diff options
author | schneems <richard.schneeman@gmail.com> | 2012-08-01 15:33:15 -0500 |
---|---|---|
committer | schneems <richard.schneeman@gmail.com> | 2012-08-28 08:53:45 -0700 |
commit | 0b6175ac2df96ebfca1baac89c20deaa13e61142 (patch) | |
tree | 60ede308f82037c713bc516100a60798272081c5 /actionpack/test/dispatch | |
parent | db8ff15a497191a952e47ed8ba761f6c2cf4a1f3 (diff) | |
download | rails-0b6175ac2df96ebfca1baac89c20deaa13e61142.tar.gz rails-0b6175ac2df96ebfca1baac89c20deaa13e61142.tar.bz2 rails-0b6175ac2df96ebfca1baac89c20deaa13e61142.zip |
Add Missing Keys from Journey on failed URL format
Many named routes have keys that are required to successfully resolve. If a key is left off like this:
<%= link_to 'user', user_path %>
This will produce an error like this:
No route matches {:action=>"show", :controller=>"users"}
Since we know that the :id is missing, we can add extra debugging information to the error message.
No route matches {:action=>"show", :controller=>"users"} missing required keys: [:id]
This will help new and seasoned developers look closer at their parameters. I've also subclassed the routing error to be clear that this error is a result of attempting to generate a url and not because the user is trying to visit a bad url.
While this may sound trivial this error message is misleading and confuses most developers. The important part isn't what's in the options its's what's missing. Adding this information to the error message will make debugging much more obvious.
This is the sister pull request of https://github.com/rails/journey/pull/44 which will be required to get they missing keys into the correct error message.
Example Development Error in Rails: http://cl.ly/image/3S0T0n1T3421
Diffstat (limited to 'actionpack/test/dispatch')
-rw-r--r-- | actionpack/test/dispatch/debug_exceptions_test.rb | 11 | ||||
-rw-r--r-- | actionpack/test/dispatch/routing_test.rb | 12 |
2 files changed, 17 insertions, 6 deletions
diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index 6ff651ad52..d236b14e02 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -37,6 +37,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest raise ActionView::Template::Error.new('template', AbstractController::ActionNotFound.new) when "/bad_request" raise ActionController::BadRequest + when "/missing_keys" + raise ActionController::UrlGenerationError, "No route matches" else raise "puke!" end @@ -113,6 +115,15 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest assert_match(/AbstractController::ActionNotFound/, body) end + test "named urls missing keys raise 500 level error" do + @app = DevelopmentApp + + get "/missing_keys", {}, {'action_dispatch.show_exceptions' => true} + assert_response 500 + + assert_match(/ActionController::UrlGenerationError/, body) + end + test "show the controller name in the diagnostics template when controller name is present" do @app = DevelopmentApp get("/runtime_error", {}, { diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b029131ad8..856248e2ac 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -1799,7 +1799,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/movies/00001' assert_equal 'Not Found', @response.body - assert_raises(ActionController::RoutingError){ movie_path(:id => '00001') } + assert_raises(ActionController::UrlGenerationError){ movie_path(:id => '00001') } get '/movies/0001/reviews' assert_equal 'reviews#index', @response.body @@ -1807,7 +1807,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/movies/00001/reviews' assert_equal 'Not Found', @response.body - assert_raises(ActionController::RoutingError){ movie_reviews_path(:movie_id => '00001') } + assert_raises(ActionController::UrlGenerationError){ movie_reviews_path(:movie_id => '00001') } get '/movies/0001/reviews/0001' assert_equal 'reviews#show', @response.body @@ -1815,7 +1815,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/movies/00001/reviews/0001' assert_equal 'Not Found', @response.body - assert_raises(ActionController::RoutingError){ movie_path(:movie_id => '00001', :id => '00001') } + assert_raises(ActionController::UrlGenerationError){ movie_path(:movie_id => '00001', :id => '00001') } get '/movies/0001/trailer' assert_equal 'trailers#show', @response.body @@ -1823,7 +1823,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/movies/00001/trailer' assert_equal 'Not Found', @response.body - assert_raises(ActionController::RoutingError){ movie_trailer_path(:movie_id => '00001') } + assert_raises(ActionController::UrlGenerationError){ movie_trailer_path(:movie_id => '00001') } end def test_only_should_be_read_from_scope @@ -2142,7 +2142,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get '/lists/2/todos/1' assert_equal 'Not Found', @response.body - assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') } + assert_raises(ActionController::UrlGenerationError){ list_todo_path(:list_id => '2', :id => '1') } end def test_named_routes_collision_is_avoided_unless_explicitly_given_as @@ -2625,7 +2625,7 @@ class TestNamedRouteUrlHelpers < ActionDispatch::IntegrationTest get "/categories/1" assert_response :success - assert_raises(ActionController::RoutingError) { product_path(nil) } + assert_raises(ActionController::UrlGenerationError) { product_path(nil) } end end |