heroku-buildpack-ruby icon indicating copy to clipboard operation
heroku-buildpack-ruby copied to clipboard

Consider adding a warning when PORT environment variable is missing at runtime

Open thephw opened this issue 5 years ago • 2 comments

Description

heroku-buildpack-ruby require that the runtime specify a PORT environment variable. Although consistent across buildpacks -- a recurring issue is building a rack image and forgetting to specify the port at runtime. For many buildpacks, the error is self evident.

When building a rack application with the ruby buildpack, the error is a little more cryptic:

bundler: failed to load command: rackup (/layers/heroku_ruby/gems/vendor/bundle/ruby/2.5.0/bin/rackup)
OptionParser::MissingArgument: missing argument: -p
  /layers/heroku_ruby/gems/vendor/bundle/ruby/2.5.0/gems/rack-2.0.7/lib/rack/server.rb:97:in `parse!'
  /layers/heroku_ruby/gems/vendor/bundle/ruby/2.5.0/gems/rack-2.0.7/lib/rack/server.rb:333:in `parse_options'
  /layers/heroku_ruby/gems/vendor/bundle/ruby/2.5.0/gems/rack-2.0.7/lib/rack/server.rb:195:in `initialize'

Suggestion

We could clarify the error message for all application without affecting compliance and integration with other language buildpacks.

if [[ -z "${PORT}" ]]; then
  >&2 echo "Runtime is missing environment variable PORT"
fi

Alternate Considerations

  • Could be resolved by specifying a default at buildtime
  • Could be resolved by specifying a default in the buildpack
  • Could be left as it currently operates

Code in question

https://github.com/heroku/heroku-buildpack-ruby/blob/22b24b66c42afdf8c95cc16143221ffd7b2d7f7b/lib/language_pack/rack.rb#L32

Reference

https://github.com/buildpacks/samples/issues/46

thephw avatar Dec 16 '19 17:12 thephw

Some historical context from the CNCF working group minutes:

2019.01.15 PORT

  • Assume for now: set in launch, not in build
  • For now, pack build can set on images defaulting to 8080
  • pack run can pick a port that’s the same for container and host

thephw avatar Dec 18 '19 00:12 thephw

@schneems You have any thoughts/feelings about this?

thephw avatar Jan 23 '20 19:01 thephw