fix(rpc-service): Consider all HTTP errors as service failures except 429

packages/controller-utils/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Allow overriding `isServiceFailure` in `createServicePolicy` ([#9123](https://github.com/MetaMask/core/pull/9123))
+
 ### Changed
 
 - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074))

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

@@ -53,6 +53,10 @@ export type CreateServicePolicyOptions = {
    * regarded as degraded (affecting when `onDegraded` is called).
    */
   degradedThreshold?: number;
+  /**
+   * Predicate function for when an error should be considered a service failure.
+   */
+  isServiceFailure?: (error: unknown) => boolean;
   /**
    * The maximum number of times that the service is allowed to fail before
    * pausing further retries.
@@ -189,7 +193,7 @@ export const DEFAULT_CIRCUIT_BREAK_DURATION = 30 * 60 * 1000;
  */
 export const DEFAULT_DEGRADED_THRESHOLD = 5_000;
 
-const isServiceFailure = (error: unknown): boolean => {
+const defaultIsServiceFailure = (error: unknown): boolean => {
   if (
     typeof error === 'object' &&
     error !== null &&
@@ -283,6 +287,7 @@ export function createServicePolicy(
     circuitBreakDuration = DEFAULT_CIRCUIT_BREAK_DURATION,
     degradedThreshold = DEFAULT_DEGRADED_THRESHOLD,
     backoff = new ExponentialBackoff(),
+    isServiceFailure = defaultIsServiceFailure,
   } = options;
 
   let availabilityStatus: AvailabilityStatus = AVAILABILITY_STATUSES.Unknown;

packages/network-controller/CHANGELOG.md

@@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - The constructor argument `isRpcFailoverEnabled` is no longer available.
   - `RemoteFeatureFlagController:stateChange` and `RemoteFeatureFlagController:getState` are now required.
 - Drop `async-mutex` dependency, which was no longer used in source ([#9064](https://github.com/MetaMask/core/pull/9064))
+- Consider all HTTP errors as service failures except `429` ([#9123](https://github.com/MetaMask/core/pull/9123))
 
 ### Removed
 

packages/network-controller/src/rpc-service/rpc-service.ts

@@ -67,10 +67,13 @@ export type RpcServiceOptions = {
    */
   logger?: Pick<Logger, 'warn'>;
   /**
-   * Options to pass to `createServicePolicy`. Note that `retryFilterPolicy` is
-   * not accepted, as it is overwritten. See {@link createServicePolicy}.
+   * Options to pass to `createServicePolicy`. Note that `retryFilterPolicy` and `isServiceFailure`
+   * are not accepted, as they are overwritten. See {@link createServicePolicy}.
    */
-  policyOptions?: Omit<CreateServicePolicyOptions, 'retryFilterPolicy'>;
+  policyOptions?: Omit<
+    CreateServicePolicyOptions,
+    'retryFilterPolicy' | 'isServiceFailure'
+  >;
   /**
    * A function that checks if the user is currently offline. If it returns true,
    * connection errors will not be retried, preventing degraded and break
@@ -372,6 +375,20 @@ export class RpcService {
       maxRetries: DEFAULT_MAX_RETRIES,
       maxConsecutiveFailures: DEFAULT_MAX_CONSECUTIVE_FAILURES,
       ...policyOptions,
+      isServiceFailure: (error) => {
+        if (
+          typeof error === 'object' &&
+          error !== null &&
+          hasProperty(error, 'httpStatus') &&
+          typeof error.httpStatus === 'number'
+        ) {
+          return error.httpStatus !== 429;
+        }
+
+        // If the error is not an object, or doesn't have a numeric code property,
+        // consider it a service failure (e.g., network errors, timeouts, etc.)
+        return true;
+      },
       retryFilterPolicy: handleWhen((error) => {
         // If user is offline, don't retry any errors
         // This prevents degraded/break callbacks from being triggered