Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 6.1 produces UrlGenerationError with some route helpers of engines that worked fine with Rails 6.0 #41038

Closed
vtamara opened this issue Jan 7, 2021 · 11 comments · Fixed by #41463
Assignees

Comments

@vtamara
Copy link

vtamara commented Jan 7, 2021

Steps to reproduce

CONFIG_HOSTS=www.example.com bin/rails test test/controllers/rutas_test.rb` 

Expected behavior

$ CONFIG_HOSTS=www.example.com bin/rails test test/controllers/rutas_test.rb 
Run options: --seed 53016

# Running:

/heb412/personas
.

Finished in 0.816051s, 1.2254 runs/s, 3.6762 assertions/s.
1 runs, 3 assertions, 0 failures, 0 errors, 0 skips

Passing test as it happens by using Rails 6.0 instead of Rails 6.1. To confirm:

Actual behavior

  • It will produce the error
Error:                                                            
RutasTest#test_personas_path_usable:       
ActionController::UrlGenerationError: No route matches {:action=>"index", :controller=>"sip/personas"}
    app/controllers/sip/personas_controller.rb:15:in `index'
    test/controllers/rutas_test.rb:17:in `block in <class:RutasTest>'
puts personas_path
  • However this exception is not generated if this statement is in another controller neither in the test. Even the test includes the assertion: assert_equal '/heb412/personas', personas_path that is passing.
  • The application includes 3 open source engines (sip, mr519_gen and heb412_gen see in the Gemfile) all of them hosted in github. The exception is not generated in the test application of the engine sip (i.e the one available at test/dummy) neither in the test application of the engine mr519_gen (which uses the engine sip). But it can be reproduced in the test application of the engine heb412_gen.
  • The route personas_path is defined in the motor sip. A workaround is to change personas_path with sip.personas_path. However that was not required with rails 6.0 (and anyway there are situations where this change cannot be made easily --like with will_paginate).
  • The controller app/controllers/sip/personas_controller.rb already has include Sip::Engine.routes.url_helpers

System configuration

Rails version: 6.1
Ruby version: 2.7.1

@rafaelfranca
Copy link
Member

Thank you for the issue. Since you know which version of Rails this work, and which version of Rails doesn't, can you use git bisect to find which commit changed the behavior you seeing?

@samuelgiles
Copy link

samuelgiles commented Jan 11, 2021

We're seeing the same behaviour under 6.1, its definitely something to do with engines (specifically we're using Administrate), we've yet to get to the bottom of it. The URL helpers available within views sometimes appear not to include the applications own URL helpers but only those of an engine being used.

We were able to work around this for now by doing include MyApplication.routes.url_helpers at the top of our ApplicationHelper.

Without looking through the changes yet I'd guess its something relating to this:

Bring back the feature that allows loading external route files from the router.
https://github.com/rails/rails/blob/6-1-stable/actionpack/CHANGELOG.md

Edit:
Some more potentially useful info, within _routes (here) is set to an engine where we'd expect it to be the application. You can tell where the routes are coming from with _routes.instance_variable_get(:@draw_paths).

@FlickStuart
Copy link

Experiencing a similar issue to the original poster. Bisecting shows the tests passing at commit 4500ebba21836c4bf264a043b68cf580cfa55611 and failing at the subsequent commit 37e778b6b44ae087ce052ff380b1e3aaee31473d - see: Cache routes.url_helpers built modules

Have tested this with the original poster's example repo and found the same pass/fail result before/at that commit.

@FlickStuart
Copy link

Hi @rafaelfranca is there anything else I can usefully add apart from the bisected commit hash?

@vtamara
Copy link
Author

vtamara commented Feb 15, 2021

Experiencing a similar issue to the original poster. Bisecting shows the tests passing at commit 4500ebba21836c4bf264a043b68cf580cfa55611 and failing at the subsequent commit 37e778b6b44ae087ce052ff380b1e3aaee31473d - see: Cache routes.url_helpers built modules

Have tested this with the original poster's example repo and found the same pass/fail result before/at that commit.

I confirm the result of this bisection of @FlickStuart

For me, using include MyApplication.routes.url_helpers, as @samuelgiles suggested, didn't work.

@vtamara
Copy link
Author

vtamara commented Feb 16, 2021

I could create a much smaller and simple case that shows the problem.

  1. Create a mountable engine e1
  2. Create a second mountable engine e2
  3. Create an application a3
  4. In e1 create a controller people_controller
  5. In e2 include e1 in the Gemfile and change e2/app/controllers/e2/application_controller.rb to make it descendant of E1::ApplicationController
  6. In a3 include e1 and e2 in the Gemfile and change config/routes.rb to mount both engines
  7. In a3 implement E1::PeopleController to become descendant of E2::ApplicationController and in its method index just add puts people_path.
  8. In a3 create the test test/controllers/routes_test.rb with:
require "test_helper"                                                            
                                                                                 
class RoutesTest < ActionDispatch::IntegrationTest                               
  include E1::Engine.routes.url_helpers                                          
                                                                                 
  test "use people_path" do·                                                     
    assert_equal '/people', people_path                                          
    get people_path                                                              
    assert_response :success                                                     
  end                                                                            
                                                                                 
end       

and run it to with rails 6.1 to produce the error:

Error:
RoutesTest#test_use_people_path:
DRb::DRbRemoteError: No route matches {:action=>"index", :controller=>"e1/people"} (ActionController::UrlGenerationError)
    app/controllers/e1/people_controller.rb:6:in `index'
    test/controllers/routes_test.rb:8:in `block in <class:RoutesTest>'

The following script recreates the situation when using rails 6.1.

genprob61.sh.txt

If you change e1/e1.gemspec, e2/e2.gemspec, a3/Gemfile and a3/config/application.rb to use rails 6.0, no error will be produced as the following script does:

fixreturning60.sh.txt

I wonder how to recreate this case in a test case (?)

@vtamara
Copy link
Author

vtamara commented Feb 16, 2021

That example also passes with 4500ebb and fails with the subsequent 37e778b

IMHO it would be good to introduce a configuration variable, like config.action_dispatch.cache_url_helpers_built_modules, that would activate the caching of 37e778b only when it would be true.

@vtamara
Copy link
Author

vtamara commented Feb 16, 2021

I guess the same problem happens in the main branch. Later I could create another PR for that branch.

@FlickStuart
Copy link

I can confirm the issue we saw is fixed by the PR #41463 - cheers @jhawthorn 👍🏻

@vtamara
Copy link
Author

vtamara commented Feb 17, 2021

Confirming that the issue is solved by using the branch main, that includes the solution of @jhawthorn.
Thank you very much.

vtamara added a commit to pasosdeJesus/sip that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/cor1440_gen that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/heb412_gen that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/sal7711_gen that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/sivel2_gen that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/sivel2_sjr that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/sivel2_sjrcol that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/jn316_gen that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/cor1440 that referenced this issue Feb 17, 2021
vtamara added a commit to pasosdeJesus/sal7711_web that referenced this issue Feb 17, 2021
@rubydesign
Copy link

rubydesign commented Dec 20, 2022

I'm on 7.0.4 and i seem to have this issue (or a similar one?)
The main app routes are not found when the engine controller renders the application layout.
Tried deriving the engine Controller from ::ApplicationController, or just setting layout 'application'
Can't put main_app in the apps layout (obviously).

Just saying, because the include tip @samuelgiles gave (include MyApplication.routes.url_helpers at the top of ApplicationHelper.) does the trick, or so i though
(edit) The above helper stopped exceptions, but the routes generated were scoped to the engine.
The only fix was to call the url helpers on the above routes.url_helper
So pretty sure no that this is a different bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants