From 747d761c9b3b3c790ce9d0610060121cbb2a2538 Mon Sep 17 00:00:00 2001 From: hongwei Date: Wed, 28 Jan 2026 16:00:57 +0100 Subject: [PATCH] feature/(http4s): handle errors in executeAndRespond and improve error parsing Refactor executeAndRespond to properly handle exceptions from Future and convert them to HTTP responses using ErrorResponseConverter. This ensures consistent error handling across HTTP4S endpoints. Simplify error response creation by parsing APIFailureNewStyle exceptions from JSON message instead of direct type matching, making error handling more robust. Update API version validation in Http4s700 to use NewStyle.function.tryons and Helper.booleanToFuture for consistent error handling patterns. Adjust test to use proper error message constant for invalid API version. --- .../util/http4s/ErrorResponseConverter.scala | 48 ++++++------------- .../code/api/util/http4s/Http4sSupport.scala | 11 +++-- .../scala/code/api/v7_0_0/Http4s700.scala | 28 +++++++---- .../code/api/v7_0_0/Http4s700RoutesTest.scala | 5 +- 4 files changed, 44 insertions(+), 48 deletions(-) diff --git a/obp-api/src/main/scala/code/api/util/http4s/ErrorResponseConverter.scala b/obp-api/src/main/scala/code/api/util/http4s/ErrorResponseConverter.scala index 856b0f1ee..25c2d7829 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/ErrorResponseConverter.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/ErrorResponseConverter.scala @@ -1,10 +1,9 @@ package code.api.util.http4s import cats.effect._ -import code.api.APIFailureNewStyle import code.api.util.ErrorMessages._ import code.api.util.CallContext -import net.liftweb.common.{Failure => LiftFailure} +import net.liftweb.json.{JInt, JString, parseOpt} import net.liftweb.json.compactRender import net.liftweb.json.JsonDSL._ import org.http4s._ @@ -30,6 +29,8 @@ object ErrorResponseConverter { implicit val formats: Formats = CustomJsonFormats.formats private val jsonContentType: `Content-Type` = `Content-Type`(MediaType.application.json) + private val internalFieldsFailCode = "failCode" + private val internalFieldsFailMsg = "failMsg" /** * OBP standard error response format. @@ -51,39 +52,20 @@ object ErrorResponseConverter { * Convert any error to http4s Response[IO]. */ def toHttp4sResponse(error: Throwable, callContext: CallContext): IO[Response[IO]] = { - error match { - case e: APIFailureNewStyle => apiFailureToResponse(e, callContext) - case _ => unknownErrorToResponse(error, callContext) + parseApiFailureFromExceptionMessage(error).map { failure => + createErrorResponse(failure.code, failure.message, callContext) + }.getOrElse { + unknownErrorToResponse(error, callContext) } } - /** - * Convert APIFailureNewStyle to http4s Response. - * Uses failCode as HTTP status and failMsg as error message. - */ - def apiFailureToResponse(failure: APIFailureNewStyle, callContext: CallContext): IO[Response[IO]] = { - val errorJson = OBPErrorResponse(failure.failCode, failure.failMsg) - val status = org.http4s.Status.fromInt(failure.failCode).getOrElse(org.http4s.Status.BadRequest) - IO.pure( - Response[IO](status) - .withEntity(toJsonString(errorJson)) - .withContentType(jsonContentType) - .putHeaders(org.http4s.Header.Raw(CIString("Correlation-Id"), callContext.correlationId)) - ) - } - - /** - * Convert Lift Box Failure to http4s Response. - * Returns 400 Bad Request with failure message. - */ - def boxFailureToResponse(failure: LiftFailure, callContext: CallContext): IO[Response[IO]] = { - val errorJson = OBPErrorResponse(400, failure.msg) - IO.pure( - Response[IO](org.http4s.Status.BadRequest) - .withEntity(toJsonString(errorJson)) - .withContentType(jsonContentType) - .putHeaders(org.http4s.Header.Raw(CIString("Correlation-Id"), callContext.correlationId)) - ) + private def parseApiFailureFromExceptionMessage(error: Throwable): Option[OBPErrorResponse] = { + Option(error.getMessage).flatMap(parseOpt).flatMap { json => + (json \ internalFieldsFailCode, json \ internalFieldsFailMsg) match { + case (JInt(code), JString(message)) => Some(OBPErrorResponse(code.toInt, message)) + case _ => None + } + } } /** @@ -91,7 +73,7 @@ object ErrorResponseConverter { * Returns 500 Internal Server Error. */ def unknownErrorToResponse(e: Throwable, callContext: CallContext): IO[Response[IO]] = { - val errorJson = OBPErrorResponse(500, s"$UnknownError: ${e.getMessage}") + val errorJson = OBPErrorResponse(500, UnknownError) IO.pure( Response[IO](org.http4s.Status.InternalServerError) .withEntity(toJsonString(errorJson)) diff --git a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala index f231ba002..1f95980fc 100644 --- a/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala +++ b/obp-api/src/main/scala/code/api/util/http4s/Http4sSupport.scala @@ -106,9 +106,14 @@ object Http4sRequestAttributes { def executeAndRespond[A](req: Request[IO])(f: CallContext => Future[A])(implicit formats: Formats): IO[Response[IO]] = { implicit val cc: CallContext = req.callContext for { - result <- IO.fromFuture(IO(f(cc))) - jsonString = prettyRender(Extraction.decompose(result)) - response <- Ok(jsonString) + attempted <- IO.fromFuture(IO(f(cc))).attempt + response <- attempted match { + case Right(result) => + val jsonString = prettyRender(Extraction.decompose(result)) + Ok(jsonString) + case Left(error) => + ErrorResponseConverter.toHttp4sResponse(error, cc) + } } yield response } diff --git a/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala b/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala index e812a4f9e..6d28b18a1 100644 --- a/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala +++ b/obp-api/src/main/scala/code/api/v7_0_0/Http4s700.scala @@ -26,7 +26,7 @@ import org.http4s.dsl.io._ import scala.collection.mutable.ArrayBuffer import scala.concurrent.Future import scala.language.{higherKinds, implicitConversions} -import scala.util.{Failure, Success, Try} +import code.util.Helper object Http4s700 { @@ -217,16 +217,24 @@ object Http4s700 { .map(_.trim) .filter(_.nonEmpty) - Try(ApiVersionUtils.valueOf(requestedApiVersionString)) match { - case Success(requestedApiVersion) if requestedApiVersion == ApiVersion.v7_0_0 => - val http4sOnlyDocs = ResourceDocsAPIMethodsUtil.filterResourceDocs(resourceDocs.toList, tags, functions) - EndpointHelpers.executeAndRespond(req) { _ => - Future.successful(JSONFactory1_4_0.createResourceDocsJson(http4sOnlyDocs, isVersion4OrHigher = true, localeParam, includeTechnology = true)) + EndpointHelpers.executeAndRespond(req) { _ => + for { + requestedApiVersion <- NewStyle.function.tryons( + failMsg = s"$InvalidApiVersionString Current value: $requestedApiVersionString", + failCode = 400, + callContext = Some(cc) + ) { + ApiVersionUtils.valueOf(requestedApiVersionString) } - case Success(_) => - ErrorResponseConverter.createErrorResponse(400, s"API Version not supported by this server: $requestedApiVersionString", cc) - case Failure(_) => - ErrorResponseConverter.createErrorResponse(400, s"Invalid API Version: $requestedApiVersionString", cc) + _ <- Helper.booleanToFuture( + failMsg = s"$InvalidApiVersionString This server supports only ${ApiVersion.v7_0_0}. Current value: $requestedApiVersionString", + failCode = 400, + cc = Some(cc) + ) { + requestedApiVersion == ApiVersion.v7_0_0 + } + http4sOnlyDocs = ResourceDocsAPIMethodsUtil.filterResourceDocs(resourceDocs.toList, tags, functions) + } yield JSONFactory1_4_0.createResourceDocsJson(http4sOnlyDocs, isVersion4OrHigher = true, localeParam, includeTechnology = true) } } diff --git a/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala b/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala index 8b0f445c0..3f8e80795 100644 --- a/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala +++ b/obp-api/src/test/scala/code/api/v7_0_0/Http4s700RoutesTest.scala @@ -3,7 +3,7 @@ package code.api.v7_0_0 import cats.effect.IO import cats.effect.unsafe.implicits.global import code.api.util.ApiRole.{canGetCardsForBank, canReadResourceDoc} -import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, BankNotFound, UserHasMissingRoles} +import code.api.util.ErrorMessages.{AuthenticatedUserIsRequired, BankNotFound, InvalidApiVersionString, UserHasMissingRoles} import code.setup.ServerSetupWithTestData import net.liftweb.json.JValue import net.liftweb.json.JsonAST.{JArray, JField, JObject, JString} @@ -333,7 +333,8 @@ class Http4s700RoutesTest extends ServerSetupWithTestData { case JObject(fields) => toFieldMap(fields).get("message") match { case Some(JString(message)) => - message should include("API Version not supported") + message should include(InvalidApiVersionString) + message should include("v6.0.0") case _ => fail("Expected message field as JSON string for invalid-version response") }