Issues when using Retry and CircuitBreaker Policy

[mrkaspa]

I have made this small example to test how these two policies worked together, and simulate some requests made over the web

import { sleep } from '@src/utils'
import {
  ConsecutiveBreaker,
  ExponentialBackoff,
  circuitBreaker,
  handleAll,
  retry,
  wrap,
} from 'cockatiel'

// Create a retry policy that'll try whatever function we execute 3
// times with a randomized exponential backoff.
const retryPolicy = retry(handleAll, {
  maxAttempts: 3,
  backoff: new ExponentialBackoff({
    initialDelay: 1000,
  }),
})

// Create a circuit breaker that'll stop calling the executed function for 10
// seconds if it fails 5 times in a row. This can give time for e.g. a database
// to recover without getting tons of traffic.
const circuitBreakerPolicy = circuitBreaker(handleAll, {
  halfOpenAfter: 2000,
  breaker: new ConsecutiveBreaker(3),
})

circuitBreakerPolicy.onBreak(ctx => {
  console.log('circuit is open', ctx, new Date().toISOString())
})
circuitBreakerPolicy.onStateChange(ctx => {
  console.log('circuit state changed', ctx, new Date().toISOString())
})

// Combine these! Create a policy that retries 3 times, calling through the circuit breaker
const retryWithBreaker = wrap(retryPolicy, circuitBreakerPolicy)

async function main() {
  // Call your database safely!

  await Promise.all([
    getInfo('a'),
    getInfo('b'),
    getInfo('c'),
    getInfo('d'),
    getInfo('e'),
  ])
}

let faileds = 0
let mapTimes: Record<string, { start: Date, end?: Date }> = {}

async function getInfo(procName: string) {
  return await retryWithBreaker.execute(async ctx => {
    if (ctx.attempt === 0) {
      mapTimes[procName] = { start: new Date() }
    }
    mapTimes[procName].end = new Date()
    console.log('getInfo', procName, ctx)
    let delay = 1000 //randomBetween(2000, 3000)
    // if (faileds < 3) {
    //   delay = 1000 //randomBetween(1000, 2000)
    // }
    // console.log('sleep delay', procName, delay, new Date().toISOString())
    await sleep(delay)
    if (faileds < 3) {
      console.log('raising error', procName, faileds, new Date().toISOString())
      faileds++
      throw new Error(`test ${procName} ${ctx.attempt}`)
    }
    console.log('returning ok', procName)
    return 'ok'
  })
}

const randomBetween = (min: number, max: number) => Math.floor(Math.random() * (max - min + 1)) + min;

(async () => {
  await main()
  for (const procName in mapTimes) {
    console.log(
      'Times by proc',
      procName,
      mapTimes[procName].end!.getTime() - mapTimes[procName].start.getTime(),
    )
  }
})()

// console.log(randomBetween(1000, 3000))

What I have found is that even when the circuit breaker is open it keeps retrying the requests and incrementing the counter for the attempts, and I would like to ask if this is the desired behaviour when these two policies work together.

getInfo a { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo b { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo c { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo d { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo e { signal: AbortSignal { aborted: false }, attempt: 0 }
raising error a 0 2025-02-19T18:28:17.304Z
raising error b 1 2025-02-19T18:28:17.307Z
raising error c 2 2025-02-19T18:28:17.307Z
circuit is open {
  error: Error: test c 0
      at /Users/michael.perez/code/js/blvd-migrations/cb.ts:71:13
      at runNextTicks (node:internal/process/task_queues:60:5)
      at listOnTimeout (node:internal/timers:545:9)
      at processTimers (node:internal/timers:519:7)
      at ExecuteWrapper.invoke (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/common/Executor.ts:52:21)
      at CircuitBreakerPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/CircuitBreakerPolicy.ts:225:24)
      at ExecuteWrapper.invoke (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/common/Executor.ts:52:21)
      at RetryPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/RetryPolicy.ts:105:22)
      at getInfo (/Users/michael.perez/code/js/blvd-migrations/cb.ts:56:10)
      at async Promise.all (index 2)
} 2025-02-19T18:28:17.307Z
circuit state changed 1 2025-02-19T18:28:17.315Z
returning ok d
returning ok e
getInfo a { signal: AbortSignal { aborted: false }, attempt: 2 }
circuit state changed 2 2025-02-19T18:28:19.384Z
returning ok a
circuit state changed 0 2025-02-19T18:28:20.386Z
getInfo b { signal: AbortSignal { aborted: false }, attempt: 2 }
returning ok b
getInfo c { signal: AbortSignal { aborted: false }, attempt: 3 }
returning ok c
Times by proc a 3082
Times by proc b 4086
Times by proc c 5875
Times by proc d 0
Times by proc e 0

When the halfOpenClose time is too big and the retries are finished before that timewindow the code crashes like this:

getInfo a { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo b { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo c { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo d { signal: AbortSignal { aborted: false }, attempt: 0 }
getInfo e { signal: AbortSignal { aborted: false }, attempt: 0 }
raising error a 0 2025-02-19T18:26:30.913Z
raising error b 1 2025-02-19T18:26:30.914Z
raising error c 2 2025-02-19T18:26:30.914Z
circuit is open {
  error: Error: test c 0
      at /Users/michael.perez/code/js/blvd-migrations/cb.ts:71:13
      at runNextTicks (node:internal/process/task_queues:60:5)
      at listOnTimeout (node:internal/timers:545:9)
      at processTimers (node:internal/timers:519:7)
      at ExecuteWrapper.invoke (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/common/Executor.ts:52:21)
      at CircuitBreakerPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/CircuitBreakerPolicy.ts:225:24)
      at ExecuteWrapper.invoke (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/common/Executor.ts:52:21)
      at RetryPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/RetryPolicy.ts:105:22)
      at getInfo (/Users/michael.perez/code/js/blvd-migrations/cb.ts:56:10)
      at async Promise.all (index 2)
} 2025-02-19T18:26:30.914Z
circuit state changed 1 2025-02-19T18:26:30.921Z
returning ok d
returning ok e
/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/dist/CircuitBreakerPolicy.js:161
                    throw new Errors_1.BrokenCircuitError();
                          ^

BrokenCircuitError: Execution prevented because the circuit breaker is open
    at CircuitBreakerPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/CircuitBreakerPolicy.ts:247:17)
    at run (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/Policy.ts:392:18)
    at /Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/Policy.ts:392:34
    at ExecuteWrapper.invoke (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/common/Executor.ts:52:27)
    at RetryPolicy.execute (/Users/michael.perez/code/js/blvd-migrations/node_modules/cockatiel/src/RetryPolicy.ts:105:42)
    at getInfo (/Users/michael.perez/code/js/blvd-migrations/cb.ts:56:10)
    at async Promise.all (index 1)
    at main (/Users/michael.perez/code/js/blvd-migrations/cb.ts:43:3)
    at /Users/michael.perez/code/js/blvd-migrations/cb.ts:81:3 {
  isBrokenCircuitError: true
}