Reword circuit break error message (#6423)

The error that is thrown when an RPC endpoint responds with too many 5xx
errors in a row — causing the underlying circuit to break — is too
cryptic for end users. This commit rewords it to be more friendly and
recommend that users switch to another endpoint. It also logs the
original Cockatiel error so that users can inspect DevTools to learn
more.

- Old error: "Execution prevented because the circuit breaker is open"
- New error: "RPC endpoint returned too many errors, retrying in X
minutes. Consider using a different RPC endpoint."

Author: mcmire

packages/controller-utils/CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `circuitBreakDuration` to the object returned by `createServicePolicy` ([#6423](https://github.com/MetaMask/core/pull/6423))
+  - This is the amount of time that the underlying circuit breaker policy will pause execution of the input function while the circuit is broken.
+- Add `getRemainingCircuitOpenDuration` to the object returned by `createServicePolicy` ([#6423](https://github.com/MetaMask/core/pull/6423))
+  - This returns the amount of time after which the underlying circuit breaker policy will resume execution of the input function after the circuit reopens.
+
 ## [11.12.0]
 
 ### Added

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

@@ -80,6 +80,17 @@ export type ServicePolicy = IPolicy & {
    * internally.
    */
   circuitBreakerPolicy: CircuitBreakerPolicy;
+  /**
+   * The amount of time to pause requests to the service if the number of
+   * maximum consecutive failures is reached.
+   */
+  circuitBreakDuration: number;
+  /**
+   * 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;
   /**
    * The Cockatiel retry policy that the service policy uses internally.
    */
@@ -105,6 +116,17 @@ export type ServicePolicy = IPolicy & {
   onRetry: RetryPolicy['onRetry'];
 };
 
+/**
+ * Parts of the circuit breaker's internal and external state as necessary in
+ * order to compute the time remaining before the circuit will reopen.
+ */
+type InternalCircuitState =
+  | {
+      state: CircuitState.Open;
+      openedAt: number;
+    }
+  | { state: Exclude<CircuitState, CircuitState.Open> };
+
 /**
  * The maximum number of times that a failing service should be re-run before
  * giving up.
@@ -146,6 +168,25 @@ const isServiceFailure = (error: unknown) => {
   return true;
 };
 
+/**
+ * The circuit breaker policy inside of the Cockatiel library exposes some of
+ * its state, but not all of it. Notably, the time that the circuit opened is
+ * not publicly accessible. So we have to record this ourselves.
+ *
+ * This function therefore allows us to obtain the circuit breaker state that we
+ * wish we could access.
+ *
+ * @param state - The public state of a circuit breaker policy.
+ * @returns if the circuit is open, the state of the circuit breaker policy plus
+ * the time that it opened, otherwise just the circuit state.
+ */
+function getInternalCircuitState(state: CircuitState): InternalCircuitState {
+  if (state === CircuitState.Open) {
+    return { state, openedAt: Date.now() };
+  }
+  return { state };
+}
+
 /**
  * Constructs an object exposing an `execute` method which, given a function —
  * hereafter called the "service" — will retry that service with ever increasing
@@ -228,6 +269,13 @@ export function createServicePolicy(
     halfOpenAfter: circuitBreakDuration,
     breaker: new ConsecutiveBreaker(maxConsecutiveFailures),
   });
+
+  let internalCircuitState: InternalCircuitState = getInternalCircuitState(
+    circuitBreakerPolicy.state,
+  );
+  circuitBreakerPolicy.onStateChange((state) => {
+    internalCircuitState = getInternalCircuitState(state);
+  });
   const onBreak = circuitBreakerPolicy.onBreak.bind(circuitBreakerPolicy);
 
   const onDegradedEventEmitter =
@@ -251,9 +299,18 @@ export function createServicePolicy(
   // breaker policy, which executes the service.
   const policy = wrap(retryPolicy, circuitBreakerPolicy);
 
+  const getRemainingCircuitOpenDuration = () => {
+    if (internalCircuitState.state === CircuitState.Open) {
+      return internalCircuitState.openedAt + circuitBreakDuration - Date.now();
+    }
+    return null;
+  };
+
   return {
     ...policy,
     circuitBreakerPolicy,
+    circuitBreakDuration,
+    getRemainingCircuitOpenDuration,
     retryPolicy,
     onBreak,
     onDegraded,

packages/network-controller/CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Bump `@metamask/base-controller` from `^8.1.0` to `^8.3.0` ([#6355](https://github.com/MetaMask/core/pull/6355), [#6465](https://github.com/MetaMask/core/pull/6465))
+- Rephrase "circuit broken" errors so they are more user-friendly ([#6423](https://github.com/MetaMask/core/pull/6423))
+  - These are errors produced when a request is made to an RPC endpoint after it returns too many consecutive 5xx responses and the underlying circuit is open.
 
 ### Deprecated
 

packages/network-controller/src/NetworkController.ts

@@ -2800,6 +2800,7 @@ export class NetworkController extends BaseController<
           getBlockTrackerOptions: this.#getBlockTrackerOptions,
           messenger: this.messagingSystem,
           isRpcFailoverEnabled: this.#isRpcFailoverEnabled,
+          logger: this.#log,
         });
       } else {
         autoManagedNetworkClientRegistry[NetworkClientType.Custom][
@@ -2816,6 +2817,7 @@ export class NetworkController extends BaseController<
           getBlockTrackerOptions: this.#getBlockTrackerOptions,
           messenger: this.messagingSystem,
           isRpcFailoverEnabled: this.#isRpcFailoverEnabled,
+          logger: this.#log,
         });
       }
     }
@@ -2978,6 +2980,7 @@ export class NetworkController extends BaseController<
               getBlockTrackerOptions: this.#getBlockTrackerOptions,
               messenger: this.messagingSystem,
               isRpcFailoverEnabled: this.#isRpcFailoverEnabled,
+              logger: this.#log,
             }),
           ] as const;
         }
@@ -2995,6 +2998,7 @@ export class NetworkController extends BaseController<
             getBlockTrackerOptions: this.#getBlockTrackerOptions,
             messenger: this.messagingSystem,
             isRpcFailoverEnabled: this.#isRpcFailoverEnabled,
+            logger: this.#log,
           }),
         ] as const;
       });

packages/network-controller/src/create-auto-managed-network-client.ts

@@ -1,4 +1,5 @@
 import type { PollingBlockTrackerOptions } from '@metamask/eth-block-tracker';
+import type { Logger } from 'loglevel';
 
 import type { NetworkClient } from './create-network-client';
 import { createNetworkClient } from './create-network-client';
@@ -76,6 +77,7 @@ const UNINITIALIZED_TARGET = { __UNINITIALIZED__: true };
  * @param args.isRpcFailoverEnabled - Whether or not requests sent to the
  * primary RPC endpoint for this network should be automatically diverted to
  * provided failover endpoints if the primary is unavailable.
+ * @param args.logger - A `loglevel` logger.
  * @returns The auto-managed network client.
  */
 export function createAutoManagedNetworkClient<
@@ -86,6 +88,7 @@ export function createAutoManagedNetworkClient<
   getBlockTrackerOptions = () => ({}),
   messenger,
   isRpcFailoverEnabled: givenIsRpcFailoverEnabled,
+  logger,
 }: {
   networkClientConfiguration: Configuration;
   getRpcServiceOptions: (
@@ -96,6 +99,7 @@ export function createAutoManagedNetworkClient<
   ) => Omit<PollingBlockTrackerOptions, 'provider'>;
   messenger: NetworkControllerMessenger;
   isRpcFailoverEnabled: boolean;
+  logger?: Logger;
 }): AutoManagedNetworkClient<Configuration> {
   let isRpcFailoverEnabled = givenIsRpcFailoverEnabled;
   let networkClient: NetworkClient | undefined;
@@ -107,6 +111,7 @@ export function createAutoManagedNetworkClient<
       getBlockTrackerOptions,
       messenger,
       isRpcFailoverEnabled,
+      logger,
     });
 
     if (networkClient === undefined) {

packages/network-controller/src/create-network-client.ts

@@ -25,6 +25,7 @@ import {
 } from '@metamask/json-rpc-engine';
 import type { JsonRpcMiddleware } from '@metamask/json-rpc-engine';
 import type { Hex, Json, JsonRpcParams } from '@metamask/utils';
+import type { Logger } from 'loglevel';
 
 import type { NetworkControllerMessenger } from './NetworkController';
 import type { RpcServiceOptions } from './rpc-service/rpc-service';
@@ -64,6 +65,7 @@ export type NetworkClient = {
  * provided failover endpoints if the primary is unavailable. This effectively
  * causes the `failoverRpcUrls` property of the network client configuration
  * to be honored or ignored.
+ * @param args.logger - A `loglevel` logger.
  * @returns The network client.
  */
 export function createNetworkClient({
@@ -72,6 +74,7 @@ export function createNetworkClient({
   getBlockTrackerOptions,
   messenger,
   isRpcFailoverEnabled,
+  logger,
 }: {
   configuration: NetworkClientConfiguration;
   getRpcServiceOptions: (
@@ -82,6 +85,7 @@ export function createNetworkClient({
   ) => Omit<PollingBlockTrackerOptions, 'provider'>;
   messenger: NetworkControllerMessenger;
   isRpcFailoverEnabled: boolean;
+  logger?: Logger;
 }): NetworkClient {
   const primaryEndpointUrl =
     configuration.type === NetworkClientType.Infura
@@ -94,6 +98,7 @@ export function createNetworkClient({
     availableEndpointUrls.map((endpointUrl) => ({
       ...getRpcServiceOptions(endpointUrl),
       endpointUrl,
+      logger,
     })),
   );
   rpcServiceChain.onBreak(({ endpointUrl, failoverEndpointUrl, ...rest }) => {