latest build of Docker image fails to start

[eschild]

I was using ottomated/crewlink-server:latest and it was working fine. I did a docker pull and got a new image and it would crash on start. I attempted to build my own image as documented but that image also failed on start. After some troubleshooting I found that adding ENV ADDRESS=9736 to the Dockerfile just before the EXPOSE command fixed the problem

[nathanparekh]

Trying to deploy to Heroku from the master branch is also not working, also having to do with the address environment variable not being set. Can be fairly easily fixed by setting a config var directly in the settings for the Heroku app, but it's annoying and breaks the 1-click button.

Heroku output:

2020-12-27T05:42:59.492478+00:00 heroku[web.1]: Starting process with command `node dist/index.js`
2020-12-27T05:43:02.262304+00:00 app[web.1]: 2020-12-27T05:43:02+0000 <error> You must set the ADDRESS environment variable.
2020-12-27T05:43:02.329948+00:00 heroku[web.1]: Process exited with status 1
2020-12-27T05:43:02.383760+00:00 heroku[web.1]: State changed from starting to crashed

[jgroffen]

Can confirm same issue when deploying to Heroku. A workaround is, in the Heroku deployed CrewLink Server:

• Go to the settings tab
• Press 'Reveal Config Vars' button
• Add a 'Config Var' with Key=ADDRESS and value=yourserver.herokuapp.com (of course, change 'yourserver' to your heroku app name).

After doing this it seems to auto-restart. You can test it by browsing to the link in a browser as well, it'll report an error or tell you how many users are connected.

The solution is to calculate the ADDRESS dynamically from Heroku? I'm not familiar with Heroku, but I'm sure you could pull the app URL value and set the config var during deployment :)

[nathanparekh]

Looked around a bit... looks like the address environment variable is only used for display (like on the web display). This seems like it's the commit that "broke" this: https://github.com/ottomated/CrewLink-server/commit/274560a823b37f62f62bac918d2c2e85e9f9a32e, which looks like it's there to stop the server from inferring where it's located (which has some benefits - it used to show the IP instead of the full URL).

As for the Heroku fix, I or someone else could create a PR to make the 1-click-deploy button have a field to enter the URL on the interface and provide a default using app.json, but there's no way (as far as I can tell) for Heroku to provide the address automatically (the closest would be the Dyno Metadata lab, but that requires additional steps to turn on and only provides the ID, not the full URL).

I don't know enough about Docker to figure out a nice way to do something similar, other than maybe updating instructions on Docker usage or providing a default value in the application itself so that the address value isn't required or even just removing it entirely. That would be easier for Heroku too, for what it's worth, but I'm not sure how feasible that is.

[jgroffen]

Hello @nathanparekh - I just checked too and agree, looks like it's only used to render the HTML or JSON queries to the web service. The HTTP response could check the host header to detect the URL, but that's not exactly reverse-proxy friendly.

It may be easy to do for Heroku deploys, but I think it's better if it's an optional field for Docker - so you can set the preferred, canonical URL if you want to, and the server will advertise that URL in HTML and JSon responses, but still work with a default, derived value otherwise.

So, just change the error to an info message in the log output 'ADDRESS not set, a default (possibly ugly / incorrect) address will be determined instead. Set ADDRESS if you want this server to advertise a canonical or reverse-proxied URL.'

[joshuaho96]

I can confirm this causes issues with Azure deployments without FQDN, the IP is dynamically assigned and cannot be known prior to the deployment which causes failures.

[nathanparekh]

@jgroffen - maybe the best option would be to fall back to the old behavior (using public-ip from npm) if ADDRESS isn't provided?

Along with a log warning or something?

[jgroffen]

@nathanparekh Agreed - that is what I'm proposing - and as you say, add the info log output when it is defaulted.

[nathanparekh]

I'm super new at this but I'm starting to look at implementing something like that and I'm already realizing one place this could be really confusing... Heroku provides a PORT environment variable that applications are supposed to bind their HTTP servers to, and it seems like they serve as a reverse proxy... so CrewLink reads the PORT value from Heroku as the port and the old method would display that (ie. http://3.83.122.41:44390).

So not only do you have an ephemeral IP address, you end up with a plain wrong port, and the old method also ignored whether or not HTTPS was used (resulting in HTTP instead of HTTPS on Heroku).

Someone please correct me if I'm wrong, but I don't believe that anything can really be done about these other than Heroku-specific code, which starts to get messy quick and is probably a bad idea. Given all that and that people using the Docker deployment will likely be fairly technically savvy, I'm thinking maybe it'd just be better to not show a URL at all when ADDRESS isn't set. People using Heroku would be able to just copy-and-paste the webpage URL (which should be intuitive), and for people using Docker (which is more involved anyways) if it were just a little more clear in the instructions they should be able to figure it out without it being fed to them.

Alternatively, we could just go back to using the Heroku form fields on the 1-click button and update the Docker instructions to tell people to use -e to specify ADDRESS and hope nobody accidentally specifies an ADDRESS that isn't what it actually is.

@jgroffen, what do you think?

[jgroffen]

Hello @nathanparekh

Heroku provides a PORT environment variable that applications are supposed to bind their HTTP servers to, and it seems like they serve as a reverse proxy

Yeah - that is exactly why the setting is a good one because it's common for the host have an internal name / port that differs from the external name / port. Also HTTPS is getting terminated at the boundary by the looks of it too in Heroku.

Someone please correct me if I'm wrong, but I don't believe that anything can really be done about these other than Heroku-specific code

You are right, if the externally resolvable address doesn't match the derived values, there isn't any reliable way to know what that name is. The best I can think of would be to change the address advertised to reflect the one used by the client - using HTTP headers like Host or X-Forwarded-Host and X-Forwarded-Port (and X-Forwarded-Proto !). These aren't exactly super reliable either though - it's a pseudo-standard and not all rev. proxies will set it (or intermediary proxies may confound the problem if not configured properly).

Alternatively, we could just go back to using the Heroku form fields on the 1-click button and update the Docker instructions to tell people to use -e to specify ADDRESS and hope nobody accidentally specifies an ADDRESS that isn't what it actually is.

The setting is very valid for Docker. I agree it's kind of pointless to show the URL when it hasn't been explicitly set - why tell people how to get to the site they just got to unless there is a preferred URL to use?

Agreed we could update the Docker instructions to tell people the ADDRESS option is available if they are behind a proxy and want to advertise a different URL. The wording of the docco doesn't have to be too strong - If they set it wrong it's not that impactful - they should work out it's wrong pretty quickly and it's a common setting in webservers.

It would be very ideal if Heroku can set the environment variable correctly - it knows what the external name is. I'll look into it ...