Explicitly return 408 Request Timeout when configured timeout is hit

Previously we would return 503 Service Unavailable which sometimes can make it unclear what the reason for failure was. As the server *is* allowing retry, therefore 408 is more explicit.
snowplow · Aug 21, 2024 · 109e9d0 · 109e9d0
1 parent 6693e7e
commit 109e9d0
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 17 deletions.
diff --git a/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/HttpServer.scala b/core/src/main/scala/com.snowplowanalytics.snowplow.collector.core/HttpServer.scala
@@ -119,7 +119,10 @@ object HttpServer {
     } else routes
 
   private def timeoutMiddleware[F[_]: Async](routes: HttpRoutes[F], networking: Config.Networking): HttpRoutes[F] =
-    Timeout.httpRoutes[F](timeout = networking.responseHeaderTimeout)(routes)
+    Timeout.httpRoutes[F](timeout = networking.responseHeaderTimeout)(routes).collect {
+      case Response(Status.ServiceUnavailable, httpVersion, headers, body, attributes) =>
+        Response[F](Status.RequestTimeout, httpVersion, headers, body, attributes)
+    }
 
   implicit class ConditionalAction[A](item: A) {
     def cond(cond: Boolean, action: A => A): A =

diff --git a/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/HttpServerSpec.scala b/core/src/test/scala/com.snowplowanalytics.snowplow.collector.core/HttpServerSpec.scala
@@ -15,9 +15,8 @@ class HttpServerSpecification extends Specification with CatsEffect {
   "HttpServer" should {
     "manage request timeout" should {
       "timeout threshold is configured" in {
-        val config = TestUtils
-          .testConfig
-          .copy(networking = TestUtils.testConfig.networking.copy(responseHeaderTimeout = 100.millis))
+        val config =
+          TestUtils.testConfig.copy(networking = TestUtils.testConfig.networking.copy(responseHeaderTimeout = 100.millis))
         val routes = HttpRoutes.of[IO] {
           case _ -> Root / "fast" =>
             Ok("Fast")
@@ -42,21 +41,10 @@ class HttpServerSpecification extends Specification with CatsEffect {
         res
           .attempt
           .map(_ must beLeft[Throwable].which {
-            case (_: org.http4s.client.UnexpectedStatus) => true
-            case _                                       => false
+            case org.http4s.client.UnexpectedStatus(Status.RequestTimeout, _, _) => true
+            case _                                                               => false
           })
       }
     }
-    "manage request length" should {
-      "request line length is configured" in {
-        false
-      }
-      "request headers length is configured" in {
-        false
-      }
-      "request payload length is configured" in {
-        false
-      }
-    }
   }
 }