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

Info about running endpoints wrong #3149

Closed
benperiton opened this issue Nov 8, 2018 · 12 comments
Closed

Info about running endpoints wrong #3149

benperiton opened this issue Nov 8, 2018 · 12 comments

Comments

@benperiton
Copy link

Environment

  • Elixir version (elixir -v): Elixir 1.7.3 (compiled with Erlang/OTP 19)
  • Phoenix version (mix deps): 1.4.0 (phoenix) 56fe9a80
  • NodeJS version (node -v): v10.3.0
  • NPM version (npm -v): 6.4.1
  • Operating system: OSX 10.11.6

Expected behavior

Using this config:

config :web, Web.Endpoint,
  http: [
    port: 3999
  ],
  https: [
    port: 4000,
    cipher_suite: :strong,
    keyfile: "priv/cert/selfsigned_key.pem",
    certfile: "priv/cert/selfsigned.pem"
  ],
  debug_errors: true,
  code_reloader: true,
  check_origin: false,
  watchers: [
    node: [
      "node_modules/webpack/bin/webpack.js",
      "--mode",
      "development",
      "--watch-stdin",
      cd: Path.expand("../assets", __DIR__)
    ]
  ]

It should show the correct ports for the endpoints that are running:

Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at http://localhost:3999
[info] Running Web.Endpoint with cowboy 2.5.0 at https://localhost:4000

Actual behavior

Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at https://localhost:4000
[info] Running Web.Endpoint with cowboy 2.5.0 at https://localhost:4000
@benperiton
Copy link
Author

Looks like it might have been introduced in #3105 ?

@sionide21
Copy link
Contributor

What would the expected behavior be if the config looked like the following?

config :web, Web.Endpoint,
  http: [
    port: 3999
  ],
  https: [
    port: 4000,
    cipher_suite: :strong,
    keyfile: "priv/cert/selfsigned_key.pem",
    certfile: "priv/cert/selfsigned.pem"
  ],
  url: [scheme: "https", host: "server.local", port: 4040],
  debug_errors: true,
  code_reloader: true,
  check_origin: false,
  watchers: [
    node: [
      "node_modules/webpack/bin/webpack.js",
      "--mode",
      "development",
      "--watch-stdin",
      cd: Path.expand("../assets", __DIR__)
    ]
  ]

It seems like it could go a few different ways:

  • We could exclusively use the configured URL:
Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4040
  • We could list all three:
Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at http://localhost:3999
[info] Running Web.Endpoint with cowboy 2.5.0 at https://localhost:4000
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4040
  • We could replace one (I don't know which would be the correct one to replace though):
Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at http://localhost:3999
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4040

Or

Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4040
[info] Running Web.Endpoint with cowboy 2.5.0 at https://localhost:4000

@benperiton
Copy link
Author

benperiton commented Nov 8, 2018

That is what is addressed in #3105 I believe, so the output should be (in my view)

Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 at http://server.local:3999
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4000

@chrismccord
Copy link
Member

Ah I see the issue now. Let's go with the "replace one" approach, where the configured URL scheme wins, so:

[info] Running Web.Endpoint with cowboy 2.5.0 at http://localhost:3999
[info] Running Web.Endpoint with cowboy 2.5.0 at https://server.local:4040

from your config. Thanks!

@sionide21
Copy link
Contributor

I have a PR to make it match @chrismccord's example. #3152

I'm not sure if this is the right way to go though, it seems a bit confusing not to know the port you're actually bound too.

I wonder if it would be better to always use the hostname from url and always show the port we actually bound to. That would match what @benperiton has in their example.

@rmoorman
Copy link
Contributor

rmoorman commented Nov 9, 2018

Wouldn't it be better to explicitly state the configured url next to the endpoint's bound ip/port? Maybe for example showing https://localhost:4000 by default and in case a url is defined add (url set to https://server.local:4040) or something like that. This could imo make more sense to the folks reading the logs.

@sionide21
Copy link
Contributor

This PR would match @benperiton's example above. #3158

@sionide21
Copy link
Contributor

Wouldn't it be better to explicitly state the configured url next to the endpoint's bound ip/port? Maybe for example showing https://localhost:4000 by default and in case a url is defined add (url set to https://server.local:4040) or something like that. This could imo make more sense to the folks reading the logs.

Maybe the configured URL should be displayed completely separate from the listeners and we should just display what's bound here. For example:

Generated web app
[info] Running Web.Endpoint with cowboy 2.5.0 on 0.0.0.0:3999 (http)
[info] Running Web.Endpoint with cowboy 2.5.0 on 0.0.0.0:4000 (https)
[info] Serving Web.Endpoint at https://server.local:4040

@rmoorman
Copy link
Contributor

rmoorman commented Nov 12, 2018

@sionide21 that would be even clearer indeed IMO 👍 (I really like that the information needed to grasp what is going on regarding listening and url configuration is made visible)

@benperiton
Copy link
Author

@sionide21 Yea, that looks pretty good - all the info is right there

@chrismccord
Copy link
Member

@sionide21 that is a great idea! I would only s/Serving/Access and we are good to go!

@sionide21
Copy link
Contributor

@chrismccord How does #3160 look?

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

No branches or pull requests

4 participants