From edbfe3d11eefe10f6d45752d1132e7349e1c6750 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 28 Sep 2017 10:28:55 -0700 Subject: Add DriverRoute trait and clean up DriverApp --- src/main/scala/xyz/driver/core/app/DriverApp.scala | 156 ++++++--------------- src/main/scala/xyz/driver/core/app/module.scala | 32 +++-- .../scala/xyz/driver/core/rest/DriverRoute.scala | 84 +++++++++++ .../xyz/driver/core/rest/errors/APIError.scala | 16 +++ 4 files changed, 164 insertions(+), 124 deletions(-) create mode 100644 src/main/scala/xyz/driver/core/rest/DriverRoute.scala create mode 100644 src/main/scala/xyz/driver/core/rest/errors/APIError.scala (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/app/DriverApp.scala b/src/main/scala/xyz/driver/core/app/DriverApp.scala index 901d6e2..cb3a38e 100644 --- a/src/main/scala/xyz/driver/core/app/DriverApp.scala +++ b/src/main/scala/xyz/driver/core/app/DriverApp.scala @@ -1,10 +1,8 @@ package xyz.driver.core.app -import java.sql.SQLException - import akka.actor.ActorSystem import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport -import akka.http.scaladsl.model.StatusCodes.{BadRequest, Conflict, InternalServerError, MethodNotAllowed} +import akka.http.scaladsl.model.StatusCodes.MethodNotAllowed import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.server.Directives._ @@ -27,9 +25,8 @@ import xyz.driver.core.time.provider.{SystemTimeProvider, TimeProvider} import xyz.driver.tracing.TracingDirectives.trace import xyz.driver.tracing.{NoTracer, Tracer} -import scala.compat.Platform.ConcurrentModificationException import scala.concurrent.duration._ -import scala.concurrent.{Await, ExecutionContext, Future} +import scala.concurrent.{Await, ExecutionContext} import scala.reflect.runtime.universe._ import scala.util.Try import scala.util.control.NonFatal @@ -74,69 +71,58 @@ class DriverApp(appName: String, private def extractHeader(request: HttpRequest)(headerName: String): Option[String] = request.headers.find(_.name().toLowerCase === headerName).map(_.value()) - protected def bindHttp(modules: Seq[Module]): Unit = { + protected def appRoute: Route = { val serviceTypes = modules.flatMap(_.routeTypes) val swaggerService = swaggerOverride(serviceTypes) val swaggerRoutes = swaggerService.routes ~ swaggerService.swaggerUI val versionRt = versionRoute(version, gitHash, time.currentTime()) - val _ = Future { - http.bindAndHandle( - route2HandlerFlow(extractHost { origin => - trace(tracer) { - extractClientIP { ip => - optionalHeaderValueByType[Origin](()) { - originHeader => - { - ctx => - val trackingId = rest.extractTrackingId(ctx.request) - MDC.put("trackingId", trackingId) - - val updatedStacktrace = - (rest.extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") - MDC.put("stack", updatedStacktrace) - - storeRequestContextToMdc(ctx.request, origin, ip) - - def requestLogging: Future[Unit] = Future { - log.info( - s"""Received request {"method":"${ctx.request.method.value}","url": "${ctx.request.uri}"}""") - } - - val contextWithTrackingId = - ctx.withRequest( - ctx.request - .addHeader(RawHeader(ContextHeaders.TrackingIdHeader, trackingId)) - .addHeader(RawHeader(ContextHeaders.StacktraceHeader, updatedStacktrace))) - - handleExceptions(ExceptionHandler(exceptionHandler))({ - c => - requestLogging.flatMap { _ => - val trackingHeader = RawHeader(ContextHeaders.TrackingIdHeader, trackingId) - - val responseHeaders = List[HttpHeader]( - trackingHeader, - allowOrigin(originHeader), - `Access-Control-Allow-Headers`(rest.AllowedHeaders: _*), - `Access-Control-Expose-Headers`(rest.AllowedHeaders: _*) - ) - - respondWithHeaders(responseHeaders) { - modules.map(_.route).foldLeft(versionRt ~ healthRoute ~ swaggerRoutes)(_ ~ _) - }(c) - } - })(contextWithTrackingId) - } - } - } + extractHost { origin => + extractClientIP { ip => + optionalHeaderValueByType[Origin](()) { originHeader => + trace(tracer) { ctx => + val trackingId = rest.extractTrackingId(ctx.request) + MDC.put("trackingId", trackingId) + + val updatedStacktrace = + (rest.extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") + MDC.put("stack", updatedStacktrace) + + storeRequestContextToMdc(ctx.request, origin, ip) + + val trackingHeader = RawHeader(ContextHeaders.TrackingIdHeader, trackingId) + + val responseHeaders = List[HttpHeader]( + trackingHeader, + allowOrigin(originHeader), + `Access-Control-Allow-Headers`(rest.AllowedHeaders: _*), + `Access-Control-Expose-Headers`(rest.AllowedHeaders: _*) + ) + + log.info(s"""Received request {"method":"${ctx.request.method.value}","url": "${ctx.request.uri}"}""") + + val contextWithTrackingId = + ctx.withRequest( + ctx.request + .addHeader(RawHeader(ContextHeaders.TrackingIdHeader, trackingId)) + .addHeader(RawHeader(ContextHeaders.StacktraceHeader, updatedStacktrace))) + + respondWithHeaders(responseHeaders) { + modules + .flatMap(_.routes) + .map(_.routeWithDefaults) + .foldLeft(versionRt ~ healthRoute ~ swaggerRoutes)(_ ~ _) + }(contextWithTrackingId) } - }), - interface, - port - )(materializer) + } + } } } + protected def bindHttp(modules: Seq[Module]): Unit = { + val _ = http.bindAndHandle(route2HandlerFlow(appRoute), interface, port)(materializer) + } + private def storeRequestContextToMdc(request: HttpRequest, origin: String, ip: RemoteAddress): Unit = { MDC.put("origin", origin) @@ -181,58 +167,6 @@ class DriverApp(appName: String, } } - /** - * Override me for custom exception handling - * - * @return Exception handling route for exception type - */ - protected def exceptionHandler: PartialFunction[Throwable, Route] = { - - case is: IllegalStateException => - ctx => - log.warn(s"Request is not allowed to ${ctx.request.method} ${ctx.request.uri}", is) - errorResponse(ctx, BadRequest, message = is.getMessage, is)(ctx) - - case cm: ConcurrentModificationException => - ctx => - log.warn(s"Concurrent modification of the resource ${ctx.request.method} ${ctx.request.uri}", cm) - errorResponse(ctx, Conflict, "Resource was changed concurrently, try requesting a newer version", cm)(ctx) - - case se: SQLException => - ctx => - log.warn(s"Database exception for the resource ${ctx.request.method} ${ctx.request.uri}", se) - errorResponse(ctx, InternalServerError, "Data access error", se)(ctx) - - case t: Throwable => - ctx => - log.warn(s"Request to ${ctx.request.method} ${ctx.request.uri} could not be handled normally", t) - errorResponse(ctx, InternalServerError, t.getMessage, t)(ctx) - } - - protected def errorResponse[T <: Throwable](ctx: RequestContext, - statusCode: StatusCode, - message: String, - exception: T): Route = { - - val trackingId = rest.extractTrackingId(ctx.request) - val tracingHeader = RawHeader(ContextHeaders.TrackingIdHeader, rest.extractTrackingId(ctx.request)) - - MDC.put("trackingId", trackingId) - - optionalHeaderValueByType[Origin](()) { originHeader => - val responseHeaders = List[HttpHeader]( - tracingHeader, - allowOrigin(originHeader), - `Access-Control-Allow-Headers`(rest.AllowedHeaders: _*), - `Access-Control-Expose-Headers`(rest.AllowedHeaders: _*) - ) - - respondWithHeaders(responseHeaders) { - complete(HttpResponse(statusCode, entity = message)) - } - } - } - protected def versionRoute(version: String, gitHash: String, startupTime: Time): Route = { import spray.json._ import DefaultJsonProtocol._ diff --git a/src/main/scala/xyz/driver/core/app/module.scala b/src/main/scala/xyz/driver/core/app/module.scala index c6f979f..6baa457 100644 --- a/src/main/scala/xyz/driver/core/app/module.scala +++ b/src/main/scala/xyz/driver/core/app/module.scala @@ -3,13 +3,14 @@ package xyz.driver.core.app import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.server.Directives.complete import akka.http.scaladsl.server.{Route, RouteConcatenation} -import xyz.driver.core.rest.{NoServiceDiscovery, SavingUsedServiceDiscovery, ServiceDiscovery} +import com.typesafe.scalalogging.Logger +import xyz.driver.core.rest.{DriverRoute, NoServiceDiscovery, SavingUsedServiceDiscovery, ServiceDiscovery} import scala.reflect.runtime.universe._ trait Module { val name: String - def route: Route + def routes: Seq[DriverRoute] def routeTypes: Seq[Type] val serviceDiscovery: ServiceDiscovery with SavingUsedServiceDiscovery = new NoServiceDiscovery() @@ -21,13 +22,22 @@ trait Module { class EmptyModule extends Module { override val name: String = "Nothing" - override def route: Route = complete(StatusCodes.OK) + override def routes: Seq[DriverRoute] = + Seq(new DriverRoute { + override def route: Route = complete(StatusCodes.OK) + override val log: Logger = xyz.driver.core.logging.NoLogger + }) override def routeTypes: Seq[Type] = Seq.empty[Type] } -class SimpleModule(override val name: String, override val route: Route, routeType: Type) extends Module { - def routeTypes: Seq[Type] = Seq(routeType) +class SimpleModule(override val name: String, route: Route, routeType: Type) extends Module { self => + override def routes: Seq[DriverRoute] = + Seq(new DriverRoute { + override def route: Route = self.route + override val log: Logger = xyz.driver.core.logging.NoLogger + }) + override def routeTypes: Seq[Type] = Seq(routeType) } /** @@ -39,12 +49,8 @@ class SimpleModule(override val name: String, override val route: Route, routeTy * @param modules modules to compose into a single one */ class CompositeModule(override val name: String, modules: Seq[Module]) extends Module with RouteConcatenation { - - override def route: Route = RouteConcatenation.concat(modules.map(_.route): _*) - - override def routeTypes: Seq[Type] = modules.flatMap(_.routeTypes) - - override def activate(): Unit = modules.foreach(_.activate()) - - override def deactivate(): Unit = modules.reverse.foreach(_.deactivate()) + override def routes: Seq[DriverRoute] = modules.flatMap(_.routes) + override def routeTypes: Seq[Type] = modules.flatMap(_.routeTypes) + override def activate(): Unit = modules.foreach(_.activate()) + override def deactivate(): Unit = modules.reverse.foreach(_.deactivate()) } diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala new file mode 100644 index 0000000..20cc556 --- /dev/null +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -0,0 +1,84 @@ +package xyz.driver.core.rest + +import java.sql.SQLException + +import akka.http.scaladsl.model._ +import akka.http.scaladsl.model.StatusCodes.{BadRequest, Conflict, InternalServerError} +import akka.http.scaladsl.model.headers._ +import akka.http.scaladsl.server.Directives._ +import akka.http.scaladsl.server.{ExceptionHandler, RequestContext, Route} +import com.typesafe.scalalogging.Logger +import org.slf4j.MDC +import xyz.driver.core.rest +import xyz.driver.core.rest.errors.APIError + +import scala.compat.Platform.ConcurrentModificationException + +trait DriverRoute { + val log: Logger + + def route: Route + + def routeWithDefaults: Route = handleExceptions(ExceptionHandler(exceptionHandler)) { + route + } + + /** + * Override me for custom exception handling + * + * @return Exception handling route for exception type + */ + protected def exceptionHandler: PartialFunction[Throwable, Route] = { + case api: APIError if api.isPatientSensitive => + ctx => + log.info("PHI Sensitive error") + errorResponse(ctx, InternalServerError, "Server error", api)(ctx) + + case api: APIError => + ctx => + log.info("API Error") + errorResponse(ctx, api.statusCode, api.message, api)(ctx) + + case is: IllegalStateException => + ctx => + log.warn(s"Request is not allowed to ${ctx.request.method} ${ctx.request.uri}", is) + errorResponse(ctx, BadRequest, message = is.getMessage, is)(ctx) + + case cm: ConcurrentModificationException => + ctx => + log.warn(s"Concurrent modification of the resource ${ctx.request.method} ${ctx.request.uri}", cm) + errorResponse(ctx, Conflict, "Resource was changed concurrently, try requesting a newer version", cm)(ctx) + + case se: SQLException => + ctx => + log.warn(s"Database exception for the resource ${ctx.request.method} ${ctx.request.uri}", se) + errorResponse(ctx, InternalServerError, "Data access error", se)(ctx) + + case t: Throwable => + ctx => + log.warn(s"Request to ${ctx.request.method} ${ctx.request.uri} could not be handled normally", t) + errorResponse(ctx, InternalServerError, t.getMessage, t)(ctx) + } + + protected def errorResponse[T <: Throwable](ctx: RequestContext, + statusCode: StatusCode, + message: String, + exception: T): Route = { + + val trackingId = rest.extractTrackingId(ctx.request) + val tracingHeader = RawHeader(ContextHeaders.TrackingIdHeader, rest.extractTrackingId(ctx.request)) + + MDC.put("trackingId", trackingId) + + optionalHeaderValueByType[Origin](()) { originHeader => + val responseHeaders = List[HttpHeader](tracingHeader, + allowOrigin(originHeader), + `Access-Control-Allow-Headers`(AllowedHeaders: _*), + `Access-Control-Expose-Headers`(AllowedHeaders: _*)) + + respondWithHeaders(responseHeaders) { + complete(HttpResponse(statusCode, entity = message)) + } + } + } +} diff --git a/src/main/scala/xyz/driver/core/rest/errors/APIError.scala b/src/main/scala/xyz/driver/core/rest/errors/APIError.scala new file mode 100644 index 0000000..f2bfae1 --- /dev/null +++ b/src/main/scala/xyz/driver/core/rest/errors/APIError.scala @@ -0,0 +1,16 @@ +package xyz.driver.core.rest.errors + +import akka.http.scaladsl.model.{StatusCode, StatusCodes} + +abstract class APIError extends Throwable { + def isPatientSensitive: Boolean = false + + def statusCode: StatusCode + def message: String +} + +final case class InvalidInputError(override val message: String = "Invalid input", + override val isPatientSensitive: Boolean = false) + extends APIError { + override def statusCode: StatusCode = StatusCodes.BadRequest +} -- cgit v1.2.3 From 4715cdab47febe8bb75c81d45d9619fc224477bc Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 12 Oct 2017 18:17:36 -0700 Subject: Add more API Error types --- .../xyz/driver/core/rest/errors/APIError.scala | 24 ++++++++++++++++++++++ 1 file changed, 24 insertions(+) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/errors/APIError.scala b/src/main/scala/xyz/driver/core/rest/errors/APIError.scala index f2bfae1..e0400fb 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/APIError.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/APIError.scala @@ -14,3 +14,27 @@ final case class InvalidInputError(override val message: String = "Invalid input extends APIError { override def statusCode: StatusCode = StatusCodes.BadRequest } + +final case class InvalidActionError(override val message: String = "This action is not allowed", + override val isPatientSensitive: Boolean = false) + extends APIError { + override def statusCode: StatusCode = StatusCodes.Forbidden +} + +final case class ResourceNotFoundError(override val message: String = "Resource not found", + override val isPatientSensitive: Boolean = false) + extends APIError { + override def statusCode: StatusCode = StatusCodes.NotFound +} + +final case class ExternalServiceTimeoutError(override val message: String = "Another service took too long to respond", + override val isPatientSensitive: Boolean = false) + extends APIError { + override def statusCode: StatusCode = StatusCodes.GatewayTimeout +} + +final case class DatabaseError(override val message: String = "Database access error", + override val isPatientSensitive: Boolean = false) + extends APIError { + override def statusCode: StatusCode = StatusCodes.InternalServerError +} -- cgit v1.2.3 From e5911d0f0fa382f37a22978a5bae1d6c4b47963f Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 17 Oct 2017 14:57:31 -0700 Subject: Rename APIError -> ServiceException and subclasses --- .../scala/xyz/driver/core/rest/DriverRoute.scala | 6 ++-- .../xyz/driver/core/rest/errors/APIError.scala | 40 --------------------- .../driver/core/rest/errors/serviceException.scala | 41 ++++++++++++++++++++++ 3 files changed, 44 insertions(+), 43 deletions(-) delete mode 100644 src/main/scala/xyz/driver/core/rest/errors/APIError.scala create mode 100644 src/main/scala/xyz/driver/core/rest/errors/serviceException.scala (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index 20cc556..f3260d0 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -10,7 +10,7 @@ import akka.http.scaladsl.server.{ExceptionHandler, RequestContext, Route} import com.typesafe.scalalogging.Logger import org.slf4j.MDC import xyz.driver.core.rest -import xyz.driver.core.rest.errors.APIError +import xyz.driver.core.rest.errors.ServiceException import scala.compat.Platform.ConcurrentModificationException @@ -29,12 +29,12 @@ trait DriverRoute { * @return Exception handling route for exception type */ protected def exceptionHandler: PartialFunction[Throwable, Route] = { - case api: APIError if api.isPatientSensitive => + case api: ServiceException if api.isPatientSensitive => ctx => log.info("PHI Sensitive error") errorResponse(ctx, InternalServerError, "Server error", api)(ctx) - case api: APIError => + case api: ServiceException => ctx => log.info("API Error") errorResponse(ctx, api.statusCode, api.message, api)(ctx) diff --git a/src/main/scala/xyz/driver/core/rest/errors/APIError.scala b/src/main/scala/xyz/driver/core/rest/errors/APIError.scala deleted file mode 100644 index e0400fb..0000000 --- a/src/main/scala/xyz/driver/core/rest/errors/APIError.scala +++ /dev/null @@ -1,40 +0,0 @@ -package xyz.driver.core.rest.errors - -import akka.http.scaladsl.model.{StatusCode, StatusCodes} - -abstract class APIError extends Throwable { - def isPatientSensitive: Boolean = false - - def statusCode: StatusCode - def message: String -} - -final case class InvalidInputError(override val message: String = "Invalid input", - override val isPatientSensitive: Boolean = false) - extends APIError { - override def statusCode: StatusCode = StatusCodes.BadRequest -} - -final case class InvalidActionError(override val message: String = "This action is not allowed", - override val isPatientSensitive: Boolean = false) - extends APIError { - override def statusCode: StatusCode = StatusCodes.Forbidden -} - -final case class ResourceNotFoundError(override val message: String = "Resource not found", - override val isPatientSensitive: Boolean = false) - extends APIError { - override def statusCode: StatusCode = StatusCodes.NotFound -} - -final case class ExternalServiceTimeoutError(override val message: String = "Another service took too long to respond", - override val isPatientSensitive: Boolean = false) - extends APIError { - override def statusCode: StatusCode = StatusCodes.GatewayTimeout -} - -final case class DatabaseError(override val message: String = "Database access error", - override val isPatientSensitive: Boolean = false) - extends APIError { - override def statusCode: StatusCode = StatusCodes.InternalServerError -} diff --git a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala new file mode 100644 index 0000000..94f9734 --- /dev/null +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -0,0 +1,41 @@ +package xyz.driver.core.rest.errors + +import akka.http.scaladsl.model.{StatusCode, StatusCodes} + +abstract class ServiceException extends Exception { + def isPatientSensitive: Boolean = false + + def statusCode: StatusCode + def message: String +} + +final case class InvalidInputException(override val message: String = "Invalid input", + override val isPatientSensitive: Boolean = false) + extends ServiceException { + override def statusCode: StatusCode = StatusCodes.BadRequest +} + +final case class InvalidActionException(override val message: String = "This action is not allowed", + override val isPatientSensitive: Boolean = false) + extends ServiceException { + override def statusCode: StatusCode = StatusCodes.Forbidden +} + +final case class ResourceNotFoundException(override val message: String = "Resource not found", + override val isPatientSensitive: Boolean = false) + extends ServiceException { + override def statusCode: StatusCode = StatusCodes.NotFound +} + +final case class ExternalServiceTimeoutException(override val message: String = + "Another service took too long to respond", + override val isPatientSensitive: Boolean = false) + extends ServiceException { + override def statusCode: StatusCode = StatusCodes.GatewayTimeout +} + +final case class DatabaseException(override val message: String = "Database access error", + override val isPatientSensitive: Boolean = false) + extends ServiceException { + override def statusCode: StatusCode = StatusCodes.InternalServerError +} -- cgit v1.2.3 From 2e2d4df8749c02df20c29a36090daa1a76344656 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 17 Oct 2017 15:27:51 -0700 Subject: Remove unnecessary terminated val --- src/main/scala/xyz/driver/core/app/DriverApp.scala | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/app/DriverApp.scala b/src/main/scala/xyz/driver/core/app/DriverApp.scala index cb3a38e..8f2cf70 100644 --- a/src/main/scala/xyz/driver/core/app/DriverApp.scala +++ b/src/main/scala/xyz/driver/core/app/DriverApp.scala @@ -2,15 +2,15 @@ package xyz.driver.core.app import akka.actor.ActorSystem import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport -import akka.http.scaladsl.model.StatusCodes.MethodNotAllowed +import akka.http.scaladsl.model.StatusCodes._ import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.server.Directives._ -import akka.http.scaladsl.server.RouteResult.route2HandlerFlow +import akka.http.scaladsl.server.RouteResult._ import akka.http.scaladsl.server._ import akka.http.scaladsl.{Http, HttpExt} import akka.stream.ActorMaterializer -import com.github.swagger.akka.SwaggerHttpService.{logger, toJavaTypeSet} +import com.github.swagger.akka.SwaggerHttpService._ import com.typesafe.config.Config import com.typesafe.scalalogging.Logger import io.swagger.models.Scheme @@ -18,12 +18,12 @@ import io.swagger.util.Json import org.slf4j.{LoggerFactory, MDC} import xyz.driver.core import xyz.driver.core.rest -import xyz.driver.core.rest.{ContextHeaders, Swagger} +import xyz.driver.core.rest._ import xyz.driver.core.stats.SystemStats import xyz.driver.core.time.Time import xyz.driver.core.time.provider.{SystemTimeProvider, TimeProvider} -import xyz.driver.tracing.TracingDirectives.trace -import xyz.driver.tracing.{NoTracer, Tracer} +import xyz.driver.tracing.TracingDirectives._ +import xyz.driver.tracing._ import scala.concurrent.duration._ import scala.concurrent.{Await, ExecutionContext} @@ -61,8 +61,7 @@ class DriverApp(appName: String, def stop(): Unit = { http.shutdownAllConnectionPools().onComplete { _ => Await.result(tracer.close(), 15.seconds) // flush out any remaining traces from the buffer - val _ = actorSystem.terminate() - val terminated = Await.result(actorSystem.whenTerminated, 30.seconds) + val terminated = Await.result(actorSystem.terminate(), 30.seconds) val addressTerminated = if (terminated.addressTerminated) "is" else "is not" Console.print(s"${this.getClass.getName} App $addressTerminated stopped ") } -- cgit v1.2.3 From e7d849b55f975f1c119cebb22f95d9de903ae3c3 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Thu, 19 Oct 2017 13:49:22 -0700 Subject: Stop catching throwable, remove PHI filtering, move status code logic to exception handler --- .../scala/xyz/driver/core/rest/DriverRoute.scala | 54 +++++++++++++++------- .../driver/core/rest/errors/serviceException.scala | 40 ++++------------ 2 files changed, 46 insertions(+), 48 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index f3260d0..be9c783 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -3,14 +3,14 @@ package xyz.driver.core.rest import java.sql.SQLException import akka.http.scaladsl.model._ -import akka.http.scaladsl.model.StatusCodes.{BadRequest, Conflict, InternalServerError} +import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server.{ExceptionHandler, RequestContext, Route} import com.typesafe.scalalogging.Logger import org.slf4j.MDC import xyz.driver.core.rest -import xyz.driver.core.rest.errors.ServiceException +import xyz.driver.core.rest.errors._ import scala.compat.Platform.ConcurrentModificationException @@ -29,38 +29,58 @@ trait DriverRoute { * @return Exception handling route for exception type */ protected def exceptionHandler: PartialFunction[Throwable, Route] = { - case api: ServiceException if api.isPatientSensitive => - ctx => - log.info("PHI Sensitive error") - errorResponse(ctx, InternalServerError, "Server error", api)(ctx) - - case api: ServiceException => - ctx => - log.info("API Error") - errorResponse(ctx, api.statusCode, api.message, api)(ctx) + case serviceException: ServiceException => + serviceExceptionHandler(serviceException) case is: IllegalStateException => ctx => log.warn(s"Request is not allowed to ${ctx.request.method} ${ctx.request.uri}", is) - errorResponse(ctx, BadRequest, message = is.getMessage, is)(ctx) + errorResponse(ctx, StatusCodes.BadRequest, message = is.getMessage, is)(ctx) case cm: ConcurrentModificationException => ctx => log.warn(s"Concurrent modification of the resource ${ctx.request.method} ${ctx.request.uri}", cm) - errorResponse(ctx, Conflict, "Resource was changed concurrently, try requesting a newer version", cm)(ctx) + errorResponse(ctx, + StatusCodes.Conflict, + "Resource was changed concurrently, try requesting a newer version", + cm)(ctx) case se: SQLException => ctx => log.warn(s"Database exception for the resource ${ctx.request.method} ${ctx.request.uri}", se) - errorResponse(ctx, InternalServerError, "Data access error", se)(ctx) + errorResponse(ctx, StatusCodes.InternalServerError, "Data access error", se)(ctx) - case t: Throwable => + case t: Exception => ctx => log.warn(s"Request to ${ctx.request.method} ${ctx.request.uri} could not be handled normally", t) - errorResponse(ctx, InternalServerError, t.getMessage, t)(ctx) + errorResponse(ctx, StatusCodes.InternalServerError, t.getMessage, t)(ctx) + } + + protected def serviceExceptionHandler(serviceException: ServiceException): Route = { + val statusCode = serviceException match { + case e: InvalidInputException => + log.info("Invalid client input error", e) + StatusCodes.BadRequest + case e: InvalidActionException => + log.info("Invalid client action error", e) + StatusCodes.Forbidden + case e: ResourceNotFoundException => + log.info("Resource not found error", e) + StatusCodes.NotFound + case e: ExternalServiceTimeoutException => + log.error("Service timeout error", e) + StatusCodes.GatewayTimeout + case e: DatabaseException => + log.error("Database error", e) + StatusCodes.InternalServerError + } + + { (ctx: RequestContext) => + errorResponse(ctx, statusCode, serviceException.message, serviceException)(ctx) + } } - protected def errorResponse[T <: Throwable](ctx: RequestContext, + protected def errorResponse[T <: Exception](ctx: RequestContext, statusCode: StatusCode, message: String, exception: T): Route = { 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 94f9734..7aa70bf 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -1,41 +1,19 @@ package xyz.driver.core.rest.errors -import akka.http.scaladsl.model.{StatusCode, StatusCodes} - abstract class ServiceException extends Exception { - def isPatientSensitive: Boolean = false - - def statusCode: StatusCode def message: String } -final case class InvalidInputException(override val message: String = "Invalid input", - override val isPatientSensitive: Boolean = false) - extends ServiceException { - override def statusCode: StatusCode = StatusCodes.BadRequest -} +final case class InvalidInputException(override val message: String = "Invalid input") extends ServiceException -final case class InvalidActionException(override val message: String = "This action is not allowed", - override val isPatientSensitive: Boolean = false) - extends ServiceException { - override def statusCode: StatusCode = StatusCodes.Forbidden -} +final case class InvalidActionException(override val message: String = "This action is not allowed") + extends ServiceException -final case class ResourceNotFoundException(override val message: String = "Resource not found", - override val isPatientSensitive: Boolean = false) - extends ServiceException { - override def statusCode: StatusCode = StatusCodes.NotFound -} +final case class ResourceNotFoundException(override val message: String = "Resource not found") + extends ServiceException -final case class ExternalServiceTimeoutException(override val message: String = - "Another service took too long to respond", - override val isPatientSensitive: Boolean = false) - extends ServiceException { - override def statusCode: StatusCode = StatusCodes.GatewayTimeout -} +final case class ExternalServiceTimeoutException( + override val message: String = "Another service took too long to respond") + extends ServiceException -final case class DatabaseException(override val message: String = "Database access error", - override val isPatientSensitive: Boolean = false) - extends ServiceException { - override def statusCode: StatusCode = StatusCodes.InternalServerError -} +final case class DatabaseException(override val message: String = "Database access error") extends ServiceException -- cgit v1.2.3 From 0ba232ad3d3752e2cdc585de727f0506d598d50d Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 24 Oct 2017 14:34:26 -0700 Subject: Change Module back to having a simple route: Route method --- src/main/scala/xyz/driver/core/app/DriverApp.scala | 3 +- src/main/scala/xyz/driver/core/app/module.scala | 32 ++++++++++------------ 2 files changed, 15 insertions(+), 20 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/app/DriverApp.scala b/src/main/scala/xyz/driver/core/app/DriverApp.scala index 8f2cf70..4110c37 100644 --- a/src/main/scala/xyz/driver/core/app/DriverApp.scala +++ b/src/main/scala/xyz/driver/core/app/DriverApp.scala @@ -108,8 +108,7 @@ class DriverApp(appName: String, respondWithHeaders(responseHeaders) { modules - .flatMap(_.routes) - .map(_.routeWithDefaults) + .map(_.route) .foldLeft(versionRt ~ healthRoute ~ swaggerRoutes)(_ ~ _) }(contextWithTrackingId) } diff --git a/src/main/scala/xyz/driver/core/app/module.scala b/src/main/scala/xyz/driver/core/app/module.scala index 6baa457..bbb29f4 100644 --- a/src/main/scala/xyz/driver/core/app/module.scala +++ b/src/main/scala/xyz/driver/core/app/module.scala @@ -10,7 +10,7 @@ import scala.reflect.runtime.universe._ trait Module { val name: String - def routes: Seq[DriverRoute] + def route: Route def routeTypes: Seq[Type] val serviceDiscovery: ServiceDiscovery with SavingUsedServiceDiscovery = new NoServiceDiscovery() @@ -22,26 +22,22 @@ trait Module { class EmptyModule extends Module { override val name: String = "Nothing" - override def routes: Seq[DriverRoute] = - Seq(new DriverRoute { - override def route: Route = complete(StatusCodes.OK) - override val log: Logger = xyz.driver.core.logging.NoLogger - }) - + override def route: Route = complete(StatusCodes.OK) override def routeTypes: Seq[Type] = Seq.empty[Type] } -class SimpleModule(override val name: String, route: Route, routeType: Type) extends Module { self => - override def routes: Seq[DriverRoute] = - Seq(new DriverRoute { - override def route: Route = self.route - override val log: Logger = xyz.driver.core.logging.NoLogger - }) +class SimpleModule(override val name: String, theRoute: Route, routeType: Type) extends Module { + private val driverRoute: DriverRoute = new DriverRoute { + override def route: Route = theRoute + override val log: Logger = xyz.driver.core.logging.NoLogger + } + + override def route: Route = driverRoute.routeWithDefaults override def routeTypes: Seq[Type] = Seq(routeType) } /** - * Module implementation which may be used to composed a few + * Module implementation which may be used to compose multiple modules * * @param name more general name of the composite module, * must be provided as there is no good way to automatically @@ -49,8 +45,8 @@ class SimpleModule(override val name: String, route: Route, routeType: Type) ext * @param modules modules to compose into a single one */ class CompositeModule(override val name: String, modules: Seq[Module]) extends Module with RouteConcatenation { - override def routes: Seq[DriverRoute] = modules.flatMap(_.routes) - override def routeTypes: Seq[Type] = modules.flatMap(_.routeTypes) - override def activate(): Unit = modules.foreach(_.activate()) - override def deactivate(): Unit = modules.reverse.foreach(_.deactivate()) + override def route: Route = RouteConcatenation.concat(modules.map(_.route): _*) + override def routeTypes: Seq[Type] = modules.flatMap(_.routeTypes) + override def activate(): Unit = modules.foreach(_.activate()) + override def deactivate(): Unit = modules.reverse.foreach(_.deactivate()) } -- cgit v1.2.3 From 473d2586c20d35fbb56e11da816140ae40bcb463 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 24 Oct 2017 14:35:23 -0700 Subject: Change DriverRoute log val to def --- src/main/scala/xyz/driver/core/rest/DriverRoute.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index be9c783..79dff8b 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -15,7 +15,7 @@ import xyz.driver.core.rest.errors._ import scala.compat.Platform.ConcurrentModificationException trait DriverRoute { - val log: Logger + def log: Logger def route: Route -- cgit v1.2.3 From 5db157103a1982f00ca72e8f6b925344debce36e Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Wed, 25 Oct 2017 13:34:46 -0700 Subject: Move all CORS headers to DriverRoute from DriverApp --- src/main/scala/xyz/driver/core/app/DriverApp.scala | 52 ++++++++-------------- .../scala/xyz/driver/core/rest/DriverRoute.scala | 21 +++++++-- 2 files changed, 36 insertions(+), 37 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/app/DriverApp.scala b/src/main/scala/xyz/driver/core/app/DriverApp.scala index 4110c37..2bb61c0 100644 --- a/src/main/scala/xyz/driver/core/app/DriverApp.scala +++ b/src/main/scala/xyz/driver/core/app/DriverApp.scala @@ -73,47 +73,31 @@ class DriverApp(appName: String, protected def appRoute: Route = { val serviceTypes = modules.flatMap(_.routeTypes) val swaggerService = swaggerOverride(serviceTypes) - val swaggerRoutes = swaggerService.routes ~ swaggerService.swaggerUI + val swaggerRoute = swaggerService.routes ~ swaggerService.swaggerUI val versionRt = versionRoute(version, gitHash, time.currentTime()) + val combinedRoute = modules.map(_.route).foldLeft(versionRt ~ healthRoute ~ swaggerRoute)(_ ~ _) - extractHost { origin => - extractClientIP { ip => - optionalHeaderValueByType[Origin](()) { originHeader => - trace(tracer) { ctx => - val trackingId = rest.extractTrackingId(ctx.request) - MDC.put("trackingId", trackingId) + (extractHost & extractClientIP & trace(tracer)) { + case (origin, ip) => + ctx => + val trackingId = rest.extractTrackingId(ctx.request) + MDC.put("trackingId", trackingId) - val updatedStacktrace = - (rest.extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") - MDC.put("stack", updatedStacktrace) + val updatedStacktrace = + (rest.extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") + MDC.put("stack", updatedStacktrace) - storeRequestContextToMdc(ctx.request, origin, ip) + storeRequestContextToMdc(ctx.request, origin, ip) - val trackingHeader = RawHeader(ContextHeaders.TrackingIdHeader, trackingId) + log.info(s"""Received request {"method":"${ctx.request.method.value}","url": "${ctx.request.uri}"}""") - val responseHeaders = List[HttpHeader]( - trackingHeader, - allowOrigin(originHeader), - `Access-Control-Allow-Headers`(rest.AllowedHeaders: _*), - `Access-Control-Expose-Headers`(rest.AllowedHeaders: _*) - ) - - log.info(s"""Received request {"method":"${ctx.request.method.value}","url": "${ctx.request.uri}"}""") + val contextWithTrackingId = + ctx.withRequest( + ctx.request + .addHeader(RawHeader(ContextHeaders.TrackingIdHeader, trackingId)) + .addHeader(RawHeader(ContextHeaders.StacktraceHeader, updatedStacktrace))) - val contextWithTrackingId = - ctx.withRequest( - ctx.request - .addHeader(RawHeader(ContextHeaders.TrackingIdHeader, trackingId)) - .addHeader(RawHeader(ContextHeaders.StacktraceHeader, updatedStacktrace))) - - respondWithHeaders(responseHeaders) { - modules - .map(_.route) - .foldLeft(versionRt ~ healthRoute ~ swaggerRoutes)(_ ~ _) - }(contextWithTrackingId) - } - } - } + combinedRoute(contextWithTrackingId) } } diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index 79dff8b..d42bd75 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -6,7 +6,7 @@ import akka.http.scaladsl.model._ import akka.http.scaladsl.model.StatusCodes import akka.http.scaladsl.model.headers._ import akka.http.scaladsl.server.Directives._ -import akka.http.scaladsl.server.{ExceptionHandler, RequestContext, Route} +import akka.http.scaladsl.server.{Directive0, ExceptionHandler, RequestContext, Route} import com.typesafe.scalalogging.Logger import org.slf4j.MDC import xyz.driver.core.rest @@ -19,8 +19,23 @@ trait DriverRoute { def route: Route - def routeWithDefaults: Route = handleExceptions(ExceptionHandler(exceptionHandler)) { - route + def routeWithDefaults: Route = { + (defaultResponseHeaders & handleExceptions(ExceptionHandler(exceptionHandler)))(route) + } + + protected def defaultResponseHeaders: Directive0 = { + (extractRequest & optionalHeaderValueByType[Origin](())) tflatMap { + case (request, originHeader) => + val tracingHeader = RawHeader(ContextHeaders.TrackingIdHeader, rest.extractTrackingId(request)) + val responseHeaders = List[HttpHeader]( + tracingHeader, + allowOrigin(originHeader), + `Access-Control-Allow-Headers`(AllowedHeaders: _*), + `Access-Control-Expose-Headers`(AllowedHeaders: _*) + ) + + respondWithHeaders(responseHeaders) + } } /** -- cgit v1.2.3 From d28544ead7aefc6c2ed1fe0b9a0f75fac25c81d6 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Wed, 25 Oct 2017 16:41:04 -0700 Subject: Remove duplicate CORS header code from errorResponse method --- src/main/scala/xyz/driver/core/rest/DriverRoute.scala | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index d42bd75..9af6657 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -99,21 +99,8 @@ trait DriverRoute { statusCode: StatusCode, message: String, exception: T): Route = { - - val trackingId = rest.extractTrackingId(ctx.request) - val tracingHeader = RawHeader(ContextHeaders.TrackingIdHeader, rest.extractTrackingId(ctx.request)) - + val trackingId = rest.extractTrackingId(ctx.request) MDC.put("trackingId", trackingId) - - optionalHeaderValueByType[Origin](()) { originHeader => - val responseHeaders = List[HttpHeader](tracingHeader, - allowOrigin(originHeader), - `Access-Control-Allow-Headers`(AllowedHeaders: _*), - `Access-Control-Expose-Headers`(AllowedHeaders: _*)) - - respondWithHeaders(responseHeaders) { - complete(HttpResponse(statusCode, entity = message)) - } - } + complete(HttpResponse(statusCode, entity = message)) } } -- cgit v1.2.3 From 5f3330ffd4df8d87b97d88789aced1b1b8f7410d Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Mon, 30 Oct 2017 20:27:22 -0700 Subject: Add ExternalServiceException and use in ServiceTransport --- .../core/rest/HttpRestServiceTransport.scala | 24 ++++++++++++++++------ .../driver/core/rest/errors/serviceException.scala | 4 ++++ src/main/scala/xyz/driver/core/rest/package.scala | 4 +++- 3 files changed, 25 insertions(+), 7 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala index 1e95811..2bec4c3 100644 --- a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala +++ b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala @@ -4,9 +4,12 @@ import akka.actor.ActorSystem import akka.http.scaladsl.model._ import akka.http.scaladsl.model.headers.RawHeader import akka.http.scaladsl.unmarshalling.Unmarshal +import akka.stream.Materializer +import akka.stream.scaladsl.TcpIdleTimeoutException import com.typesafe.scalalogging.Logger import org.slf4j.MDC import xyz.driver.core.Name +import xyz.driver.core.rest.errors.{ExternalServiceException, ExternalServiceTimeoutException} import xyz.driver.core.time.provider.TimeProvider import scala.concurrent.{ExecutionContext, Future} @@ -55,18 +58,27 @@ class HttpRestServiceTransport(applicationName: Name[App], log.warn(s"Failed to receive response from ${request.method} ${request.uri} in $responseLatency ms", t) }(executionContext) - response + response.transformWith { + case Success(r) => Future.successful(r) + case Failure(_: TcpIdleTimeoutException) => + Future.failed(ExternalServiceTimeoutException()) + case Failure(t: Throwable) => Future.failed(t) + } } - def sendRequest(context: ServiceRequestContext)(requestStub: HttpRequest): Future[Unmarshal[ResponseEntity]] = { + def sendRequest(context: ServiceRequestContext)(requestStub: HttpRequest)( + implicit mat: Materializer): Future[Unmarshal[ResponseEntity]] = { - sendRequestGetResponse(context)(requestStub) map { response => + sendRequestGetResponse(context)(requestStub) flatMap { response => if (response.status == StatusCodes.NotFound) { - Unmarshal(HttpEntity.Empty: ResponseEntity) + Future.successful(Unmarshal(HttpEntity.Empty: ResponseEntity)) } else if (response.status.isFailure()) { - throw new Exception(s"Http status is failure ${response.status} for ${requestStub.method} ${requestStub.uri}") + val serviceCalled = s"${requestStub.method} ${requestStub.uri}" + Unmarshal(response.entity).to[String] flatMap { error => + Future.failed(ExternalServiceException(serviceCalled, error)) + } } else { - Unmarshal(response.entity) + 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 7aa70bf..82a1838 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -12,6 +12,10 @@ final case class InvalidActionException(override val message: String = "This act final case class ResourceNotFoundException(override val message: String = "Resource not found") extends ServiceException +final case class ExternalServiceException(serviceName: String, serviceMessage: String) extends ServiceException { + override def message = s"Error while calling another service: $serviceMessage" +} + final case class ExternalServiceTimeoutException( override val message: String = "Another service took too long to respond") extends ServiceException diff --git a/src/main/scala/xyz/driver/core/rest/package.scala b/src/main/scala/xyz/driver/core/rest/package.scala index 942ca3a..531cd8a 100644 --- a/src/main/scala/xyz/driver/core/rest/package.scala +++ b/src/main/scala/xyz/driver/core/rest/package.scala @@ -6,6 +6,7 @@ import akka.http.scaladsl.model.{HttpRequest, HttpResponse, ResponseEntity, Stat import akka.http.scaladsl.server.Directives._ import akka.http.scaladsl.server._ import akka.http.scaladsl.unmarshalling.Unmarshal +import akka.stream.Materializer import akka.stream.scaladsl.Flow import akka.util.ByteString import xyz.driver.tracing.TracingDirectives @@ -25,7 +26,8 @@ trait ServiceTransport { def sendRequestGetResponse(context: ServiceRequestContext)(requestStub: HttpRequest): Future[HttpResponse] - def sendRequest(context: ServiceRequestContext)(requestStub: HttpRequest): Future[Unmarshal[ResponseEntity]] + def sendRequest(context: ServiceRequestContext)(requestStub: HttpRequest)( + implicit mat: Materializer): Future[Unmarshal[ResponseEntity]] } final case class Pagination(pageSize: Int, pageNumber: Int) -- cgit v1.2.3 From c4fc9d9496ba382eb371a65860caf2cd49261a06 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 31 Oct 2017 09:09:13 -0700 Subject: Change transformWith to recoverWith --- .../scala/xyz/driver/core/rest/HttpRestServiceTransport.scala | 10 +++++----- .../scala/xyz/driver/core/rest/errors/serviceException.scala | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala index 2bec4c3..376b154 100644 --- a/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala +++ b/src/main/scala/xyz/driver/core/rest/HttpRestServiceTransport.scala @@ -58,11 +58,11 @@ class HttpRestServiceTransport(applicationName: Name[App], log.warn(s"Failed to receive response from ${request.method} ${request.uri} in $responseLatency ms", t) }(executionContext) - response.transformWith { - case Success(r) => Future.successful(r) - case Failure(_: TcpIdleTimeoutException) => - Future.failed(ExternalServiceTimeoutException()) - case Failure(t: Throwable) => Future.failed(t) + response.recoverWith { + case _: TcpIdleTimeoutException => + val serviceCalled = s"${requestStub.method} ${requestStub.uri}" + Future.failed(ExternalServiceTimeoutException(serviceCalled)) + case t: Throwable => Future.failed(t) } } 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 82a1838..ca1f759 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -16,8 +16,8 @@ final case class ExternalServiceException(serviceName: String, serviceMessage: S override def message = s"Error while calling another service: $serviceMessage" } -final case class ExternalServiceTimeoutException( - override val message: String = "Another service took too long to respond") - extends ServiceException +final case class ExternalServiceTimeoutException(serviceName: String) extends ServiceException { + override def message = s"$serviceName took too long to respond" +} final case class DatabaseException(override val message: String = "Database access error") extends ServiceException -- cgit v1.2.3 From 2c80cea86c0c322aad86303e5da5e2c382b20871 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Tue, 31 Oct 2017 09:57:40 -0700 Subject: Remove duplicate allowOrigin function --- src/main/scala/xyz/driver/core/app/DriverApp.scala | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/app/DriverApp.scala b/src/main/scala/xyz/driver/core/app/DriverApp.scala index 2bb61c0..751bef7 100644 --- a/src/main/scala/xyz/driver/core/app/DriverApp.scala +++ b/src/main/scala/xyz/driver/core/app/DriverApp.scala @@ -17,7 +17,6 @@ import io.swagger.models.Scheme import io.swagger.util.Json import org.slf4j.{LoggerFactory, MDC} import xyz.driver.core -import xyz.driver.core.rest import xyz.driver.core.rest._ import xyz.driver.core.stats.SystemStats import xyz.driver.core.time.Time @@ -80,11 +79,11 @@ class DriverApp(appName: String, (extractHost & extractClientIP & trace(tracer)) { case (origin, ip) => ctx => - val trackingId = rest.extractTrackingId(ctx.request) + val trackingId = extractTrackingId(ctx.request) MDC.put("trackingId", trackingId) val updatedStacktrace = - (rest.extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") + (extractStacktrace(ctx.request) ++ Array(appName)).mkString("->") MDC.put("stack", updatedStacktrace) storeRequestContextToMdc(ctx.request, origin, ip) @@ -250,11 +249,6 @@ class DriverApp(appName: String, } object DriverApp { - - private def allowOrigin(originHeader: Option[Origin]) = - `Access-Control-Allow-Origin`( - originHeader.fold[HttpOriginRange](HttpOriginRange.*)(h => HttpOriginRange(h.origins: _*))) - implicit def rejectionHandler: RejectionHandler = RejectionHandler .newBuilder() @@ -268,8 +262,8 @@ object DriverApp { Allow(methods), `Access-Control-Allow-Methods`(methods), allowOrigin(originHeader), - `Access-Control-Allow-Headers`(rest.AllowedHeaders: _*), - `Access-Control-Expose-Headers`(rest.AllowedHeaders: _*) + `Access-Control-Allow-Headers`(AllowedHeaders: _*), + `Access-Control-Expose-Headers`(AllowedHeaders: _*) )) { complete(s"Supported methods: $names.") } @@ -278,5 +272,4 @@ object DriverApp { complete(MethodNotAllowed -> s"HTTP method not allowed, supported methods: $names!") } .result() - } -- cgit v1.2.3 From 595d199f5e41c8e48131cec98b23452bc7ed6ef1 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Wed, 1 Nov 2017 09:24:16 -0700 Subject: Add DriverRouteTest --- .../scala/xyz/driver/core/rest/DriverRoute.scala | 3 + .../driver/core/rest/errors/serviceException.scala | 4 +- src/test/scala/xyz/driver/core/RestTest.scala | 16 ---- .../xyz/driver/core/rest/DriverRouteTest.scala | 89 ++++++++++++++++++++++ src/test/scala/xyz/driver/core/rest/RestTest.scala | 15 ++++ 5 files changed, 109 insertions(+), 18 deletions(-) delete mode 100644 src/test/scala/xyz/driver/core/RestTest.scala create mode 100644 src/test/scala/xyz/driver/core/rest/DriverRouteTest.scala create mode 100644 src/test/scala/xyz/driver/core/rest/RestTest.scala (limited to 'src/main/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala index 9af6657..eb9a31a 100644 --- a/src/main/scala/xyz/driver/core/rest/DriverRoute.scala +++ b/src/main/scala/xyz/driver/core/rest/DriverRoute.scala @@ -82,6 +82,9 @@ trait DriverRoute { case e: ResourceNotFoundException => log.info("Resource not found error", e) StatusCodes.NotFound + case e: ExternalServiceException => + log.error("Error while calling another service", e) + StatusCodes.InternalServerError case e: ExternalServiceTimeoutException => log.error("Service timeout error", e) StatusCodes.GatewayTimeout 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 ca1f759..e91a3c2 100644 --- a/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala +++ b/src/main/scala/xyz/driver/core/rest/errors/serviceException.scala @@ -1,6 +1,6 @@ package xyz.driver.core.rest.errors -abstract class ServiceException extends Exception { +sealed abstract class ServiceException extends Exception { def message: String } @@ -13,7 +13,7 @@ final case class ResourceNotFoundException(override val message: String = "Resou extends ServiceException final case class ExternalServiceException(serviceName: String, serviceMessage: String) extends ServiceException { - override def message = s"Error while calling another service: $serviceMessage" + override def message = s"Error while calling '$serviceName': $serviceMessage" } final case class ExternalServiceTimeoutException(serviceName: String) extends ServiceException { diff --git a/src/test/scala/xyz/driver/core/RestTest.scala b/src/test/scala/xyz/driver/core/RestTest.scala deleted file mode 100644 index efb9d07..0000000 --- a/src/test/scala/xyz/driver/core/RestTest.scala +++ /dev/null @@ -1,16 +0,0 @@ -package xyz.driver.core.rest - -import org.scalatest.{FlatSpec, Matchers} - -import akka.util.ByteString - -class RestTest extends FlatSpec with Matchers { - "`escapeScriptTags` function" should "escap script tags properly" in { - val dirtyString = " akkaComplete} +import akka.http.scaladsl.server.Route +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.rest.errors._ + +import scala.concurrent.Future + +class DriverRouteTest extends AsyncFlatSpec with ScalatestRouteTest with Matchers { + class TestRoute(override val route: Route) extends DriverRoute { + override def log: Logger = NoLogger + } + + "DriverRoute" should "respond with 200 OK for a basic route" in { + val route = new TestRoute(akkaComplete(StatusCodes.OK)) + + Get("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.OK + } + } + + it should "respond with a 400 for an InvalidInputException" in { + val route = new TestRoute(akkaComplete(Future.failed[String](InvalidInputException()))) + + Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.BadRequest + responseAs[String] shouldBe "Invalid input" + } + } + + it should "respond with a 400 for InvalidActionException" in { + val route = new TestRoute(akkaComplete(Future.failed[String](InvalidActionException()))) + + Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.Forbidden + responseAs[String] shouldBe "This action is not allowed" + } + } + + it should "respond with a 404 for ResourceNotFoundException" in { + val route = new TestRoute(akkaComplete(Future.failed[String](ResourceNotFoundException()))) + + Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.NotFound + responseAs[String] shouldBe "Resource not found" + } + } + + it should "respond with a 500 for ExternalServiceException" in { + val error = ExternalServiceException("GET /api/v1/users/", "Permission denied") + 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" + } + } + + it should "respond with a 503 for ExternalServiceTimeoutException" in { + val error = ExternalServiceTimeoutException("GET /api/v1/users/") + val route = new TestRoute(akkaComplete(Future.failed[String](error))) + + 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" + } + } + + it should "respond with a 500 for DatabaseException" in { + val route = new TestRoute(akkaComplete(Future.failed[String](DatabaseException()))) + + Post("/api/v1/foo/bar") ~> route.routeWithDefaults ~> check { + handled shouldBe true + status shouldBe StatusCodes.InternalServerError + responseAs[String] shouldBe "Database access error" + } + } +} diff --git a/src/test/scala/xyz/driver/core/rest/RestTest.scala b/src/test/scala/xyz/driver/core/rest/RestTest.scala new file mode 100644 index 0000000..2c3fb7f --- /dev/null +++ b/src/test/scala/xyz/driver/core/rest/RestTest.scala @@ -0,0 +1,15 @@ +package xyz.driver.core.rest + +import akka.util.ByteString +import org.scalatest.{FlatSpec, Matchers} + +class RestTest extends FlatSpec with Matchers { + "`escapeScriptTags` function" should "escap script tags properly" in { + val dirtyString = "