From d61ed25db4d71ac182f09ce7bc02439470f3a998 Mon Sep 17 00:00:00 2001 From: zachdriver Date: Fri, 25 May 2018 12:44:15 -0700 Subject: Add service exception json formatters and pass through exceptions in HttpRestServiceTransport (#168) --- src/main/scala/xyz/driver/core/core.scala | 11 ++++++++ src/main/scala/xyz/driver/core/json.scala | 19 +++++++++++++ .../scala/xyz/driver/core/rest/DriverRoute.scala | 12 ++++++-- .../core/rest/HttpRestServiceTransport.scala | 7 +++-- .../driver/core/rest/errors/serviceException.scala | 5 +++- .../xyz/driver/core/rest/DriverRouteTest.scala | 33 ++++++++++++++++------ 6 files changed, 73 insertions(+), 14 deletions(-) diff --git a/src/main/scala/xyz/driver/core/core.scala b/src/main/scala/xyz/driver/core/core.scala index 14e1b10..72237b9 100644 --- a/src/main/scala/xyz/driver/core/core.scala +++ b/src/main/scala/xyz/driver/core/core.scala @@ -3,6 +3,9 @@ package xyz.driver import scalaz.{Equal, Monad, OptionT} import eu.timepit.refined.api.{Refined, Validate} import eu.timepit.refined.collection.NonEmpty +import xyz.driver.core.rest.errors.ExternalServiceException + +import scala.concurrent.{ExecutionContext, Future} package object core { @@ -54,6 +57,14 @@ package object core { def toUnitOptionT: OptionT[H, Unit] = OptionT.optionT[H](monadT(monadicValue)(_ => Option(()))) } + + implicit class FutureExtensions[T](future: Future[T]) { + def passThroughExternalServiceException(implicit executionContext: ExecutionContext): Future[T] = + future.transform(identity, { + case ExternalServiceException(_, _, Some(e)) => e + case t: Throwable => t + }) + } } package core { diff --git a/src/main/scala/xyz/driver/core/json.scala b/src/main/scala/xyz/driver/core/json.scala index c1c5862..de1df31 100644 --- a/src/main/scala/xyz/driver/core/json.scala +++ b/src/main/scala/xyz/driver/core/json.scala @@ -16,6 +16,7 @@ import spray.json._ import xyz.driver.core.auth.AuthCredentials import xyz.driver.core.date.{Date, DayOfWeek, Month} import xyz.driver.core.domain.{Email, PhoneNumber} +import xyz.driver.core.rest.errors._ import xyz.driver.core.time.{Time, TimeOfDay} import scala.reflect.runtime.universe._ @@ -368,6 +369,24 @@ object json { NonEmptyName[T](nonEmptyStringFormat.read(value)) } + implicit val serviceExceptionFormat: RootJsonFormat[ServiceException] = + GadtJsonFormat.create[ServiceException]("type") { + case _: InvalidInputException => "InvalidInputException" + case _: InvalidActionException => "InvalidActionException" + case _: ResourceNotFoundException => "ResourceNotFoundException" + case _: ExternalServiceException => "ExternalServiceException" + case _: ExternalServiceTimeoutException => "ExternalServiceTimeoutException" + case _: DatabaseException => "DatabaseException" + } { + case "InvalidInputException" => jsonFormat(InvalidInputException, "message") + case "InvalidActionException" => jsonFormat(InvalidActionException, "message") + case "ResourceNotFoundException" => jsonFormat(ResourceNotFoundException, "message") + case "ExternalServiceException" => + jsonFormat(ExternalServiceException, "serviceName", "serviceMessage", "serviceException") + case "ExternalServiceTimeoutException" => jsonFormat(ExternalServiceTimeoutException, "message") + case "DatabaseException" => jsonFormat(DatabaseException, "message") + } + val jsValueToStringMarshaller: Marshaller[JsValue, String] = Marshaller.strict[JsValue, String](value => Marshalling.Opaque[String](() => value.compactPrint)) diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index c23c44d..fe3790f 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -90,11 +90,17 @@ trait DriverRoute { } { (ctx: RequestContext) => - errorResponse(statusCode, serviceException.message, serviceException)(ctx) + import xyz.driver.core.json.serviceExceptionFormat + val entity = + HttpEntity(ContentTypes.`application/json`, serviceExceptionFormat.write(serviceException).toString()) + errorResponse(statusCode, entity, serviceException)(ctx) } } - protected def errorResponse[T <: Exception](statusCode: StatusCode, message: String, exception: T): Route = { - complete(HttpResponse(statusCode, entity = message)) + protected def errorResponse[T <: Exception](statusCode: StatusCode, message: String, exception: T): Route = + errorResponse(statusCode, HttpEntity(message), exception) + + protected def errorResponse[T <: Exception](statusCode: StatusCode, entity: ResponseEntity, exception: T): Route = { + complete(HttpResponse(statusCode, entity = entity)) } } diff --git a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala index e54f722..788729a 100644 --- a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala +++ b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala @@ -75,8 +75,11 @@ class HttpRestServiceTransport( Future.successful(Unmarshal(HttpEntity.Empty: ResponseEntity)) } else if (response.status.isFailure()) { val serviceCalled = s"${requestStub.method} ${requestStub.uri}" - Unmarshal(response.entity).to[String] flatMap { error => - Future.failed(ExternalServiceException(serviceCalled, error)) + Unmarshal(response.entity).to[String] flatMap { errorString => + import spray.json._ + import xyz.driver.core.json._ + val serviceException = util.Try(serviceExceptionFormat.read(errorString.parseJson)).toOption + Future.failed(ExternalServiceException(serviceCalled, errorString, serviceException)) } } else { Future.successful(Unmarshal(response.entity)) diff --git a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala index 095936e..db289de 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -10,7 +10,10 @@ final case class InvalidActionException(override val message: String = "This act final case class ResourceNotFoundException(override val message: String = "Resource not found") extends ServiceException(message) -final case class ExternalServiceException(serviceName: String, serviceMessage: String) +final case class ExternalServiceException( + serviceName: String, + serviceMessage: String, + serviceException: Option[ServiceException]) extends ServiceException(s"Error while calling '$serviceName': $serviceMessage") final case class ExternalServiceTimeoutException(serviceName: String) diff --git a/src/test/scala/xyz/driver/core/rest/DriverRouteTest.scala b/src/test/scala/xyz/driver/core/rest/DriverRouteTest.scala index 15693a0..aca8fdc 100644 --- a/src/test/scala/xyz/driver/core/rest/DriverRouteTest.scala +++ b/src/test/scala/xyz/driver/core/rest/DriverRouteTest.scala @@ -1,5 +1,6 @@ package xyz.driver.core.rest +import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.server.Directives.{complete => akkaComplete} import akka.http.scaladsl.server.{Directives, Route} @@ -7,11 +8,14 @@ import akka.http.scaladsl.testkit.ScalatestRouteTest import com.typesafe.scalalogging.Logger import org.scalatest.{AsyncFlatSpec, Matchers} import xyz.driver.core.logging.NoLogger +import xyz.driver.core.json.serviceExceptionFormat +import xyz.driver.core.FutureExtensions import xyz.driver.core.rest.errors._ import scala.concurrent.Future -class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matchers with Directives { +class DriverRouteTest + extends AsyncFlatSpec with ScalatestRouteTest with SprayJsonSupport with Matchers with Directives { class TestRoute(override val route: Route) extends DriverRoute { override def log: Logger = NoLogger } @@ -31,7 +35,7 @@ class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matcher Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.BadRequest - responseAs[String] shouldBe "Invalid input" + responseAs[ServiceException] shouldBe InvalidInputException() } } @@ -41,7 +45,7 @@ class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matcher Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.Forbidden - responseAs[String] shouldBe "This action is not allowed" + responseAs[ServiceException] shouldBe InvalidActionException() } } @@ -51,18 +55,31 @@ class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matcher Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.NotFound - responseAs[String] shouldBe "Resource not found" + responseAs[ServiceException] shouldBe ResourceNotFoundException() } } it should "respond with a 500 for ExternalServiceException" in { - val error = ExternalServiceException("GET /api/v1/users/", "Permission denied") + val error = ExternalServiceException("GET /api/v1/users/", "Permission denied", None) val route = new TestRoute(akkaComplete(Future.failed[String](error))) Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.InternalServerError - responseAs[String] shouldBe "Error while calling 'GET /api/v1/users/': Permission denied" + responseAs[ServiceException] shouldBe error + } + } + + it should "allow pass-through of external service exceptions" in { + val innerError = InvalidInputException() + val error = ExternalServiceException("GET /api/v1/users/", "Permission denied", Some(innerError)) + val future = Future.failed[String](error) + val route = new TestRoute(akkaComplete(future.passThroughExternalServiceException)) + + Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.BadRequest + responseAs[ServiceException] shouldBe innerError } } @@ -73,7 +90,7 @@ class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matcher Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.GatewayTimeout - responseAs[String] shouldBe "GET /api/v1/users/ took too long to respond" + responseAs[ServiceException] shouldBe error } } @@ -83,7 +100,7 @@ class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matcher Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { handled shouldBe true status shouldBe StatusCodes.InternalServerError - responseAs[String] shouldBe "Database access error" + responseAs[ServiceException] shouldBe DatabaseException() } } } -- cgit v1.2.3