zio / zio-http

A next-generation Scala framework for building scalable, correct, and efficient HTTP clients and servers
https://zio.dev/zio-http
Apache License 2.0
787 stars 396 forks source link

Http Server keeps application alive when it should exit #2961

Closed Zerkath closed 2 months ago

Zerkath commented 2 months ago

Creating a ZIO http server layer and using it will prevent the application from shutting down once all other effects are completed.

Using RC9 the following code will not exit with code 0, instead remaining running indefinitely. This behavior does not happen in RC6-8. I have not tested other versions.

import zio.*
import zio.http.*

object ExampleApp extends ZIOAppDefault {
  def run = (for {
    _ <- ZIO.service[Server]
    exitCode = ExitCode(0)
  } yield (exitCode))
    .provide(
      Server.defaultWithPort(8080)
    )
}

Running the application with either sbt run or sbt reStart produces the same result. Exiting the application with interrupt works with sbt run, reStart forks the process and fails to kill it.

kyri-petrou commented 2 months ago

Hi @Zerkath, I believe this is related to how ZIO runs applications via sbt. Can you try adding the following to the module config in build.sbt and let me know if the issue still exists?

run / fork := true
Zerkath commented 2 months ago

Forked run has the same issue. It will keep running in the background. I tested a few things, installing routes resolves the issue. I went through changes between RC8 and RC9. Where I found this PR: https://github.com/zio/zio-http/pull/2872 https://github.com/zio/zio-http/pull/2872#discussion_r1617822383

serverStarted will block indefinitely until install is called for the first time. Should we add a comment that mentions this?

Which explains my issue, it is a rare case where server is created without routes.

kyri-petrou commented 2 months ago

@Zerkath OK this sounds like a legit bug then, thanks for identifying the source of the issue. If I may ask, what is your use-case for creating but not installing a server? I have a few potential fixes in mind but just want to make sure that we cover all the possible usecases

kyri-petrou commented 2 months ago

@Zerkath the issue was a lot easier to fix by the looks of it (see linked PR). Added a test for this and also tested it locally and the server now shutdowns as expected.

PS, here's a shorter reproducer of the original issue:

import zio._
import zio.http._

object MyApp extends ZIOAppDefault {
  override val run = ZIO.unit.provide(Server.defaultWithPort(8080).unit)
}
Zerkath commented 2 months ago

I was implementing some server and noticed my tooling failed to restart the application. So in real world, no use-case where I would have a server without routes. Issue is mainly with developer experience.

Thanks for quick resolution!

kyri-petrou commented 2 months ago

@987Nabil it seems that this issue manifests in some really dangerous ways afterall (see https://github.com/ghostdogpr/caliban/issues/2342). As it turns out, it also prevents applications exiting when there is an error during the application bootstrapping. This can be quite bad for server applications as if bootstrapping fails prior to installing the server, the application hangs until its killed by the host. Errors during application bootstrapping are uncommon in production but do happen from time to time.

Do you think we can release a patch now that the issue is fixed?

petervecera-jr commented 2 months ago

Hi, I've encountered the same problem today, I see the PR is already merged but there is no official release yet. Patch release would be highly appreciated.