Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a 2nd matches, rather than introducing a boolean parameter with a default. Maybe matchesIgnoreParams.

(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](
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were these all renamed??

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After changing the order, the compiler could not infer the right apply. So I gave them explicit names.

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
Loading