From a342a3d60209e9b3bf3f5cbab7b0b6d6a7e1f2eb Mon Sep 17 00:00:00 2001
From: David Francoeur <dfrancoeur04@gmail.com>
Date: Wed, 14 Jun 2023 15:27:27 -0400
Subject: [PATCH] Revert original behavior where middleware get all errors

When merging https://github.com/disneystreaming/smithy4s/pull/877, I
introduced a behaviour change that I had not anticipated: the errors
defined in the smithy specs were turned into http response before
users had a chance to react to them in the middleware they provide

this is problematic and this fix reverts that
---
 .../SmithyHttp4sServerEndpoint.scala          |  7 ++--
 .../http4s/ServerEndpointMiddlewareSpec.scala | 33 +++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala
index baf028e44..600a10c32 100644
--- a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala
+++ b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala
@@ -107,7 +107,11 @@ private[http4s] class SmithyHttp4sServerEndpointImpl[F[_], Op[_, _, _, _, _], I,
     httpEndpoint.matches(path)
   }
 
-  private val applyMiddleware: HttpApp[F] => HttpApp[F] = middleware(endpoint)
+  private val applyMiddleware: HttpApp[F] => HttpApp[F] = { app =>
+    middleware(endpoint)(app).handleErrorWith(error =>
+      Kleisli.liftF(errorResponse(error))
+    )
+  }
 
   override val httpApp: HttpApp[F] =
     httpAppErrorHandle(applyMiddleware(HttpApp[F] { req =>
@@ -122,7 +126,6 @@ private[http4s] class SmithyHttp4sServerEndpointImpl[F[_], Op[_, _, _, _, _], I,
       run
         .recoverWith(transformError)
         .map(successResponse)
-        .handleErrorWith(errorResponse)
     }))
 
   private def httpAppErrorHandle(app: HttpApp[F]): HttpApp[F] = {
diff --git a/modules/http4s/test/src/smithy4s/http4s/ServerEndpointMiddlewareSpec.scala b/modules/http4s/test/src/smithy4s/http4s/ServerEndpointMiddlewareSpec.scala
index f4d3201d2..ac3136e7f 100644
--- a/modules/http4s/test/src/smithy4s/http4s/ServerEndpointMiddlewareSpec.scala
+++ b/modules/http4s/test/src/smithy4s/http4s/ServerEndpointMiddlewareSpec.scala
@@ -80,6 +80,39 @@ object ServerEndpointMiddlewareSpec extends SimpleIOSuite {
     List(throwCheck, pureCheck).combineAll
   }
 
+  test("server - middleware can catch spec error") {
+    val catchSpecErrorMiddleware = new ServerEndpointMiddleware.Simple[IO]() {
+      def prepareWithHints(
+          serviceHints: Hints,
+          endpointHints: Hints
+      ): HttpApp[IO] => HttpApp[IO] = { inputApp =>
+        HttpApp[IO] { req =>
+          inputApp(req).handleError { case _: SpecificServerError =>
+            Response[IO]()
+          }
+        }
+      }
+    }
+
+    SimpleRestJsonBuilder
+      .routes(new HelloWorldService[IO] {
+        def hello(name: String, town: Option[String]): IO[Greeting] =
+          IO.raiseError(
+            SpecificServerError(Some("to be caught in middleware"))
+          )
+      })
+      .middleware(catchSpecErrorMiddleware)
+      .make
+      .toOption
+      .get
+      .apply(Request[IO](Method.POST, Uri.unsafeFromString("/bob")))
+      // would be 599 w/o the middleware
+      .flatMap(res => OptionT.pure(expect.eql(res.status.code, 200)))
+      .getOrElse(
+        failure("unable to run request")
+      )
+  }
+
   test("server - middleware is applied") {
     serverMiddlewareTest(
       shouldFailInMiddleware = true,