Feature/improve error handling (#1290)

* - Fixed unexpected errors returning success result. * - Added TimeoutException as a baker exception, and convert timeout exceptions from akka or BakerF into this new type of exception. * - Added UnknownBakerException if baker exception could not be decoded. - Disabled stack trace if the baker exception is created in the json decoder (otherwise the stacktrace points to the decoder) - Updated unit test and fixed its warnings. * - Rename method to be consistent. * - Removed addition that was not supposed to be there. Co-authored-by: Wessel W. Bakker <wessel.bakker@ing.com>
ing-bank · Jul 25, 2022 · 0857cff · 0857cff
1 parent 7a674ec
commit 0857cff
Show file tree

Hide file tree

Showing 7 changed files with 132 additions and 79 deletions.
diff --git a/bakery/baker-state-k8s/src/test/scala/com/ing/bakery/baker/StateRuntimeSpec.scala b/bakery/baker-state-k8s/src/test/scala/com/ing/bakery/baker/StateRuntimeSpec.scala
@@ -7,7 +7,7 @@ import com.ing.baker.runtime.akka.{AkkaBaker, AkkaBakerConfig}
 import com.ing.baker.runtime.common.BakerException.NoSuchProcessException
 import com.ing.baker.runtime.common.{BakerException, SensoryEventStatus}
 import com.ing.baker.runtime.model.{InteractionInstance, InteractionManager}
-import com.ing.baker.runtime.scaladsl.{Baker, EventInstance, InteractionInstanceDescriptor, InteractionInstanceInput}
+import com.ing.baker.runtime.scaladsl.{Baker, EventInstance, InteractionInstanceInput}
 import com.ing.baker.types._
 import com.ing.bakery.baker.mocks.KubeApiServer
 import com.ing.bakery.mocks.{EventListener, RemoteInteraction}
@@ -72,9 +72,9 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
 
     test("Adding a recipe directly") { context =>
       for {
-        _ <- io(context.client.addRecipe(recipe, true))
+        _ <- io(context.client.addRecipe(recipe, validate = true))
         allRecipesBefore <- io(context.client.getAllRecipes)
-        _ <- io(context.client.addRecipe(otherRecipe, true))
+        _ <- io(context.client.addRecipe(otherRecipe, validate = true))
         allRecipesAfter <- io(context.client.getAllRecipes)
       } yield {
         allRecipesBefore.values.map(_.compiledRecipe.name).toSet shouldBe Set("ItemReservation.recipe")
@@ -112,8 +112,8 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
         _ <- context.remoteInteractionKubernetes.respondsWithReserveItems()
         _ <- context.kubeApiServer.deployInteraction()
         _ <- awaitForInteractionDiscovery(context)
-        _ <- io(context.client.addRecipe(recipe, true))
-        _ <- io(context.client.addRecipe(SimpleRecipe.compiledRecipe, true))
+        _ <- io(context.client.addRecipe(recipe, validate = true))
+        _ <- io(context.client.addRecipe(SimpleRecipe.compiledRecipe, validate = true))
         recipeInformation <- io(context.client.getRecipe(recipeId))
         interactionInformation <- io(context.client.getInteraction("ReserveItems"))
         noSuchRecipeError <- io(context.client
@@ -133,7 +133,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
           "ItemsReserved" -> Map("reservedItems" -> RecordType(Seq(RecordField("items",
             ListType(RecordType(Seq(RecordField("itemId", CharArray))))), RecordField("data", ByteArray))))
         ))
-        noSuchRecipeError shouldBe Some(BakerException.NoSuchRecipeException("nonexistent"))
+        noSuchRecipeError shouldBe Some(BakerException.NoSuchRecipeException("nonexistent", disableStackTrace = true))
         allRecipes.get(recipeId).map(_.compiledRecipe.name) shouldBe Some(recipe.name)
         allRecipes.get(SimpleRecipe.compiledRecipe.recipeId).map(_.compiledRecipe.name) shouldBe Some("Simple")
       }
@@ -170,7 +170,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
           .recover { case e: BakerException => Some(e) })
         state <- io(context.client.getRecipeInstanceState(recipeInstanceId))
       } yield {
-        e shouldBe Some(BakerException.ProcessAlreadyExistsException(recipeInstanceId))
+        e shouldBe Some(BakerException.ProcessAlreadyExistsException(recipeInstanceId, disableStackTrace = true))
         state.recipeInstanceId shouldBe recipeInstanceId
       }
     }
@@ -185,7 +185,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
           .bake("nonexistent", recipeInstanceId)
           .map(_ => None)
           .recover { case e => Some(e) })
-      } yield e shouldBe Some(BakerException.NoSuchRecipeException("nonexistent"))
+      } yield e shouldBe Some(BakerException.NoSuchRecipeException("nonexistent", disableStackTrace = true))
     }
 
     test("Baker.getRecipeInstanceState (fails with NoSuchProcessException)") { context =>
@@ -198,7 +198,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
           .map(_ => None)
           .recover { case e => Some(e) })
       } yield {
-        e shouldBe Some(NoSuchProcessException("nonexistent"))
+        e shouldBe Some(NoSuchProcessException("nonexistent", disableStackTrace = true))
       }
     }
 
@@ -242,8 +242,8 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
         serverState <- io(context.client.getRecipeInstanceState(recipeInstanceId))
         _ <- context.remoteInteractionKubernetes.didNothing
       } yield {
-        result shouldBe Some(BakerException.IllegalEventException("No event with name 'nonexistent' found in recipe 'ItemReservation.recipe'"))
-        serverState.events.map(_.name) should not contain ("OrderPlaced")
+        result shouldBe Some(BakerException.IllegalEventException("No event with name 'nonexistent' found in recipe 'ItemReservation.recipe'", disableStackTrace = true))
+        serverState.events.map(_.name) should not contain "OrderPlaced"
       }
     }
 
@@ -310,7 +310,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
         }
       } yield {
         state1 should contain("OrderPlaced")
-        state1 should not contain ("ItemsReserved")
+        state1 should not contain "ItemsReserved"
         state2 should contain("OrderPlaced")
         state2 should contain("ItemsReserved")
       }
@@ -339,7 +339,7 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
         }
       } yield {
         state1 should contain("OrderPlaced")
-        state1 should not contain ("ItemsReserved")
+        state1 should not contain "ItemsReserved"
         state2 should contain("OrderPlaced")
         state2 should contain("ItemsReserved")
         eventState shouldBe Some("resolution-item")
@@ -364,9 +364,9 @@ class StateRuntimeSpec extends BakeryFunSpec with Matchers {
         }
       } yield {
         state1 should contain("OrderPlaced")
-        state1 should not contain ("ItemsReserved")
+        state1 should not contain "ItemsReserved"
         state2 should contain("OrderPlaced")
-        state2 should not contain ("ItemsReserved")
+        state2 should not contain "ItemsReserved"
       }
     }
   }

diff --git a/bakery/dashboard/package.json b/bakery/dashboard/package.json
@@ -48,4 +48,4 @@
     "ts-node": "^8.10.2",
     "typescript": "~4.6.4"
   }
-}
+}
diff --git a/bakery/state/src/main/scala/com/ing/bakery/baker/BakeryExecutorJava.scala b/bakery/state/src/main/scala/com/ing/bakery/baker/BakeryExecutorJava.scala
@@ -10,7 +10,7 @@ import io.circe.generic.auto._
 import io.circe.parser.parse
 
 import java.nio.charset.Charset
-import java.util.Optional
+import java.util.{Optional, UUID}
 import java.util.concurrent.{CompletableFuture => JFuture}
 import scala.compat.java8.FutureConverters.FutureOps
 import scala.concurrent.{ExecutionContext, Future}
@@ -26,8 +26,9 @@ class BakeryExecutorJava(bakery: Bakery) extends LazyLogging {
     }.recover {
       case e: BakerException => BakerResult(e)
       case e: Throwable =>
-        logger.error(s"Unexpected exception happened when calling Baker", e)
-        BakerResult(e.getMessage)
+        val errorId = UUID.randomUUID().toString
+        logger.error(s"Unexpected exception happened when calling Baker (id='$errorId').", e)
+        BakerResult(BakerException.UnexpectedException(errorId))
     }.map(bakerResultEncoder.apply(_).noSpaces)
   }
 

diff --git a/core/akka-runtime/src/main/scala/com/ing/baker/runtime/akka/AkkaBaker.scala b/core/akka-runtime/src/main/scala/com/ing/baker/runtime/akka/AkkaBaker.scala
@@ -14,7 +14,7 @@ import com.ing.baker.runtime.akka.actor.recipe_manager.RecipeManagerProtocol
 import com.ing.baker.runtime.akka.actor.recipe_manager.RecipeManagerProtocol.RecipeFound
 import com.ing.baker.runtime.akka.internal.CachingInteractionManager
 import com.ing.baker.runtime.common.BakerException._
-import com.ing.baker.runtime.common.{BakerException, InteractionExecutionFailureReason, RecipeRecord, SensoryEventStatus}
+import com.ing.baker.runtime.common.{InteractionExecutionFailureReason, RecipeRecord, SensoryEventStatus}
 import com.ing.baker.runtime.recipe_manager.{ActorBasedRecipeManager, DefaultRecipeManager, RecipeManager}
 import com.ing.baker.runtime.scaladsl._
 import com.ing.baker.runtime.{javadsl, scaladsl}
@@ -195,7 +195,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
                   InteractionExecutionResult.Failure(InteractionExecutionFailureReason.TIMEOUT, Some(interactionInstance.name), None)))))(
               timer = cats.effect.IO.timer(config.system.dispatcher), cs = cats.effect.IO.contextShift(config.system.dispatcher))
       }
-      .unsafeToFuture()
+      .unsafeToFuture().javaTimeoutToBakerTimeout("executeSingleInteraction")
 
   /**
     * Creates a process instance for the given recipeId with the given RecipeInstanceId as identifier
@@ -205,7 +205,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * @return
     */
   override def bake(recipeId: String, recipeInstanceId: String): Future[Unit] = {
-    processIndexActor.ask(CreateProcess(recipeId, recipeInstanceId))(config.timeouts.defaultBakeTimeout).flatMap {
+    processIndexActor.ask(CreateProcess(recipeId, recipeInstanceId))(config.timeouts.defaultBakeTimeout).javaTimeoutToBakerTimeout("bake").flatMap {
       case _: Initialized =>
         Future.successful(())
       case ProcessAlreadyExists(_) =>
@@ -222,7 +222,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
       correlationId = correlationId,
       timeout = config.timeouts.defaultProcessEventTimeout,
       reaction = FireSensoryEventReaction.NotifyWhenReceived
-    ))(config.timeouts.defaultProcessEventTimeout).flatMap {
+    ))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("fireEventAndResolveWhenReceived").flatMap {
       // TODO MOVE THIS TO A FUNCTION
       case FireSensoryEventRejection.InvalidEvent(_, message) =>
         Future.failed(IllegalEventException(message))
@@ -247,7 +247,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
       correlationId = correlationId,
       timeout = config.timeouts.defaultProcessEventTimeout,
       reaction = FireSensoryEventReaction.NotifyWhenCompleted(waitForRetries = true)
-    ))(config.timeouts.defaultProcessEventTimeout).flatMap {
+    ))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("fireEventAndResolveWhenCompleted").flatMap {
       case FireSensoryEventRejection.InvalidEvent(_, message) =>
         Future.failed(IllegalEventException(message))
       case FireSensoryEventRejection.NoSuchRecipeInstance(recipeInstanceId0) =>
@@ -271,7 +271,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
       correlationId = correlationId,
       timeout = config.timeouts.defaultProcessEventTimeout,
       reaction = FireSensoryEventReaction.NotifyOnEvent(waitForRetries = true, onEvent)
-    ))(config.timeouts.defaultProcessEventTimeout).flatMap {
+    ))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("fireEventAndResolveOnEvent").flatMap {
       case FireSensoryEventRejection.InvalidEvent(_, message) =>
         Future.failed(IllegalEventException(message))
       case FireSensoryEventRejection.NoSuchRecipeInstance(recipeInstanceId0) =>
@@ -299,7 +299,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
         reaction = FireSensoryEventReaction.NotifyBoth(
           waitForRetries = true,
           completeReceiver = futureRef.ref)
-      ))(config.timeouts.defaultProcessEventTimeout).flatMap {
+      ))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("fireEvent").flatMap {
         case FireSensoryEventRejection.InvalidEvent(_, message) =>
           Future.failed(IllegalEventException(message))
         case FireSensoryEventRejection.NoSuchRecipeInstance(recipeInstanceId0) =>
@@ -316,7 +316,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
           Future.successful(status)
       }
     val futureCompleted =
-      futureRef.future.flatMap {
+      futureRef.future.javaTimeoutToBakerTimeout("fireEvent").flatMap {
         case FireSensoryEventRejection.InvalidEvent(_, message) =>
           Future.failed(IllegalEventException(message))
         case FireSensoryEventRejection.NoSuchRecipeInstance(recipeInstanceId0) =>
@@ -341,7 +341,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * @return
     */
   override def retryInteraction(recipeInstanceId: String, interactionName: String): Future[Unit] = {
-    processIndexActor.ask(RetryBlockedInteraction(recipeInstanceId, interactionName))(config.timeouts.defaultProcessEventTimeout).map(_ => ())
+    processIndexActor.ask(RetryBlockedInteraction(recipeInstanceId, interactionName))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("retryInteraction").map(_ => ())
   }
 
   /**
@@ -352,7 +352,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * @return
     */
   override def resolveInteraction(recipeInstanceId: String, interactionName: String, event: EventInstance): Future[Unit] = {
-    processIndexActor.ask(ResolveBlockedInteraction(recipeInstanceId, interactionName, event))(config.timeouts.defaultProcessEventTimeout).map(_ => ())
+    processIndexActor.ask(ResolveBlockedInteraction(recipeInstanceId, interactionName, event))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("resolveInteraction").map(_ => ())
   }
 
   /**
@@ -361,7 +361,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * @return
     */
   override def stopRetryingInteraction(recipeInstanceId: String, interactionName: String): Future[Unit] = {
-    processIndexActor.ask(StopRetryingInteraction(recipeInstanceId, interactionName))(config.timeouts.defaultProcessEventTimeout).map(_ => ())
+    processIndexActor.ask(StopRetryingInteraction(recipeInstanceId, interactionName))(config.timeouts.defaultProcessEventTimeout).javaTimeoutToBakerTimeout("stopRetryingInteraction").map(_ => ())
   }
 
   /**
@@ -375,9 +375,10 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * @return An index of all processes
     */
   override def getAllRecipeInstancesMetadata: Future[Set[RecipeInstanceMetadata]] = {
-    Future.successful(config.bakerActorProvider
+    Future(config.bakerActorProvider
       .getAllProcessesMetadata(processIndexActor)(system, config.timeouts.defaultInquireTimeout)
       .map(p => RecipeInstanceMetadata(p.recipeId, p.recipeInstanceId, p.createdDateTime)).toSet)
+      .javaTimeoutToBakerTimeout("getAllRecipeInstancesMetadata")
   }
 
   /**
@@ -389,6 +390,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
   override def getRecipeInstanceState(recipeInstanceId: String): Future[RecipeInstanceState] =
     processIndexActor
       .ask(GetProcessState(recipeInstanceId))(Timeout.durationToTimeout(config.timeouts.defaultInquireTimeout))
+      .javaTimeoutToBakerTimeout("getRecipeInstanceState")
       .flatMap {
         case instance: InstanceState => Future.successful(instance.state.asInstanceOf[RecipeInstanceState])
         case Uninitialized(id) => Future.failed(NoSuchProcessException(id))
@@ -433,7 +435,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
   @throws[NoSuchProcessException]("If the process is not found")
   override def getVisualState(recipeInstanceId: String, style: RecipeVisualStyle = RecipeVisualStyle.default): Future[String] = {
     for {
-      getRecipeResponse <- processIndexActor.ask(GetCompiledRecipe(recipeInstanceId))(config.timeouts.defaultInquireTimeout)
+      getRecipeResponse <- processIndexActor.ask(GetCompiledRecipe(recipeInstanceId))(config.timeouts.defaultInquireTimeout).javaTimeoutToBakerTimeout("getVisualState")
       processState <- getRecipeInstanceState(recipeInstanceId)
       response <- getRecipeResponse match {
         case RecipeFound(compiledRecipe, _) =>
@@ -506,6 +508,7 @@ class AkkaBaker private[runtime](config: AkkaBakerConfig) extends scaladsl.Baker
     * Attempts to gracefully shutdown the baker system.
     */
   @nowarn
-  override def gracefulShutdown: Future[Unit] =
-    Future.successful(GracefulShutdown.gracefulShutdownActorSystem(system, config.timeouts.defaultShutdownTimeout))
+  override def gracefulShutdown(): Future[Unit] =
+    Future(GracefulShutdown.gracefulShutdownActorSystem(system, config.timeouts.defaultShutdownTimeout))
+      .javaTimeoutToBakerTimeout("gracefulShutdown").map(_ => ())
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -48,4 +48,4 @@ @@
         "ts-node": "^8.10.2",
         "typescript": "~4.6.4"
       }
-    }
+    }