From 42d93f5b89e694c406b071fe95cc2320ea30519f Mon Sep 17 00:00:00 2001 From: fan-tom Date: Sun, 21 Jan 2024 19:54:29 +0400 Subject: [PATCH 1/4] Don't print long ZIO traces when server cannot start due to port binding error --- .../forsyte/apalache/shai/v1/RpcServer.scala | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/shai/src/main/scala/at/forsyte/apalache/shai/v1/RpcServer.scala b/shai/src/main/scala/at/forsyte/apalache/shai/v1/RpcServer.scala index 179bad094a..d213edfe73 100644 --- a/shai/src/main/scala/at/forsyte/apalache/shai/v1/RpcServer.scala +++ b/shai/src/main/scala/at/forsyte/apalache/shai/v1/RpcServer.scala @@ -1,6 +1,6 @@ package at.forsyte.apalache.shai.v1 -import zio.{console, Ref, ZEnv, ZIO} +import zio.{console, ExitCode, Ref, ZEnv, ZIO} import java.util.UUID import scalapb.zio_grpc.ServerMain @@ -8,6 +8,9 @@ import scalapb.zio_grpc.ServiceList import com.typesafe.scalalogging.LazyLogging import io.grpc.ServerBuilder +import java.io.IOException +import java.net.BindException + /** * The Shai RPC server handling gRPC requests to interact with the model checker * @@ -39,10 +42,24 @@ class RpcServer(override val port: Int) extends ServerMain with LazyLogging { // https://github.com/informalsystems/apalache/issues/2622 override def builder: (x0) forSome { type x0 <: ServerBuilder[x0] } = super.builder.maxInboundMessageSize(8 * 1024 * 1024) + + override def run(args: List[String]) = { + myAppLogic.catchAll { t => + t match { + // treat "post already in use" error specially + case e: IOException if e.getCause.isInstanceOf[BindException] => + logger.error(s"${RpcServer.STARTUP_ERROR_PROMPT} ${e.getMessage}: ${e.getCause.getMessage}") + case _ => + logger.error(RpcServer.STARTUP_ERROR_PROMPT, t) + } + ZIO.succeed(ExitCode.failure) + } + } } object RpcServer { val DEFAULT_PORT = 8822 + val STARTUP_ERROR_PROMPT = "Error while starting Apalache server:" def apply(port: Int = DEFAULT_PORT) = new RpcServer(port) } From 9fe802934d386a4831005671d0a21c26fb7c1395 Mon Sep 17 00:00:00 2001 From: fan-tom Date: Fri, 26 Jan 2024 02:46:05 +0400 Subject: [PATCH 2/4] Add integration test --- test/tla/cli-integration-tests.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/tla/cli-integration-tests.md b/test/tla/cli-integration-tests.md index 0695446c29..efdb340d4d 100644 --- a/test/tla/cli-integration-tests.md +++ b/test/tla/cli-integration-tests.md @@ -3978,6 +3978,31 @@ The Apalache server is running on port 8888. Press Ctrl-C to stop. ... ``` +### server mode: error is nicely reported when port is already in use + +Start socat in the background to occupy the port, save its pid and wait a second to let binding happen +```sh +$ socat TCP-L:8888,fork,reuseaddr - & echo $! > pid.pid && sleep 1 +``` + +Try to start the Apalache server on the occupied port, +redirect its output to the file, +save its exit code, +strip logger prompt +and exit with status code of the server +```sh +$ apalache-mc server --port=8888 1>out 2>out; ext=$? && cat out | sed -E 's/E@.*$//' && exit $ext +... +Error while starting Apalache server: Failed to bind to address 0.0.0.0/0.0.0.0:8888: Address already in use +[1] +``` + +Cleanup: kill socat and delete temporary files +```sh +$ cat pid.pid | xargs kill -9 +$ rm pid.pid out +``` + ## quint input ### quint input: quint spec can be checked From cbfc23d830872e3c13628d16fff655a492c3e28f Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Thu, 25 Jan 2024 20:33:16 -0500 Subject: [PATCH 3/4] Add changelog entry --- .unreleased/bug-fixes/2811-server-bind-error.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .unreleased/bug-fixes/2811-server-bind-error.md diff --git a/.unreleased/bug-fixes/2811-server-bind-error.md b/.unreleased/bug-fixes/2811-server-bind-error.md new file mode 100644 index 0000000000..da379caf93 --- /dev/null +++ b/.unreleased/bug-fixes/2811-server-bind-error.md @@ -0,0 +1,2 @@ +When expected server port is already bound, report clean user error instead of +crashing with a traceback (#2676). From 38d4aff11e00774d07fc7046b53b495ba1838f35 Mon Sep 17 00:00:00 2001 From: Shon Feder Date: Fri, 2 Feb 2024 11:15:12 -0500 Subject: [PATCH 4/4] Don't run the port binding tests We know this works thanks to local testing, and while it is a nice UX improvement, it doesn't effect essential functionality enough to complicate our CI configuration to account for it. --- test/tla/cli-integration-tests.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/tla/cli-integration-tests.md b/test/tla/cli-integration-tests.md index 892d57ab93..3237804fc2 100644 --- a/test/tla/cli-integration-tests.md +++ b/test/tla/cli-integration-tests.md @@ -4001,7 +4001,12 @@ The Apalache server is running on port 8888. Press Ctrl-C to stop. ### server mode: error is nicely reported when port is already in use +NOTE: These tests are skipped because the would complicate our CI process +more than is warranted by the functionality they test. To enable these tests, +remove the `$MDX skip` annotations. + Start socat in the background to occupy the port, save its pid and wait a second to let binding happen + ```sh $ socat TCP-L:8888,fork,reuseaddr - & echo $! > pid.pid && sleep 1 ``` @@ -4011,6 +4016,7 @@ redirect its output to the file, save its exit code, strip logger prompt and exit with status code of the server + ```sh $ apalache-mc server --port=8888 1>out 2>out; ext=$? && cat out | sed -E 's/E@.*$//' && exit $ext ... @@ -4019,6 +4025,7 @@ Error while starting Apalache server: Failed to bind to address 0.0.0.0/0.0.0.0: ``` Cleanup: kill socat and delete temporary files + ```sh $ cat pid.pid | xargs kill -9 $ rm pid.pid out