From 109e9d0b77173cd1c2977d3bfaf64fc2bedb38a6 Mon Sep 17 00:00:00 2001
From: Piotr Limanowski <peel@users.noreply.github.com>
Date: Wed, 21 Aug 2024 17:19:40 +0200
Subject: [PATCH] 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.
---
 .../HttpServer.scala                          |  5 ++++-
 .../HttpServerSpec.scala                      | 20 ++++---------------
 2 files changed, 8 insertions(+), 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
index c9119deec..8e0b885f3 100644
--- 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
index f46a51473..b76fdb632 100644
--- 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
-      }
-    }
   }
 }