[#314] 'CircuitBreaker.isFailure' logic not being executed. (#317)

The current logic which checks 'isFailure' in 'CircuitBreakerImpl.apply' 'case Left(e) if isFailure.isDefinedAt(e) => ZIO.fail(e)' isn't accomplishing the desired result and in almost all cases will return 'false'. The reason is that 'isDefinedAt' only checks if 'e' is covered within the domain of the 'PartialFunction', however it does not execute the logic of the partial function. instead 'applyOrElse' should be used to perform both operations: 'case Left(e) if isFailure.applyOrElse[E1, Boolean](e, _ => false) => ZIO.fail(e)' There is also a bug in the 'CircuitBreakerSpec' that is preventing the test from failing (the 'either' is in the wrong spot). Co-authored-by: Scott T Weaver <[email protected]>
svroonland · Feb 18, 2023 · f6d32f5 · f6d32f5
1 parent dddeb6c
commit f6d32f5
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 5 deletions.
diff --git a/rezilience/shared/src/main/scala/nl/vroste/rezilience/CircuitBreaker.scala b/rezilience/shared/src/main/scala/nl/vroste/rezilience/CircuitBreaker.scala
@@ -204,9 +204,9 @@ object CircuitBreaker {
                               } yield ()).uninterruptible
 
                             f.either.flatMap {
-                              case Left(e) if isFailure.isDefinedAt(e) => ZIO.fail(e)
-                              case Left(e)                             => ZIO.left(WrappedError(e))
-                              case Right(e)                            => ZIO.right(e)
+                              case Left(e) if isFailure.applyOrElse[E1, Boolean](e, _ => false) => ZIO.fail(e)
+                              case Left(e)                                                      => ZIO.left(WrappedError(e))
+                              case Right(e)                                                     => ZIO.right(e)
 
                             }
                               .tapBoth(_ => onComplete(callSuccessful = false), _ => onComplete(callSuccessful = true))

diff --git a/rezilience/shared/src/test/scala/nl/vroste/rezilience/CircuitBreakerSpec.scala b/rezilience/shared/src/test/scala/nl/vroste/rezilience/CircuitBreakerSpec.scala
@@ -44,9 +44,11 @@ object CircuitBreakerSpec extends DefaultRunnableSpec {
         .withMaxFailures(3, Schedule.exponential(1.second), isFailure)
         .use { cb =>
           for {
-            _      <- ZIO.foreach_(1 to 3)(_ => cb(ZIO.fail(MyNotFatalError))).either
+            _      <- cb(ZIO.fail(MyNotFatalError)).either.repeatN(3)
             result <- cb(ZIO.fail(MyCallError)).either
-          } yield assert(result)(isLeft(not(equalTo(CircuitBreaker.CircuitBreakerOpen))))
+          } yield assertTrue(
+            result.left.toOption.get != CircuitBreaker.CircuitBreakerOpen
+          )
         }
     },
     testM("reset to closed state after reset timeout") {