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.

Sign up for GitHub

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

Jump to bottom

MessageFailure: simplify toHttpResponse/inHttpResponse #3110

Merged
merged 1 commit into from Jan 24, 2020
Merged

MessageFailure: simplify toHttpResponse/inHttpResponse #3110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 6 additions & 7 deletions circe/src/main/scala/org/http4s/circe/middleware/JsonDebugErrorHandler.scala
View file
Open in desktop
Expand Up @@ -32,13 +32,12 @@ object JsonDebugErrorHandler {
messageFailureLogger.debug(mf)(
s"""Message failure handling request: ${req.method} ${req.pathInfo} from ${req.remoteAddr
.getOrElse("<unknown>")}""")
mf.inHttpResponse[F, G](req.httpVersion).map { firstResp =>
Response[G](
status = firstResp.status,
httpVersion = firstResp.httpVersion,
headers = firstResp.headers.redactSensitive(redactWhen)
).withEntity(JsonErrorHandlerResponse[G](req, mf))
}
val firstResp = mf.toHttpResponse[G](req.httpVersion)
Response[G](
status = firstResp.status,
httpVersion = firstResp.httpVersion,
headers = firstResp.headers.redactSensitive(redactWhen)
).withEntity(JsonErrorHandlerResponse[G](req, mf)).pure[F]
case t =>
serviceErrorLogger.error(t)(
s"""Error servicing request: ${req.method} ${req.pathInfo} from ${req.remoteAddr
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/scala/org/http4s/Message.scala
View file
Open in desktop
Expand Up @@ -367,7 +367,10 @@ final class Request[F[_]](

def decodeWith[A](decoder: EntityDecoder[F, A], strict: Boolean)(f: A => F[Response[F]])(
implicit F: Monad[F]): F[Response[F]] =
decoder.decode(this, strict = strict).fold(_.toHttpResponse[F](httpVersion), f).flatten
decoder
.decode(this, strict = strict)
.fold(_.toHttpResponse[F](httpVersion).pure[F], f)
.flatten

/** Helper method for decoding [[Request]]s
*
Expand Down
34 changes: 11 additions & 23 deletions core/src/main/scala/org/http4s/MessageFailure.scala
View file
Open in desktop
@@ -1,6 +1,6 @@
package org.http4s

import cats.{Applicative, Eq, MonadError}
import cats.{Eq, MonadError}
import cats.implicits._
import scala.util.control.{NoStackTrace, NonFatal}

Expand All @@ -15,14 +15,10 @@ trait MessageFailure extends RuntimeException {

def cause: Option[Throwable]

final override def getCause = cause.orNull
final override def getCause: Throwable = cause.orNull

/** Provides a default rendering of this failure as a [[Response]]. */
def toHttpResponse[F[_]](httpVersion: HttpVersion)(implicit F: Applicative[F]): F[Response[F]] =
inHttpResponse[F, F](httpVersion)

def inHttpResponse[F[_], G[_]](
httpVersion: HttpVersion)(implicit F: Applicative[F], G: Applicative[G]): F[Response[G]]
def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F]
}

/**
Expand All @@ -43,11 +39,9 @@ final case class ParseFailure(sanitized: String, details: String)

def cause: Option[Throwable] = None

def inHttpResponse[F[_]: Applicative, G[_]: Applicative](
httpVersion: HttpVersion): F[Response[G]] =
def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F] =
Response(Status.BadRequest, httpVersion)
.withEntity(sanitized)(EntityEncoder.stringEncoder[G])
.pure[F]
.withEntity(sanitized)(EntityEncoder.stringEncoder[F])
}

object ParseFailure {
Expand Down Expand Up @@ -88,11 +82,9 @@ final case class MalformedMessageBodyFailure(details: String, cause: Option[Thro
def message: String =
s"Malformed message body: $details"

def inHttpResponse[F[_]: Applicative, G[_]: Applicative](
httpVersion: HttpVersion): F[Response[G]] =
def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F] =
Response(Status.BadRequest, httpVersion)
.withEntity(s"The request body was malformed.")(EntityEncoder.stringEncoder[G])
.pure[F]
.withEntity(s"The request body was malformed.")(EntityEncoder.stringEncoder[F])
}

/** Indicates a semantic error decoding the body of an HTTP [[Message]]. */
Expand All @@ -101,11 +93,9 @@ final case class InvalidMessageBodyFailure(details: String, cause: Option[Throwa
def message: String =
s"Invalid message body: $details"

def inHttpResponse[F[_]: Applicative, G[_]: Applicative](
httpVersion: HttpVersion): F[Response[G]] =
def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F] =
Response(Status.UnprocessableEntity, httpVersion)
.withEntity(s"The request body was invalid.")(EntityEncoder.stringEncoder[G])
.pure[F]
.withEntity(s"The request body was invalid.")(EntityEncoder.stringEncoder[F])
}

/** Indicates that a [[Message]] came with no supported [[MediaType]]. */
Expand All @@ -118,11 +108,9 @@ sealed abstract class UnsupportedMediaTypeFailure extends DecodeFailure with NoS
s"Expected one of the following media ranges: ${expected.map(_.show).mkString(", ")}"
protected def responseMsg: String = s"$sanitizedResponsePrefix. $expectedMsg"

def inHttpResponse[F[_]: Applicative, G[_]: Applicative](
httpVersion: HttpVersion): F[Response[G]] =
def toHttpResponse[F[_]](httpVersion: HttpVersion): Response[F] =
Response(Status.UnsupportedMediaType, httpVersion)
.withEntity(responseMsg)(EntityEncoder.stringEncoder[G])
.pure[F]
.withEntity(responseMsg)(EntityEncoder.stringEncoder[F])
}

/** Indicates that a [[Message]] attempting to be decoded has no [[MediaType]] and no
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/scala/org/http4s/server/middleware/ErrorHandling.scala
View file
Open in desktop
Expand Up @@ -8,11 +8,10 @@ import org.http4s._

object ErrorHandling {
def apply[F[_], G[_]](k: Kleisli[F, Request[G], Response[G]])(
implicit F: MonadError[F, Throwable],
G: Applicative[G]): Kleisli[F, Request[G], Response[G]] =
implicit F: MonadError[F, Throwable]): Kleisli[F, Request[G], Response[G]] =
Kleisli { req =>
val pf: PartialFunction[Throwable, F[Response[G]]] =
inDefaultServiceErrorHandler[F, G](F, G)(req)
inDefaultServiceErrorHandler[F, G](F)(req)
k.run(req).handleErrorWith { e =>
pf.lift(e) match {
case Some(resp) => resp
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/scala/org/http4s/server/middleware/UrlFormLifter.scala
View file
Open in desktop
Expand Up @@ -38,7 +38,7 @@ object UrlFormLifter {
for {
decoded <- f(UrlForm.entityDecoder[G].decode(req, strictDecode).value)
resp <- decoded.fold(
mf => f(mf.toHttpResponse[G](req.httpVersion)),
mf => f(mf.toHttpResponse[G](req.httpVersion).pure[G]),
addUrlForm
)
} yield resp
Expand Down
5 changes: 2 additions & 3 deletions server/src/main/scala/org/http4s/server/package.scala
View file
Open in desktop
Expand Up @@ -134,13 +134,12 @@ package object server {
inDefaultServiceErrorHandler[F, F]

def inDefaultServiceErrorHandler[F[_], G[_]](
implicit F: Monad[F],
G: Applicative[G]): Request[G] => PartialFunction[Throwable, F[Response[G]]] = req => {
implicit F: Monad[F]): Request[G] => PartialFunction[Throwable, F[Response[G]]] = req => {
case mf: MessageFailure =>
messageFailureLogger.debug(mf)(
s"""Message failure handling request: ${req.method} ${req.pathInfo} from ${req.remoteAddr
Copy link
Member

Choose a reason for hiding this comment

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

To @ChristopherDavenport's concern, the relevant logging I see is here, and in JsonDebugger. In both these places, we're returning in F and could compose an effect-tracked log with the response. But I don't see any logging where we're creating these responses, so the effect is more power than we want or need. Maybe I'm missing something, but I think both of you can be made happy?

Copy link
Member

Choose a reason for hiding this comment

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

Excuse me. This one is missing Sync.

.getOrElse("<unknown>")}""")
mf.inHttpResponse[F, G](req.httpVersion)
mf.toHttpResponse[G](req.httpVersion).pure[F]
case NonFatal(t) =>
serviceErrorLogger.error(t)(
s"""Error servicing request: ${req.method} ${req.pathInfo} from ${req.remoteAddr.getOrElse(
Expand Down
2 changes: 1 addition & 1 deletion tests/src/test/scala/org/http4s/EntityDecoderSpec.scala
View file
Open in desktop
Expand Up @@ -174,7 +174,7 @@ class EntityDecoderSpec extends Http4sSpec with PendingUntilFixed {
Request[IO](headers = Headers.of(`Content-Type`(MediaType.text.plain))),
strict = true)
.swap
.semiflatMap(_.toHttpResponse[IO](HttpVersion.`HTTP/1.1`)) must returnRight(
.map(_.toHttpResponse[IO](HttpVersion.`HTTP/1.1`)) must returnRight(
haveStatus(Status.UnprocessableEntity))
}

Expand Down