validate port?

[kennedymwavu]

explanation

should we do checks on the provided port? most of the times during deployment, the port number is fetched from an env file. however, if the env var is not set (mostly just forgetting), Sys.getenv() returns an empty string. if passed to ambiorix, it is coerced into NA by as.integer(). this has bitten me a few times.

reprex

library(ambiorix)

home_get <- \(req, res) {
  res$send("Ambiorix")
}

Ambiorix$
  new(port = "")$
  get("/", home_get)$
  start(open = TRUE)

when you run the above, you get this on the console:

✔ 09-10-2024 21:43:46 Listening on http://0.0.0.0:NA

question

should we validate that the given port is indeed an integer and not NA_integer_? throw error maybe?

hint

this should suffice:

port <- as.integer(given_port)
if (identical(port, NA_integer_)) {
  stop(sprintf("Invalid port: '%s'. Port must be a valid integer.", given_port))
}

this change can be made just before passing the port to httpuv::startServer(), here

[JohnCoene]

You're correct that better checks should be passed but we should also take that opportunity to rethink this a bit, namely in light of your discovery that it can be deployed on shiny server.

Perhaps the port argument could default to a convenience function that resolved the port somewhat intelligently, e.g.: port = get_port() which:

1. Check if environment variable is declared, if so uses that
2. Check if Ambiorix port is set, if so..
3. Check shiny port option, ... 4.return random port

What do you think?

[kennedymwavu]

that'd be great! AMBIORIX_PORT would be a good name for the env var, no?