connor4312/cockatiel

CircuitBreakerPolicy HalfOpen state drops AbortSignal in recursive execute()

Summary

  • Context: The CircuitBreakerPolicy’s execute method handles calls that arrive during the HalfOpen state by waiting for the half-open test to complete, then recursively calling execute to process the queued call.

  • Bug: When the circuit is in HalfOpen state and a second call arrives with an abort signal, the recursive execute() call does not pass the signal parameter.

  • Actual vs. expected: The abort signal is lost and replaced with neverAbortedSignal (the default parameter), so the executed function receives a non-abortable signal instead of the caller’s signal.

  • Impact: Functions cannot respond to cancellation requests, breaking the AbortSignal cancellation contract and potentially causing operations to run longer than intended or waste resources on cancelled operations.

Code with bug

From src/CircuitBreakerPolicy.ts:

case CircuitState.HalfOpen:
  await state.test.catch(() => undefined);

  if (this.state === CircuitState.Closed && signal.aborted) {
    throw new TaskCancelledError();
  }

  return this.execute(fn); 
  // <-- BUG 🔴 Missing signal parameter
  //     Should be: this.execute(fn, signal)

Logical proof

  • Method signature shows signal has a default:

public async execute<T>(
  fn: (context: IDefaultPolicyContext) => PromiseLike<T> | T,
  signal = neverAbortedSignal,
): Promise

  • During HalfOpen, a call arrives as execute(fn, myAbortSignal); after waiting for the half-open test, code recurses with this.execute(fn) (no signal).

  • Parameter binding uses the default for the missing second arg, so signal = neverAbortedSignal.

  • Result:

// Actual
fn({ signal: neverAbortedSignal });

// Expected
fn({ signal: myAbortSignal });
  • Therefore the original AbortSignal is lost and the function cannot observe or react to caller cancellation.

Recommended fix

Change:

return this.execute(fn);

To:

return this.execute(fn, signal);