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

Mounting on a subdomain is broken #951

Open
checkbutton opened this Issue Aug 17, 2018 · 6 comments

Comments

Projects
None yet
4 participants
@checkbutton
Copy link

checkbutton commented Aug 17, 2018

Specifying host: when mounting applications in Hanami seems to be broken.
I have the following setup:

Hanami.configure do
  mount TestOne::Application, at: '/', host: 'test-one'
  mount TestTwo::Application, at: '/', host: 'test-two'
# ...
end

In both applications, I define the following route:

get '/hello', to: ->(env) { [200, {}, ['Hello from Test One/Two!']] }

When I run my application with hanami server --host=lvh.me, as described in http://hanamirb.org/guides/1.2/routing/overview/#mounting-on-a-subdomain and try to call test-two.lvh.me/hello, I end up in TestOne.

Moreover, if I stick to the documentation and leave out the at: in mount, I end up getting

key not found: :at
/Users/user/.rvm/gems/ruby-2.5.1/gems/hanami-1.2.0/lib/hanami/configuration.rb:35:in `fetch'

Setting at: to an empty String, it does not build any route (only getting 404s).

@checkbutton checkbutton changed the title Mounting on a subdomain is not working Mounting on a subdomain is broken Aug 17, 2018

@cllns

This comment has been minimized.

Copy link
Member

cllns commented Aug 17, 2018

Sorry you're having trouble with this! I think the documentation might be wrong. Can you try mount TestOne::Application, at: '/', host: 'test-one.lvh.me'? I know this isn't idea, since the root host will chance per environment, but I seem to remember it needed the full host name, not just the subdomain.

@checkbutton

This comment has been minimized.

Copy link

checkbutton commented Aug 18, 2018

@cllns thanks for your answer. I've just tried that out, but without any effect. I've also tried to set the host in the respective TestOne::Application / TestTwo::Application configure block, but this as well had no effect.

@checkbutton

This comment has been minimized.

Copy link

checkbutton commented Aug 22, 2018

Additional info: I tried it also with different web servers, i.e. WEBrick and Puma

@greggilbert

This comment has been minimized.

Copy link

greggilbert commented Aug 24, 2018

I'm also having this issue.

Per the docs, one notation for this is to use .new, as in mount Blog.new, host: 'blog'. When you do that, though, it throws this:

NoMethodError: undefined method `controller_pattern' for nil:NilClass

/usr/local/bundle/gems/hanami-1.2.0/lib/hanami/rendering_policy.rb:32:in `initialize'
/usr/local/bundle/gems/hanami-1.2.0/lib/hanami/application.rb:152:in `new'
/usr/local/bundle/gems/hanami-1.2.0/lib/hanami/application.rb:152:in `initialize'
/usr/src/app/config/environment.rb:10:in `new'

I've also noticed that order is significant here, with the first mount line winning out:

Hanami.configure do
  mount Web::Application, at: '/'    # <-- the web routes win, and Admin is inaccessible
  mount Admin::Application, at: '/'

Setting the host in Admin::Application.configure also seems to have no effect.

I'm going to keep digging, but this seems like a big issue, especially since hanami/router appears to be able to handle it.

Edit: Might be a false positive, but it looks like :host isn't being set in this hanami/routing block: https://github.com/hanami/router/blob/master/lib/hanami/routing/http_router.rb#L145-L149

@AlfonsoUceda

This comment has been minimized.

Copy link
Member

AlfonsoUceda commented Dec 16, 2018

Hi Folks, sorry for the problem but yes, it is a bug.

I've reproduced the bug and I know how to fix it, so I'll create a PR, after that can you test it in your apps? thanks!

@AlfonsoUceda AlfonsoUceda added the bug label Dec 16, 2018

@AlfonsoUceda AlfonsoUceda self-assigned this Dec 16, 2018

@AlfonsoUceda AlfonsoUceda added this to the v1.3.1 milestone Dec 16, 2018

@AlfonsoUceda AlfonsoUceda referenced a pull request that will close this issue Dec 16, 2018

Open

Fix :host mount option #976

@AlfonsoUceda

This comment has been minimized.

Copy link
Member

AlfonsoUceda commented Dec 16, 2018

@checkbutton @greggilbert I've created a quick PR that it needs to be improved. #976

Can you test with your app? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment