Merge pull request #379 from http4s/0.24.0-RC2-to-0.25

Author: rossabaker

.github/workflows/ci.yml

@@ -1,4 +1,4 @@
-version = 3.6.1
+version = 3.9.4
 
 style = default
 

.scalafmt.conf

@@ -10,36 +10,36 @@ ThisBuild / developers := List(
 // publish website from this branch
 ThisBuild / tlSitePublishBranch := Some("main")
 
-val Scala213 = "2.13.10"
-ThisBuild / crossScalaVersions := Seq("2.12.17", Scala213, "3.2.1")
+val Scala213 = "2.13.16"
+ThisBuild / crossScalaVersions := Seq("2.12.20", Scala213, "3.3.5")
 ThisBuild / scalaVersion := Scala213 // the default Scala
 
 // Jetty 10+, for testing, requires Java 11.
 ThisBuild / githubWorkflowJavaVersions -= JavaSpec.temurin("8")
 ThisBuild / tlJdkRelease := Some(8)
+ThisBuild / startYear := Some(2013)
 
 lazy val root = tlCrossRootProject.aggregate(servlet, examples)
 
 val asyncHttpClientVersion = "2.12.3"
 val jettyVersion = "11.0.15"
-val http4sVersion = "0.23.17"
-val munitCatsEffectVersion = "1.0.7"
+val http4sVersion = "0.23.30"
+val munitCatsEffectVersion = "2.1.0"
 val servletApiVersion = "5.0.0"
 
 lazy val servlet = project
   .in(file("servlet"))
   .settings(
     name := "http4s-servlet",
     description := "Portable servlet implementation for http4s servers",
-    startYear := Some(2013),
     libraryDependencies ++= Seq(
       "jakarta.servlet" % "jakarta.servlet-api" % servletApiVersion % Provided,
       "org.eclipse.jetty" % "jetty-client" % jettyVersion % Test,
       "org.eclipse.jetty" % "jetty-server" % jettyVersion % Test,
       "org.eclipse.jetty" % "jetty-servlet" % jettyVersion % Test,
       "org.http4s" %% "http4s-dsl" % http4sVersion % Test,
       "org.http4s" %% "http4s-server" % http4sVersion,
-      "org.typelevel" %% "munit-cats-effect-3" % munitCatsEffectVersion % Test,
+      "org.typelevel" %% "munit-cats-effect" % munitCatsEffectVersion % Test,
     ),
   )
 
@@ -59,4 +59,4 @@ lazy val examples = project
   )
   .dependsOn(servlet)
 
-lazy val docs = project.in(file("site")).enablePlugins(TypelevelSitePlugin)
+lazy val docs = project.in(file("site")).enablePlugins(Http4sOrgSitePlugin)

build.sbt

@@ -1 +1 @@
-sbt.version=1.8.1
+sbt.version=1.10.11

flake.lock

@@ -1,2 +1,2 @@
-addSbtPlugin("com.earldouglas" % "xsbt-web-plugin" % "4.2.4")
-addSbtPlugin("org.http4s" % "sbt-http4s-org" % "0.14.9")
+addSbtPlugin("com.earldouglas" % "xsbt-web-plugin" % "4.2.5")
+addSbtPlugin("org.http4s" % "sbt-http4s-org" % "0.18.0")

project/build.properties

@@ -29,7 +29,7 @@ import org.http4s.server._
 import scala.annotation.nowarn
 import scala.concurrent.duration.Duration
 
-class AsyncHttp4sServlet[F[_]] @deprecated("Use AsyncHttp4sServlet.builder", "0.23.17") (
+class AsyncHttp4sServlet[F[_]] @deprecated("Use AsyncHttp4sServlet.builder", "0.23.18") (
     httpApp: HttpApp[F],
     asyncTimeout: Duration = Duration.Inf,
     servletIo: ServletIo[F],
@@ -62,20 +62,18 @@ class AsyncHttp4sServlet[F[_]] @deprecated("Use AsyncHttp4sServlet.builder", "0.
       ctx.setTimeout(asyncTimeoutMillis)
       // Must be done on the container thread for Tomcat's sake when using async I/O.
       val bodyWriter = servletIo.bodyWriter(servletResponse, dispatcher) _
-      val result = F
-        .attempt(
-          toRequest(servletRequest).fold(
-            onParseFailure(_, servletResponse, bodyWriter),
+      val result =
+        toRequest(servletRequest)
+          .fold(
+            onParseFailure(_, servletResponse),
             handleRequest(ctx, _, bodyWriter),
           )
-        )
-        .flatMap {
-          case Right(()) => F.delay(ctx.complete)
-          case Left(t) => errorHandler(servletRequest, servletResponse)(t)
-        }
+          .recoverWith(errorHandler(servletRequest, servletResponse))
       dispatcher.unsafeRunAndForget(result)
     } catch errorHandler(servletRequest, servletResponse).andThen(dispatcher.unsafeRunSync _)
 
+  private[this] val noopCancelToken = Some(F.unit)
+
   private def handleRequest(
       ctx: AsyncContext,
       request: Request[F],
@@ -85,17 +83,23 @@ class AsyncHttp4sServlet[F[_]] @deprecated("Use AsyncHttp4sServlet.builder", "0.
       // It is an error to add a listener to an async context that is
       // already completed, so we must take care to add the listener
       // before the response can complete.
-
       val timeout =
-        F.async[Response[F]](cb =>
-          gate.complete(ctx.addListener(new AsyncTimeoutHandler(cb))).as(Option.empty[F[Unit]])
+        F.async[Unit](cb =>
+          gate.complete(ctx.addListener(new AsyncTimeoutHandler(cb))).as(noopCancelToken)
         )
       val response =
         gate.get *>
           F.defer(serviceFn(request))
             .recoverWith(serviceErrorHandler(request))
-      val servletResponse = ctx.getResponse.asInstanceOf[HttpServletResponse]
-      F.race(timeout, response).flatMap(r => renderResponse(r.merge, servletResponse, bodyWriter))
+      F.race(timeout, response).flatMap {
+        case Left(_) =>
+          // In Jetty, if onTimeout is called, we need to complete on the
+          // listener's own thread.
+          F.unit
+        case Right(resp) =>
+          val servletResponse = ctx.getResponse.asInstanceOf[HttpServletResponse]
+          renderResponse(resp, servletResponse, bodyWriter) *> F.delay(ctx.complete())
+      }
     }
 
   private def errorHandler(
@@ -122,11 +126,19 @@ class AsyncHttp4sServlet[F[_]] @deprecated("Use AsyncHttp4sServlet.builder", "0.
         }
   }
 
-  private class AsyncTimeoutHandler(cb: Callback[Response[F]]) extends AbstractAsyncListener {
+  private class AsyncTimeoutHandler(cb: Callback[Unit]) extends AbstractAsyncListener {
     override def onTimeout(event: AsyncEvent): Unit = {
+      // In Jetty, we must complete on the same thread as the timeout
+      // handler.  This triggers a cancellation of the service so we
+      // can take over.
+      cb(Right(()))
+
+      val ctx = event.getAsyncContext
       val req = event.getAsyncContext.getRequest.asInstanceOf[HttpServletRequest]
       logger.info(s"Request timed out: ${req.getMethod} ${req.getServletPath}${req.getPathInfo}")
-      cb(Right(Response.timeout[F]))
+      val resp = event.getAsyncContext.getResponse.asInstanceOf[HttpServletResponse]
+      resp.sendError(Response.timeout.status.code, "Response timed out")
+      ctx.complete()
     }
   }
 }

project/plugins.sbt

@@ -24,6 +24,8 @@ import jakarta.servlet.http.HttpServletRequest
 import jakarta.servlet.http.HttpServletResponse
 import org.http4s.server._
 
+import scala.annotation.nowarn
+
 class BlockingHttp4sServlet[F[_]] private (
     service: HttpApp[F],
     servletIo: ServletIo[F],
@@ -32,6 +34,7 @@ class BlockingHttp4sServlet[F[_]] private (
 )(implicit F: Sync[F])
     extends Http4sServlet[F](service, servletIo, dispatcher) {
 
+  @deprecated("Use BlockingHttp4sServlet.builder", "0.24.0")
   def this(
       service: HttpApp[F],
       servletIo: BlockingServletIo[F],
@@ -49,7 +52,7 @@ class BlockingHttp4sServlet[F[_]] private (
         val bodyWriter = servletIo.bodyWriter(servletResponse, dispatcher) _
 
         val render = toRequest(servletRequest).fold(
-          onParseFailure(_, servletResponse, bodyWriter),
+          onParseFailure(_, servletResponse),
           handleRequest(_, servletResponse, bodyWriter),
         )
 
@@ -85,12 +88,55 @@ class BlockingHttp4sServlet[F[_]] private (
 }
 
 object BlockingHttp4sServlet {
-  def apply[F[_]: Sync](
+
+  class Builder[F[_]] private[BlockingHttp4sServlet] (
+      httpApp: HttpApp[F],
+      dispatcher: Dispatcher[F],
+      chunkSize: Option[Int],
+  ) {
+    private def copy(
+        httpApp: HttpApp[F] = httpApp,
+        dispatcher: Dispatcher[F] = dispatcher,
+        chunkSize: Option[Int] = chunkSize,
+    ): Builder[F] =
+      new Builder[F](
+        httpApp,
+        dispatcher,
+        chunkSize,
+      ) {}
+
+    @nowarn("cat=deprecation")
+    def build(implicit F: Sync[F]): BlockingHttp4sServlet[F] =
+      new BlockingHttp4sServlet(
+        httpApp,
+        BlockingServletIo(chunkSize.getOrElse(DefaultChunkSize)),
+        DefaultServiceErrorHandler,
+        dispatcher,
+      )
+
+    def withHttpApp(httpApp: HttpApp[F]): Builder[F] =
+      copy(httpApp = httpApp)
+
+    def withDispatcher(dispatcher: Dispatcher[F]): Builder[F] =
+      copy(dispatcher = dispatcher)
+
+    def withChunkSize(chunkSize: Int): Builder[F] =
+      copy(chunkSize = Some(chunkSize))
+  }
+
+  def builder[F[_]](httpApp: HttpApp[F], dispatcher: Dispatcher[F]): Builder[F] =
+    new Builder[F](httpApp, dispatcher, None) {}
+
+  @deprecated(
+    "Use `builder` instead",
+    "0.24.0",
+  )
+  def apply[F[_]](
       service: HttpApp[F],
       servletIo: ServletIo[F],
       dispatcher: Dispatcher[F],
-  ): BlockingHttp4sServlet[F] =
-    new BlockingHttp4sServlet[F](
+  )(implicit F: Sync[F]): BlockingHttp4sServlet[F] =
+    new BlockingHttp4sServlet(
       service,
       servletIo,
       DefaultServiceErrorHandler,

servlet/src/main/scala/org/http4s/servlet/AsyncHttp4sServlet.scala

@@ -68,14 +68,19 @@ abstract class Http4sServlet[F[_]](
     serverSoftware = ServerSoftware(servletContext.getServerInfo)
   }
 
+  @deprecated("Use the overload without bodyWriter.", "0.23.15")
   protected def onParseFailure(
       parseFailure: ParseFailure,
       servletResponse: HttpServletResponse,
       bodyWriter: BodyWriter[F],
-  ): F[Unit] = {
-    val response = Response[F](Status.BadRequest).withEntity(parseFailure.sanitized)
-    renderResponse(response, servletResponse, bodyWriter)
-  }
+  ): F[Unit] =
+    onParseFailure(parseFailure, servletResponse)
+
+  protected def onParseFailure(
+      parseFailure: ParseFailure,
+      servletResponse: HttpServletResponse,
+  ): F[Unit] =
+    F.delay(servletResponse.sendError(Status.BadRequest.code, parseFailure.sanitized))
 
   protected def renderResponse(
       response: Response[F],