Undocumented error handling behavior change in 0.18.x

[kubukoz]

Consider the following code:

//> using lib "com.disneystreaming.smithy4s::smithy4s-http4s:0.18.1"
//> using lib "com.disneystreaming.smithy4s::smithy4s-tests:0.18.1"
//> using option "-Wunused:imports"
import cats.effect.IO
import cats.effect.IOApp
import cats.implicits._
import org.http4s.Request
import org.http4s.implicits._
import smithy4s.Document
import smithy4s.example.HealthResponse
import smithy4s.example.PizzaAdminService
import smithy4s.http4s.SimpleRestJsonBuilder
import smithy4s.schema.Schema

object Main extends IOApp.Simple {
  val route = SimpleRestJsonBuilder
    .routes(
      new PizzaAdminService.Default[IO](IO.stub) {
        override def health(query: Option[String]): IO[HealthResponse] =
          Document.Decoder
            .fromSchema(Schema.string)
            .decode(Document.fromInt(42))
            .liftTo[IO] *> IO.stub
      }: PizzaAdminService[IO]
    )
    .make
    .toTry
    .get

  override def run: IO[Unit] =
    route.run(Request(uri = uri"/health")).value.flatMap(IO.println(_))
}

Most importantly: the handler of the request fails with a PayloadError (due to decoding a document, but it could be for other reasons as well).

In 0.17.19, the code above prints:

Some(Response(status=400, httpVersion=HTTP/1.1, headers=Headers(Content-Length: 73, Content-Type: application/json)))

In 0.18.1, it fails:

PayloadError(., expected = String, message=Wrong Json shape)

It seems to be because of some changes around PayloadError / HttpPayloadError / HttpContractError.

The old behavior can be restored by remapping the exception to a HttpContractError:

diff --git a/Main.scala b/Main.scala
index e12618b..595b53b 100644
--- a/Main.scala
+++ b/Main.scala
@@ -11,6 +11,7 @@ import smithy4s.example.HealthResponse
 import smithy4s.example.PizzaAdminService
 import smithy4s.http4s.SimpleRestJsonBuilder
 import smithy4s.schema.Schema
+import smithy4s.http.HttpContractError
 
 object Main extends IOApp.Simple {
   val route = SimpleRestJsonBuilder
@@ -20,6 +21,7 @@ object Main extends IOApp.Simple {
           Document.Decoder
             .fromSchema(Schema.string)
             .decode(Document.fromInt(42))
+            .leftMap(HttpContractError.fromPayloadError)
             .liftTo[IO] *> IO.stub
       }: PizzaAdminService[IO]
     )

Is this a regression or just something we should document?

[Baccata]

Good question. TBH, the fact that it matched your expectations previously was kinda accidental. However it may make sense from a least-surprise point of view. It may be worth treating PayloadError with special handling in the router