issue-1438: implemented RawErrorResponse error object

disneystreaming · Aug 2, 2024 · 0f9e5f1 · 0f9e5f1
1 parent 9fb8b3b
commit 0f9e5f1
Show file tree

Hide file tree

Showing 7 changed files with 104 additions and 81 deletions.
diff --git a/modules/core/src/smithy4s/http/HttpContractError.scala b/modules/core/src/smithy4s/http/HttpContractError.scala
@@ -42,12 +42,9 @@ object HttpContractError {
   val schema: Schema[HttpContractError] = {
     val payload = HttpPayloadError.schema.oneOf[HttpContractError]("payload")
     val metadata = MetadataError.schema.oneOf[HttpContractError]("metadata")
-    val upstreamServiceError = UpstreamServiceError.schema
-      .oneOf[HttpContractError]("upstreamServiceError")
-    union(payload, metadata, upstreamServiceError) {
-      case _: HttpPayloadError     => 0
-      case _: MetadataError        => 1
-      case _: UpstreamServiceError => 2
+    union(payload, metadata) {
+      case _: HttpPayloadError => 0
+      case _: MetadataError    => 1
     }
   }
 
@@ -73,7 +70,7 @@ object HttpPayloadError {
 }
 
 sealed trait MetadataError extends HttpContractError {
-  import MetadataError._
+  import MetadataError.*
 
   override def getMessage(): String = this match {
     case NotFound(field, location) =>
@@ -178,17 +175,3 @@ object MetadataError {
   }
 
 }
-
-case class UpstreamServiceError(message: String) extends HttpContractError {
-  override def toString: String =
-    s"UpstreamServiceError(message=$message)"
-
-  override def getMessage: String = message
-}
-
-object UpstreamServiceError {
-  val schema: Schema[UpstreamServiceError] = {
-    val message = string.required[UpstreamServiceError]("message", _.message)
-    struct(message)(UpstreamServiceError.apply)
-  }
-}
diff --git a/modules/core/src/smithy4s/http/HttpResponse.scala b/modules/core/src/smithy4s/http/HttpResponse.scala
@@ -228,9 +228,9 @@ object HttpResponse {
     * Creates a response decoder that dispatches  the response
     * to a given decoder, based on some discriminator.
     */
-    private def discriminating[F[_], Body, Discriminator, E](
-        discriminate: HttpResponse[Body] => F[Discriminator],
-        select: Discriminator => Option[Decoder[F, Body, E]],
+    private def discriminating[F[_], Body, E](
+        discriminate: HttpResponse[Body] => F[HttpDiscriminator],
+        select: HttpDiscriminator => Option[Decoder[F, Body, E]],
         toStrict: Body => F[(Body, Blob)]
     )(implicit F: MonadThrowLike[F]): Decoder[F, Body, E] =
       new Decoder[F, Body, E] {
@@ -239,13 +239,39 @@ object HttpResponse {
             val strictResponse = response.copy(body = strictBody)
             F.flatMap(discriminate(strictResponse)) { discriminator =>
               select(discriminator) match {
-                case Some(decoder) => decoder.decode(strictResponse)
+                case Some(decoder) =>
+                  F.flatMap(F.handleErrorWith(decoder.decode(strictResponse)) {
+                    case error: HttpContractError =>
+                      F.raiseError(
+                        RawErrorResponse(
+                          response.statusCode,
+                          response.headers,
+                          bodyBlob.toUTF8String,
+                          Some(FailedDecodeAttempt(
+                            discriminator = discriminator,
+                            contractError = error
+                          ))
+                        )
+                      )
+                    case otherError => F.raiseError(otherError)
+                  })(F.pure(_))
                 case None =>
                   F.raiseError(
-                    smithy4s.http.UnknownErrorResponse(
-                      response.statusCode,
-                      response.headers,
-                      bodyBlob.toUTF8String
+                    smithy4s.http.RawErrorResponse(
+                      code = response.statusCode,
+                      headers = response.headers,
+                      body = bodyBlob.toUTF8String,
+                      failedDecodeAttempt = Some(
+                        FailedDecodeAttempt(
+                        discriminator = HttpDiscriminator.StatusCode(response.statusCode),
+                        contractError = HttpPayloadError(
+                          path = smithy4s.codecs.PayloadPath(List()),
+                          expected = "JSON",
+                          message = "Unknown error due to unrecognised discriminator"
+                        )
+                        )
+
+                      )
                     )
                   )
               }

diff --git a/modules/core/src/smithy4s/http/HttpUnaryServerCodecs.scala b/modules/core/src/smithy4s/http/HttpUnaryServerCodecs.scala
@@ -196,11 +196,7 @@ object HttpUnaryServerCodecs {
           def encodeOutput(o: O) = F.map(base)(outputW.write(_, o))
           def encodeError(e: E) = F.map(base)(errorW.write(_, e))
           def httpContractErrorEncoder(e: HttpContractError) = {
-            val statusCode = e match {
-              case _: UpstreamServiceError => 500
-              case _                       => 400
-            }
-            F.map(base)(httpContractErrorWriters.write(_, e).withStatusCode(statusCode))
+            F.map(base)(httpContractErrorWriters.write(_, e).withStatusCode(400))
           }
 
           def throwableEncoders(throwable: Throwable): F[HttpResponse[Blob]] =

diff --git a/modules/core/src/smithy4s/http/RawErrorResponse.scala b/modules/core/src/smithy4s/http/RawErrorResponse.scala
@@ -0,0 +1,22 @@
+package smithy4s.http
+
+case class RawErrorResponse(
+    code: Int,
+    headers: Map[CaseInsensitive, Seq[String]],
+    body: String,
+    failedDecodeAttempt: Option[FailedDecodeAttempt]
+) extends Throwable {
+  override def getMessage(): String = {
+    val baseMessage = s"status $code, headers: $headers, body:\n$body"
+    failedDecodeAttempt match {
+      case Some(attempt) =>
+        baseMessage + s"\nFailedDecodeAttempt:\n  discriminator: ${attempt.discriminator}\n  contractError: ${attempt.contractError}"
+      case None => baseMessage
+    }
+  }
+}
+
+case class FailedDecodeAttempt(
+    discriminator: HttpDiscriminator,
+    contractError: HttpContractError
+)
diff --git a/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala b/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala
@@ -21,7 +21,6 @@ import cats.effect.std.UUIDGen
 import cats.implicits._
 import smithy4s.Timestamp
 import smithy4s.example._
-import smithy4s.http.UpstreamServiceError
 import smithy4s.tests.PizzaAdminServiceImpl._
 
 import java.util.UUID
@@ -51,9 +50,6 @@ class PizzaAdminServiceImpl(ref: Ref[IO, State]) extends PizzaAdminService[IO] {
   ): IO[AddMenuItemResult] =
     for {
       _ <- IO.raiseError(Boom).whenA(restaurant == "boom")
-      _ <- IO
-        .raiseError(UpstreamServiceError("Upstream service failure"))
-        .whenA(restaurant == "upstreamServiceError")
       _ <- IO
         .raiseError(
           PriceError(

diff --git a/modules/tests/src/smithy4s/tests/PizzaClientSpec.scala b/modules/tests/src/smithy4s/tests/PizzaClientSpec.scala
@@ -31,7 +31,10 @@ import smithy4s.example._
 import smithy4s.Timestamp
 import weaver._
 import smithy4s.http.CaseInsensitive
-import smithy4s.http.UnknownErrorResponse
+import smithy4s.http.RawErrorResponse
+import smithy4s.http.FailedDecodeAttempt
+import smithy4s.http.HttpPayloadError
+import smithy4s.http.HttpDiscriminator
 
 abstract class PizzaClientSpec extends IOSuite {
 
@@ -117,22 +120,52 @@ abstract class PizzaClientSpec extends IOSuite {
       .withEntity(
         Json.obj("message" -> Json.fromString("generic client error message"))
       ),
-    unknownResponse(
+    rawErrorResponse(
       407,
       Map("Content-Length" -> "42", "Content-Type" -> "application/json"),
-      """{"message":"generic client error message"}"""
+      """{"message":"generic client error message"}""",
+      Some(FailedDecodeAttempt(
+        discriminator = HttpDiscriminator.StatusCode(407),
+        contractError = HttpPayloadError(
+          path = smithy4s.codecs.PayloadPath(List()),
+          expected = "JSON",
+          message = "Unknown error due to unrecognised discriminator"
+        )
+      ))
     )
   )
 
-  private def unknownResponse(
-      code: Int,
-      headers: Map[String, String],
-      body: String
-  ): UnknownErrorResponse =
-    UnknownErrorResponse(
+  clientTestForError(
+    "Handle malformed error response",
+    Response(status = Status.InternalServerError)
+      .withEntity("goodbye world"),
+    rawErrorResponse(
+      500,
+      Map("Content-Length" -> "13", "Content-Type" -> "text/plain; charset=UTF-8"),
+      "goodbye world",
+      Some(FailedDecodeAttempt(
+        discriminator = HttpDiscriminator.StatusCode(500),
+        contractError = HttpPayloadError(
+          path = smithy4s.codecs.PayloadPath(List()),
+          expected = "JSON",
+          message = "Unknown error due to unrecognised discriminator"
+        )
+      ))
+    )
+  )
+
+
+  private def rawErrorResponse(
+     code: Int,
+     headers: Map[String, String],
+     body: String,
+     failedDecodeAttempt: Option[FailedDecodeAttempt]
+  ): RawErrorResponse =
+    RawErrorResponse(
       code,
       headers.map { case (k, v) => CaseInsensitive(k) -> List(v) },
-      body
+      body,
+      failedDecodeAttempt
     )
 
   clientTest("Headers are case insensitive") { (client, backend, log) =>

diff --git a/modules/tests/src/smithy4s/tests/PizzaSpec.scala b/modules/tests/src/smithy4s/tests/PizzaSpec.scala
@@ -30,7 +30,6 @@ import smithy4s.http.HttpPayloadError
 import smithy4s.example.PizzaAdminService
 import smithy4s.http.CaseInsensitive
 import smithy4s.http.HttpContractError
-import smithy4s.http.UpstreamServiceError
 import weaver._
 import cats.Show
 import org.http4s.EntityDecoder
@@ -464,36 +463,6 @@ abstract class PizzaSpec
       }
   }
 
-  routerTest("Upstream service error returns 500") { (client, uri, log) =>
-    val badMenuItem = Json.obj(
-      "food" -> pizzaItem,
-      "price" -> Json.fromFloatOrNull(9.0f)
-    )
-
-    for {
-      res <- client.send[Json](
-        POST(
-          badMenuItem,
-          uri / "restaurant" / "upstreamServiceError" / "menu" / "item"
-        ),
-        log
-      )
-    } yield {
-      val (code, headers, body) = res
-      val expectedBody =
-        Json.obj(
-          "upstreamServiceError" -> Json.obj(
-            "message" -> Json.fromString("Upstream service failure")
-          )
-        )
-      val discriminator = headers.get("X-Error-Type").flatMap(_.headOption)
-
-      expect(code == 500) &&
-      expect(body == expectedBody) &&
-      expect(discriminator == None)
-    }
-  }
-
   type Res = (Client[IO], Uri)
   def sharedResource: Resource[IO, (Client[IO], Uri)] = for {
     stateRef <- Resource.eval(
@@ -507,8 +476,6 @@ abstract class PizzaSpec
           smithy4s.example.GenericClientError("Oops")
         case PizzaAdminServiceImpl.Boom =>
           smithy4s.example.GenericServerError("Crash")
-        case UpstreamServiceError(message) =>
-          UpstreamServiceError(message)
         case t: Throwable if !t.isInstanceOf[HttpContractError] =>
           // This pattern allows checking that errors specified in specs
           // do not get intercepted by mapErrors/flatMapErrors methods.