Skip to content

Commit

Permalink
Explicit and customizable error messages for Endpoint BadRequest (#2650
Browse files Browse the repository at this point in the history
…) (#2714)

* Explicit and customizable error messages for Endpoint BadRequest (#2650)

* Update zio-http/shared/src/main/scala/zio/http/codec/BinaryCodecWithSchema.scala

Co-authored-by: John A. De Goes <[email protected]>

* Review changes

* Integrate changes from main

---------

Co-authored-by: John A. De Goes <[email protected]>
  • Loading branch information
987Nabil and jdegoes authored Apr 19, 2024
1 parent 22953e0 commit a4aa63f
Show file tree
Hide file tree
Showing 22 changed files with 886 additions and 443 deletions.
8 changes: 4 additions & 4 deletions docs/dsl/endpoint.md
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ object Quiz {
}

object EndpointWithMultipleOutputTypes extends ZIOAppDefault {
val endpoint: Endpoint[Unit, Unit, ZNothing, Either[Course, Quiz], None] =
val endpoint: Endpoint[Unit, Unit, ZNothing, Either[Quiz, Course], None] =
Endpoint(RoutePattern.GET / "resources")
.out[Course]
.out[Quiz]
Expand All @@ -264,8 +264,8 @@ object EndpointWithMultipleOutputTypes extends ZIOAppDefault {
endpoint.implement(handler {
ZIO.randomWith(_.nextBoolean)
.map(r =>
if (r) Left(Course("Introduction to Programming", 49.99))
else Right(Quiz("What is the boiling point of water in Celsius?", 2)),
if (r) Right(Course("Introduction to Programming", 49.99))
else Left(Quiz("What is the boiling point of water in Celsius?", 2)),
)
})
.toHttpApp).provide(Server.default, Scope.default)
Expand Down Expand Up @@ -306,7 +306,7 @@ implicit val bookSchema = DeriveSchema.gen[Book]
implicit val notFoundSchema = DeriveSchema.gen[BookNotFound]
implicit val authSchema = DeriveSchema.gen[AuthenticationError]

val endpoint: Endpoint[Int, (Int, Header.Authorization), Either[BookNotFound, AuthenticationError], Book, None] =
val endpoint: Endpoint[Int, (Int, Header.Authorization), Either[AuthenticationError, BookNotFound], Book, None] =
Endpoint(RoutePattern.GET / "books" / PathCodec.int("id"))
.header(HeaderCodec.authorization)
.out[Book]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,14 @@ private[cli] object CliEndpoint {
case Some(x) => x
case None => ""
}
CliEndpoint(body = HttpOptions.Body(name, codec.defaultMediaType, codec.schema) :: List())
CliEndpoint(body = HttpOptions.Body(name, codec.defaultMediaType, codec.defaultSchema) :: List())

case HttpCodec.ContentStream(codec, nameOption, _) =>
val name = nameOption match {
case Some(x) => x
case None => ""
}
CliEndpoint(body = HttpOptions.Body(name, codec.defaultMediaType, codec.schema) :: List())
CliEndpoint(body = HttpOptions.Body(name, codec.defaultMediaType, codec.defaultSchema) :: List())

case HttpCodec.Header(name, textCodec, _) if textCodec.isInstanceOf[TextCodec.Constant] =>
CliEndpoint(headers =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ object EndpointGen {
doc: Doc,
input: HttpCodec[CodecType, Input],
): Endpoint[Path, Input, ZNothing, ZNothing, EndpointMiddleware.None] =
Endpoint(RoutePattern.any, input, HttpCodec.unused, HttpCodec.unused, doc, EndpointMiddleware.None)
Endpoint(
RoutePattern.any,
input,
HttpCodec.unused,
HttpCodec.unused,
HttpContentCodec.responseErrorCodec,
doc,
EndpointMiddleware.None,
)

lazy val anyCliEndpoint: Gen[Any, CliReprOf[CliEndpoint]] =
anyCodec.map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ object EndpointWithMultipleErrorsUsingEither extends ZIOAppDefault {
}
}

val endpoint: Endpoint[Int, (Int, Header.Authorization), Either[BookNotFound, AuthenticationError], Book, None] =
val endpoint: Endpoint[Int, (Int, Header.Authorization), Either[AuthenticationError, BookNotFound], Book, None] =
Endpoint(RoutePattern.GET / "books" / PathCodec.int("id"))
.header(HeaderCodec.authorization)
.out[Book]
Expand All @@ -48,12 +48,12 @@ object EndpointWithMultipleErrorsUsingEither extends ZIOAppDefault {
def isUserAuthorized(authHeader: Header.Authorization) = false

val getBookHandler
: Handler[Any, Either[BookNotFound, AuthenticationError], (RuntimeFlags, Header.Authorization), Book] =
: Handler[Any, Either[AuthenticationError, BookNotFound], (RuntimeFlags, Header.Authorization), Book] =
handler { (id: Int, authHeader: Header.Authorization) =>
if (isUserAuthorized(authHeader))
BookRepo.find(id).mapError(Left(_))
BookRepo.find(id).mapError(Right(_))
else
ZIO.fail(Right(AuthenticationError("User is not authenticated", 123)))
ZIO.fail(Left(AuthenticationError("User is not authenticated", 123)))
}

val app = endpoint.implement(getBookHandler).toHttpApp @@ Middleware.debug
Expand Down
147 changes: 147 additions & 0 deletions zio-http/jvm/src/test/scala/zio/http/endpoint/BadRequestSpec.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
package zio.http.endpoint

import zio.test._

import zio.schema.codec.JsonCodec
import zio.schema.{DeriveSchema, Schema}

import zio.http._
import zio.http.codec._
import zio.http.template._

object BadRequestSpec extends ZIOSpecDefault {

override def spec =
suite("BadRequestSpec")(
test("should return html rendered error message by default for html accept header") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2").addHeader(Header.Accept(MediaType.text.`html`))
val expectedBody =
html(
body(
h1("Codec Error"),
p("There was an error en-/decoding the request/response"),
p("SchemaTransformationFailure", idAttr := "name"),
p("Expected single value for query parameter age, but got 2 instead", idAttr := "message"),
),
)
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody.encode.toString)
},
test("should return json rendered error message by default for json accept header") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2")
.addHeader(Header.Accept(MediaType.application.json))
val expectedBody =
"""{"name":"SchemaTransformationFailure","message":"Expected single value for query parameter age, but got 2 instead"}"""
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody)
},
test("should return json rendered error message by default as fallback for unsupported accept header") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2")
.addHeader(Header.Accept(MediaType.application.`atf`))
val expectedBody =
"""{"name":"SchemaTransformationFailure","message":"Expected single value for query parameter age, but got 2 instead"}"""
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody)
},
test("should return empty body after calling Endpoint#emptyErrorResponse") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
.emptyErrorResponse
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2")
.addHeader(Header.Accept(MediaType.application.`atf`))
val expectedBody = ""
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody)
},
test("should return custom error message") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2")
.addHeader(Header.Accept(MediaType.application.json))
val expectedBody =
"""{"name":"SchemaTransformationFailure","message":"Expected single value for query parameter age, but got 2 instead"}"""
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody)
},
test("should use custom error codec over default error codec") {
val endpoint = Endpoint(Method.GET / "test")
.query(QueryCodec.queryInt("age"))
.out[Unit]
.outCodecError(default)
val route = endpoint.implement(handler((_: Int) => ()))
val app = route.toHttpApp
val request =
Request(method = Method.GET, url = url"/test?age=1&age=2")
.addHeader(Header.Accept(MediaType.application.json))
val expectedBody =
"""{"name2":"SchemaTransformationFailure","message2":"Expected single value for query parameter age, but got 2 instead"}"""
for {
response <- app.runZIO(request)
body <- response.body.asString
} yield assertTrue(body == expectedBody)
},
)

private val defaultCodecErrorSchema: Schema[HttpCodecError] =
Schema[DefaultCodecError2].transformOrFail[HttpCodecError](
codecError => Right(HttpCodecError.CustomError(codecError.name2, codecError.message2)),
{
case HttpCodecError.CustomError(name, message) => Right(DefaultCodecError2(name, message))
case e: HttpCodecError => Right(DefaultCodecError2(e.productPrefix, e.getMessage()))
},
)

private val testHttpContentCodec: HttpContentCodec[HttpCodecError] =
HttpContentCodec.from(
MediaType.application.json -> BinaryCodecWithSchema(
JsonCodec.schemaBasedBinaryCodec(defaultCodecErrorSchema),
defaultCodecErrorSchema,
),
)

val default: HttpCodec[HttpCodecType.ResponseType, HttpCodecError] =
ContentCodec.content(testHttpContentCodec) ++ StatusCodec.BadRequest

final case class DefaultCodecError2(name2: String, message2: String)

private object DefaultCodecError2 {
implicit val schema: Schema[DefaultCodecError2] = DeriveSchema.gen[DefaultCodecError2]
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ object CustomErrorSpec extends ZIOHttpSpec {
Endpoint(POST / "users")
.in[User](Doc.p("User schema with id"))
.out[String]
.emptyErrorResponse
.implement {
Handler.fromFunctionZIO { _ =>
ZIO.succeed("User ID is greater than 0")
Expand Down
14 changes: 6 additions & 8 deletions zio-http/jvm/src/test/scala/zio/http/endpoint/RequestSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -136,18 +136,16 @@ object RequestSpec extends ZIOHttpSpec {
Endpoint(GET / "posts")
.query(queryInt("id"))
.out[Int]
val routes =
endpoint.implement {
Handler.succeed(id)
}

val routes = endpoint.implement { Handler.succeed(id) }
for {
response <- routes.toHttpApp.runZIO(
Request.get(URL.decode(s"/posts?id=$notAnId").toOption.get),
Request.get(url"/posts?id=$notAnId").addHeader(Header.Accept(MediaType.application.`json`)),
)
contentType = response.header(Header.ContentType)
} yield assertTrue(extractStatus(response).code == 400) &&
assertTrue(contentType.isEmpty)
} yield assertTrue(
extractStatus(response).code == 400,
contentType.map(_.mediaType).contains(MediaType.application.`json`),
)
}
},
test("header codec") {
Expand Down
4 changes: 2 additions & 2 deletions zio-http/shared/src/main/scala/zio/http/MediaType.scala
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@ final case class MediaType(
) {
lazy val fullType: String = s"$mainType/$subType"

def matches(other: MediaType): Boolean =
def matches(other: MediaType, ignoreParameters: Boolean = false): Boolean =
(mainType == "*" || other.mainType == "*" || mainType.equalsIgnoreCase(other.mainType)) &&
(subType == "*" || other.subType == "*" || subType.equalsIgnoreCase(other.subType)) &&
parameters.forall { case (key, value) => other.parameters.get(key).contains(value) }
(ignoreParameters || parameters.forall { case (key, value) => other.parameters.get(key).contains(value) })
}

object MediaType extends MediaTypes {
Expand Down
8 changes: 8 additions & 0 deletions zio-http/shared/src/main/scala/zio/http/Route.scala
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,14 @@ sealed trait Route[-Env, +Err] { self =>
final def handleError(f: Err => Response)(implicit trace: Trace): Route[Env, Nothing] =
self.handleErrorCause(Response.fromCauseWith(_)(f))

final def handleErrorZIO(f: Err => ZIO[Any, Nothing, Response])(implicit trace: Trace): Route[Env, Nothing] =
self.handleErrorCauseZIO { cause =>
cause.failureOrCause match {
case Left(err) => f(err)
case Right(cause) => ZIO.succeed(Response.fromCause(cause))
}
}

/**
* Handles all typed errors, as well as all non-recoverable errors, by
* converting them into responses. This method can be used to convert a route
Expand Down
3 changes: 3 additions & 0 deletions zio-http/shared/src/main/scala/zio/http/Routes.scala
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@ final class Routes[-Env, +Err] private (val routes: Chunk[zio.http.Route[Env, Er
def handleError(f: Err => Response)(implicit trace: Trace): Routes[Env, Nothing] =
new Routes(routes.map(_.handleError(f)))

def handleErrorZIO(f: Err => ZIO[Any, Nothing, Response])(implicit trace: Trace): Routes[Env, Nothing] =
new Routes(routes.map(_.handleErrorZIO(f)))

/**
* Handles all typed errors, as well as all non-recoverable errors, by
* converting them into responses. This method can be used to convert routes
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package zio.http.codec

import zio.schema.Schema
import zio.schema.codec.BinaryCodec

final case class BinaryCodecWithSchema[A](codec: BinaryCodec[A], schema: Schema[A])

object BinaryCodecWithSchema {
def fromBinaryCodec[A](codec: BinaryCodec[A])(implicit schema: Schema[A]): BinaryCodecWithSchema[A] =
BinaryCodecWithSchema(codec, schema)
}
14 changes: 7 additions & 7 deletions zio-http/shared/src/main/scala/zio/http/codec/HttpCodec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
def unused: HttpCodec[Any, ZNothing] = Halt

final case class Enumeration[Value](unit: Unit) extends AnyVal {
def apply[AtomTypes, Sub1 <: Value: ClassTag, Sub2 <: Value: ClassTag](
def f2[AtomTypes, Sub1 <: Value: ClassTag, Sub2 <: Value: ClassTag](
codec1: HttpCodec[AtomTypes, Sub1],
codec2: HttpCodec[AtomTypes, Sub2],
): HttpCodec[AtomTypes, Value] =
Expand All @@ -346,7 +346,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[AtomTypes, Sub1 <: Value: ClassTag, Sub2 <: Value: ClassTag, Sub3 <: Value: ClassTag](
def f3[AtomTypes, Sub1 <: Value: ClassTag, Sub2 <: Value: ClassTag, Sub3 <: Value: ClassTag](
codec1: HttpCodec[AtomTypes, Sub1],
codec2: HttpCodec[AtomTypes, Sub2],
codec3: HttpCodec[AtomTypes, Sub3],
Expand All @@ -360,7 +360,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[
def f4[
AtomTypes,
Sub1 <: Value: ClassTag,
Sub2 <: Value: ClassTag,
Expand All @@ -384,7 +384,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[
def f5[
AtomTypes,
Sub1 <: Value: ClassTag,
Sub2 <: Value: ClassTag,
Expand All @@ -411,7 +411,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[
def f6[
AtomTypes,
Sub1 <: Value: ClassTag,
Sub2 <: Value: ClassTag,
Expand Down Expand Up @@ -441,7 +441,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[
def f7[
AtomTypes,
Sub1 <: Value: ClassTag,
Sub2 <: Value: ClassTag,
Expand Down Expand Up @@ -474,7 +474,7 @@ object HttpCodec extends ContentCodecs with HeaderCodecs with MethodCodecs with
},
)

def apply[
def f8[
AtomTypes,
Sub1 <: Value: ClassTag,
Sub2 <: Value: ClassTag,
Expand Down
Loading

0 comments on commit a4aa63f

Please sign in to comment.