Revamp NetworkController RPC endpoint events

packages/controller-utils/CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `getCircuitState` method to `ServicePolicy` ([#7164](https://github.com/MetaMask/core/pull/7164))
+  - This can be used when working with a chain of services to know whether a service's underlying circuit is open or closed.
+- Add `onAvailable` method to `ServicePolicy` ([#7164](https://github.com/MetaMask/core/pull/7164))
+  - This can be used to listen for the initial successful execution of the service, or the first successful execution after the service becomes degraded or the circuit breaks.
+- Add `reset` method to `ServicePolicy` ([#7164](https://github.com/MetaMask/core/pull/7164))
+  - This can be used when working with a chain of services to reset the state of the circuit breaker policy (e.g. if a primary recovers and we want to reset the failovers).
+- Export `CockatielEventEmitter` and `CockatielFailureReason` from Cockatiel ([#7164](https://github.com/MetaMask/core/pull/7164))
+  - These can be used to further transform types for event emitters/listeners.
+
 ## [11.15.0]
 
 ### Added

packages/controller-utils/src/create-service-policy.ts

@@ -23,14 +23,15 @@ import type {
 
 export {
   BrokenCircuitError,
+  CockatielEventEmitter,
   CircuitState,
   ConstantBackoff,
   ExponentialBackoff,
   handleAll,
   handleWhen,
 };
 
-export type { CockatielEvent };
+export type { CockatielEvent, FailureReason as CockatielFailureReason };
 
 /**
  * The options for `createServicePolicy`.
@@ -85,12 +86,21 @@ export type ServicePolicy = IPolicy & {
    * maximum consecutive failures is reached.
    */
   circuitBreakDuration: number;
+  /**
+   * @returns The state of the underlying circuit.
+   */
+  getCircuitState: () => CircuitState;
   /**
    * If the circuit is open and ongoing requests are paused, returns the number
    * of milliseconds before the requests will be attempted again. If the circuit
    * is not open, returns null.
    */
   getRemainingCircuitOpenDuration: () => number | null;
+  /**
+   * Resets the internal circuit breaker policy (if it is open, it will now be
+   * closed).
+   */
+  reset: () => void;
   /**
    * The Cockatiel retry policy that the service policy uses internally.
    */
@@ -108,6 +118,12 @@ export type ServicePolicy = IPolicy & {
    * number of consecutive failures has been reached.
    */
   onDegraded: CockatielEvent<FailureReason<unknown> | void>;
+  /**
+   * A function which is called when the service succeeds for the first time,
+   * or when the service fails enough times to cause the circuit to break and
+   * then recovers.
+   */
+  onAvailable: CockatielEvent<void>;
   /**
    * A function which will be called by the retry policy each time the service
    * fails and the policy kicks off a timer to re-run the service. This is
@@ -127,6 +143,25 @@ type InternalCircuitState =
     }
   | { state: Exclude<CircuitState, CircuitState.Open> };
 
+/**
+ * List of avalability statuses.
+ *
+ * Used to keep track of whether the `onAvailable` event should be fired.
+ */
+const AVAILABILITY_STATUSES = {
+  Unknown: 'unknown',
+  Available: 'available',
+  Unavailable: 'unavailable',
+};
+
+/**
+ * An availability status.
+ *
+ * Used to keep track of whether the `onAvailable` event should be fired.
+ */
+type AvailabilityStatus =
+  (typeof AVAILABILITY_STATUSES)[keyof typeof AVAILABILITY_STATUSES];
+
 /**
  * The maximum number of times that a failing service should be re-run before
  * giving up.
@@ -249,6 +284,8 @@ export function createServicePolicy(
     backoff = new ExponentialBackoff(),
   } = options;
 
+  let availabilityStatus: AvailabilityStatus = AVAILABILITY_STATUSES.Unknown;
+
   const retryPolicy = retry(retryFilterPolicy, {
     // Note that although the option here is called "max attempts", it's really
     // maximum number of *retries* (attempts past the initial attempt).
@@ -259,6 +296,7 @@ export function createServicePolicy(
   });
   const onRetry = retryPolicy.onRetry.bind(retryPolicy);
 
+  const consecutiveBreaker = new ConsecutiveBreaker(maxConsecutiveFailures);
   const circuitBreakerPolicy = circuitBreaker(handleWhen(isServiceFailure), {
     // While the circuit is open, any additional invocations of the service
     // passed to the policy (either via automatic retries or by manually
@@ -267,7 +305,7 @@ export function createServicePolicy(
     // service will be allowed to run again. If the service succeeds, the
     // circuit will close, otherwise it will remain open.
     halfOpenAfter: circuitBreakDuration,
-    breaker: new ConsecutiveBreaker(maxConsecutiveFailures),
+    breaker: consecutiveBreaker,
   });
 
   let internalCircuitState: InternalCircuitState = getInternalCircuitState(
@@ -276,27 +314,56 @@ export function createServicePolicy(
   circuitBreakerPolicy.onStateChange((state) => {
     internalCircuitState = getInternalCircuitState(state);
   });
+
+  circuitBreakerPolicy.onBreak(() => {
+    availabilityStatus = AVAILABILITY_STATUSES.Unavailable;
+  });
   const onBreak = circuitBreakerPolicy.onBreak.bind(circuitBreakerPolicy);
 
   const onDegradedEventEmitter =
     new CockatielEventEmitter<FailureReason<unknown> | void>();
+  const onDegraded = onDegradedEventEmitter.addListener;
+
+  const onAvailableEventEmitter = new CockatielEventEmitter<void>();
+  const onAvailable = onAvailableEventEmitter.addListener;
+
   retryPolicy.onGiveUp((data) => {
     if (circuitBreakerPolicy.state === CircuitState.Closed) {
       onDegradedEventEmitter.emit(data);
     }
   });
   retryPolicy.onSuccess(({ duration }) => {
-    if (
-      circuitBreakerPolicy.state === CircuitState.Closed &&
-      duration > degradedThreshold
-    ) {
-      onDegradedEventEmitter.emit();
+    if (circuitBreakerPolicy.state === CircuitState.Closed) {
+      if (duration > degradedThreshold) {
+        onDegradedEventEmitter.emit();
+      } else if (availabilityStatus !== AVAILABILITY_STATUSES.Available) {
+        availabilityStatus = AVAILABILITY_STATUSES.Available;
+        onAvailableEventEmitter.emit();
+      }
     }
   });
-  const onDegraded = onDegradedEventEmitter.addListener;
 
   // Every time the retry policy makes an attempt, it executes the circuit
   // breaker policy, which executes the service.
+  //
+  // Calling:
+  //
+  //   policy.execute(() => {
+  //     // do what the service does
+  //   })
+  //
+  // is equivalent to:
+  //
+  //   retryPolicy.execute(() => {
+  //     circuitBreakerPolicy.execute(() => {
+  //       // do what the service does
+  //     });
+  //   });
+  //
+  // So if the retry policy succeeds or fails, it is because the circuit breaker
+  // policy succeeded or failed. And if there are any event listeners registered
+  // on the retry policy, by the time they are called, the state of the circuit
+  // breaker will have already changed.
   const policy = wrap(retryPolicy, circuitBreakerPolicy);
 
   const getRemainingCircuitOpenDuration = () => {
@@ -306,14 +373,35 @@ export function createServicePolicy(
     return null;
   };
 
+  const getCircuitState = () => {
+    return circuitBreakerPolicy.state;
+  };
+
+  const reset = () => {
+    // Set the state of the policy to "isolated" regardless of its current state
+    const { dispose } = circuitBreakerPolicy.isolate();
+    // Reset the state to "closed"
+    dispose();
+
+    // Reset the counter on the breaker as well
+    consecutiveBreaker.success();
+
+    // Re-initialize the availability status so that if the service is executed
+    // successfully, onAvailable listeners will be called again
+    availabilityStatus = AVAILABILITY_STATUSES.Unknown;
+  };
+
   return {
     ...policy,
     circuitBreakerPolicy,
     circuitBreakDuration,
+    getCircuitState,
     getRemainingCircuitOpenDuration,
+    reset,
     retryPolicy,
     onBreak,
     onDegraded,
+    onAvailable,
     onRetry,
   };
 }

packages/controller-utils/src/index.test.ts

@@ -6,6 +6,7 @@ describe('@metamask/controller-utils', () => {
       Array [
         "BrokenCircuitError",
         "CircuitState",
+        "CockatielEventEmitter",
         "ConstantBackoff",
         "DEFAULT_CIRCUIT_BREAK_DURATION",
         "DEFAULT_DEGRADED_THRESHOLD",

packages/controller-utils/src/index.ts

@@ -1,6 +1,7 @@
 export {
   BrokenCircuitError,
   CircuitState,
+  CockatielEventEmitter,
   ConstantBackoff,
   DEFAULT_CIRCUIT_BREAK_DURATION,
   DEFAULT_DEGRADED_THRESHOLD,
@@ -14,6 +15,7 @@ export {
 export type {
   CockatielEvent,
   CreateServicePolicyOptions,
+  CockatielFailureReason,
   ServicePolicy,
 } from './create-service-policy';
 export {

packages/network-controller/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `NetworkController:rpcEndpointAvailable` messenger event ([#7166](https://github.com/MetaMask/core/pull/7166))
+  - These are counterparts to the (new) `NetworkController:rpcEndpointUnavailable` and `NetworkController:rpcEndpointDegraded` events, but are published when a request to an RPC endpoint URL is made either initially or following a previously established degraded or unavailable status.
+
 ### Changed
 
 - **BREAKING:** Use `InternalProvider` instead of `SafeEventEmitterProvider` ([#6796](https://github.com/MetaMask/core/pull/6796))
@@ -19,6 +24,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - In practice, this should happen rarely if ever.
 - **BREAKING:** Migrate `NetworkClient` to `JsonRpcEngineV2` ([#7065](https://github.com/MetaMask/core/pull/7065))
   - This ought to be unobservable, but we mark it as breaking out of an abundance of caution.
+- **BREAKING:** Split up and update payload data for `NetworkController:rpcEndpoint{Degraded,Unavailable}` ([#7166](https://github.com/MetaMask/core/pull/7166))
+  - The existing events are now called `NetworkController:rpcEndpointInstance{Degraded,Unavailable}` and retain their present behavior.
+  - `NetworkController:rpcEndpointInstance{Degraded,Unavailable}` do still exist, but they are now designed to represent the entire RPC endpoint and are guaranteed to not be published multiple times in a row. In particular, `NetworkController:rpcEndpointUnavailable` is published only after trying all of the designated URLs for a particular RPC endpoint and the underlying circuit for the last URL breaks, not as each primary's or failover's circuit breaks.
+  - The event payloads have been changed as well: `failoverEndpointUrl` has been renamed to `endpointUrl`, and `endpointUrl` has been renamed to `primaryEndpointUrl`. In addition, `networkClientId` has been added.
+- **BREAKING:** Rename and update payload data for `NetworkController:rpcEndpointRequestRetried` ([#7166](https://github.com/MetaMask/core/pull/7166))
+  - This event is now called `NetworkController:rpcEndpointInstanceRequestRetried`
+  - The event payload has been changed as well: `failoverEndpointUrl` has been renamed to `endpointUrl`, and `endpointUrl` has been renamed to `primaryEndpointUrl`. In addition, `networkClientId` and `attempt` have been added.
+- **BREAKING:** Update `AbstractRpcService`/`RpcServiceRequestable` to remove `{ isolated: true }` from the `onBreak` event data type ([#7166](https://github.com/MetaMask/core/pull/7166))
+  - This represented the error produced when `.isolate` is called on a Cockatiel circuit breaker policy, which we never do.
 - Bump `@metamask/controller-utils` from `^11.14.1` to `^11.15.0` ([#7003](https://github.com/MetaMask/core/pull/7003))
 
 ### Fixed

packages/network-controller/package.json

@@ -78,6 +78,7 @@
     "@types/jest-when": "^2.7.3",
     "@types/lodash": "^4.14.191",
     "@types/node-fetch": "^2.6.12",
+    "cockatiel": "^3.1.2",
     "deep-freeze-strict": "^1.1.1",
     "deepmerge": "^4.2.2",
     "jest": "^27.5.1",