From 53072627bad17fe6ed23f82bfd51095a82cd4f9b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 15 Jun 2026 13:03:33 +0200 Subject: [PATCH 1/8] fix: Consider all HTTP errors as service failures except 429 --- .../src/create-service-policy.ts | 7 +++++- .../src/rpc-service/rpc-service.ts | 23 ++++++++++++++++--- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/controller-utils/src/create-service-policy.ts b/packages/controller-utils/src/create-service-policy.ts index 0c27fdf6155..dcac2149998 100644 --- a/packages/controller-utils/src/create-service-policy.ts +++ b/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; diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index c9904f43e61..c39f707763e 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -67,10 +67,13 @@ export type RpcServiceOptions = { */ logger?: Pick; /** - * 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; + 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 && + 'httpStatus' in error && + 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 From 79ba16615801c7f2506809f30124452516ff9f46 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 15 Jun 2026 13:25:14 +0200 Subject: [PATCH 2/8] Use hasProperty --- packages/network-controller/src/rpc-service/rpc-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index c39f707763e..cf2e2d5ee92 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -379,7 +379,7 @@ export class RpcService { if ( typeof error === 'object' && error !== null && - 'httpStatus' in error && + hasProperty(error, 'httpStatus') && typeof error.httpStatus === 'number' ) { return error.httpStatus !== 429; From 534e5c671867b5566d6dcaaf655167ffbfb9e229 Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Mon, 15 Jun 2026 14:24:54 +0200 Subject: [PATCH 3/8] Update CHANGELOG --- packages/controller-utils/CHANGELOG.md | 4 ++++ packages/network-controller/CHANGELOG.md | 1 + 2 files changed, 5 insertions(+) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index e20fed60150..4bce11d590c 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/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)) diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index fcff6929fd4..f1863bafe00 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/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 From 2cb1dcae72f4010445b663917ce2c03be0a43c4a Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 16 Jun 2026 10:29:39 +0200 Subject: [PATCH 4/8] Only apply fix for Infura and include HTTP status code 400 --- .../src/rpc-service/rpc-service.ts | 41 ++++++++++++------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index cf2e2d5ee92..ab68d94104c 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -287,6 +287,30 @@ function stripCredentialsFromUrl(url: URL): URL { return strippedUrl; } +const INFURA_NON_FAILURE_HTTP_STATUS_CODES = [400, 429]; + +/** + * Predicate function that determines if an error from Infura is treated as a service failure. + * + * @param error - The error. + * @returns True if the error should be treated as a service policy failure. Most errors are treated like failures, + * with the exception of certain HTTP status codes. + */ +function isServiceFailureInfura(error: unknown) { + if ( + typeof error === 'object' && + error !== null && + hasProperty(error, 'httpStatus') && + typeof error.httpStatus === 'number' + ) { + return !INFURA_NON_FAILURE_HTTP_STATUS_CODES.includes(error.httpStatus); + } + + // 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; +} + /** * This class is responsible for making a request to an endpoint that implements * the JSON-RPC protocol. It is designed to gracefully handle network and server @@ -371,24 +395,13 @@ export class RpcService { this.endpointUrl = stripCredentialsFromUrl(normalizedUrl); this.#logger = logger; + const isInfura = normalizedUrl.hostname.endsWith('infura.io'); + this.#policy = createServicePolicy({ 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; - }, + isServiceFailure: isInfura ? isServiceFailureInfura : undefined, retryFilterPolicy: handleWhen((error) => { // If user is offline, don't retry any errors // This prevents degraded/break callbacks from being triggered From 4aeb1d19db21dfa30edf869ed24f198cef3a8f3b Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 16 Jun 2026 10:31:00 +0200 Subject: [PATCH 5/8] Update CHANGELOG --- packages/network-controller/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index f1863bafe00..efee9f25923 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -26,7 +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)) +- Consider all Infura HTTP errors as service failures except `400` and `429` ([#9123](https://github.com/MetaMask/core/pull/9123)) ### Removed From 19e266557161ae575d3e92c9c15dd24c69002d0d Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 16 Jun 2026 10:44:01 +0200 Subject: [PATCH 6/8] Add return type --- packages/network-controller/src/rpc-service/rpc-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index ab68d94104c..06ec6677673 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -296,7 +296,7 @@ const INFURA_NON_FAILURE_HTTP_STATUS_CODES = [400, 429]; * @returns True if the error should be treated as a service policy failure. Most errors are treated like failures, * with the exception of certain HTTP status codes. */ -function isServiceFailureInfura(error: unknown) { +function isServiceFailureInfura(error: unknown): boolean { if ( typeof error === 'object' && error !== null && From a90c0768613ae98883c8abf81b1808f5cb9e464e Mon Sep 17 00:00:00 2001 From: Frederik Bolding Date: Tue, 16 Jun 2026 10:44:55 +0200 Subject: [PATCH 7/8] Tweak Infura check --- packages/network-controller/src/rpc-service/rpc-service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index 06ec6677673..482259bbb67 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -395,7 +395,7 @@ export class RpcService { this.endpointUrl = stripCredentialsFromUrl(normalizedUrl); this.#logger = logger; - const isInfura = normalizedUrl.hostname.endsWith('infura.io'); + const isInfura = normalizedUrl.hostname.endsWith('.infura.io'); this.#policy = createServicePolicy({ maxRetries: DEFAULT_MAX_RETRIES, From 02d5510eb097c7a8bd6e75d0daf5b18e0529fa0d Mon Sep 17 00:00:00 2001 From: cryptodev-2s <109512101+cryptodev-2s@users.noreply.github.com> Date: Wed, 17 Jun 2026 15:17:52 +0200 Subject: [PATCH 8/8] test: Cover isServiceFailure override and Infura failure classification (#9169) Follow up to #9123 to add the missing test coverage and fix a stale comment. Targets that PR's branch so it merges in before #9123 goes to main. ## What this adds `packages/controller-utils` (`create-service-policy.test.ts`): - Custom `isServiceFailure` opens the circuit when the predicate treats the error as a failure, and never opens it when it does not. - The predicate is called with the raw error thrown by the service. - The default predicate still applies when the option is omitted (breaks on `httpStatus >= 500`, not on `< 500`). `packages/network-controller` (`rpc-service.test.ts`): - On an Infura endpoint, 400 and 429 do not break the circuit, while 401 and 500 do. - A non Infura endpoint with the same config does not break on 401, which proves the Infura predicate is what changes the behavior. --- .../src/create-service-policy.test.ts | 112 ++++++++++++++++++ .../src/create-service-policy.ts | 5 +- .../src/rpc-service/rpc-service.test.ts | 83 +++++++++++++ .../src/rpc-service/rpc-service.ts | 5 +- 4 files changed, 201 insertions(+), 4 deletions(-) diff --git a/packages/controller-utils/src/create-service-policy.test.ts b/packages/controller-utils/src/create-service-policy.test.ts index 2d38112fe7a..27c5fcb7d19 100644 --- a/packages/controller-utils/src/create-service-policy.test.ts +++ b/packages/controller-utils/src/create-service-policy.test.ts @@ -3574,6 +3574,118 @@ describe('createServicePolicy', () => { await expect(policy.execute(mockService)).rejects.toThrow('failure'); }); }); + + describe('using a custom isServiceFailure predicate', () => { + it('opens the circuit when the predicate treats the error as a service failure', async () => { + const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; + const error = new Error('failure'); + const mockService = jest.fn(() => { + throw error; + }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ + maxConsecutiveFailures, + isServiceFailure: () => true, + }); + policy.onBreak(onBreakListener); + + const promise = policy.execute(mockService); + // It's safe not to await this promise; adding it to the promise + // queue is enough to prevent this test from running indefinitely. + // eslint-disable-next-line @typescript-eslint/no-floating-promises + jest.runAllTimersAsync(); + await ignoreRejection(promise); + + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); + }); + + it('never opens the circuit when the predicate does not treat the error as a service failure', async () => { + const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; + const error = new Error('failure'); + const mockService = jest.fn(() => { + throw error; + }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ + maxConsecutiveFailures, + isServiceFailure: () => false, + }); + policy.onBreak(onBreakListener); + + // Execute more times than the max consecutive failures so that the + // circuit would open if these errors were counted as failures. + for (let i = 0; i < maxConsecutiveFailures + 1; i++) { + const promise = policy.execute(mockService); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + jest.runAllTimersAsync(); + await ignoreRejection(promise); + } + + expect(onBreakListener).not.toHaveBeenCalled(); + }); + + it('calls the predicate with the error thrown by the service', async () => { + const error = new Error('failure'); + const mockService = jest.fn(() => { + throw error; + }); + const isServiceFailure = jest.fn(() => true); + const policy = createServicePolicy({ + maxConsecutiveFailures: DEFAULT_MAX_RETRIES + 1, + isServiceFailure, + }); + + const promise = policy.execute(mockService); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + jest.runAllTimersAsync(); + await ignoreRejection(promise); + + expect(isServiceFailure).toHaveBeenCalledWith(error); + }); + }); + + describe('using the default isServiceFailure predicate', () => { + it('opens the circuit for an error with an HTTP status >= 500', async () => { + const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; + const error = Object.assign(new Error('failure'), { httpStatus: 500 }); + const mockService = jest.fn(() => { + throw error; + }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxConsecutiveFailures }); + policy.onBreak(onBreakListener); + + const promise = policy.execute(mockService); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + jest.runAllTimersAsync(); + await ignoreRejection(promise); + + expect(onBreakListener).toHaveBeenCalledTimes(1); + }); + + it('never opens the circuit for an error with an HTTP status < 500', async () => { + const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; + const error = Object.assign(new Error('failure'), { httpStatus: 400 }); + const mockService = jest.fn(() => { + throw error; + }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxConsecutiveFailures }); + policy.onBreak(onBreakListener); + + // Execute more times than the max consecutive failures so that the + // circuit would open if these errors were counted as failures. + for (let i = 0; i < maxConsecutiveFailures + 1; i++) { + const promise = policy.execute(mockService); + // eslint-disable-next-line @typescript-eslint/no-floating-promises + jest.runAllTimersAsync(); + await ignoreRejection(promise); + } + + expect(onBreakListener).not.toHaveBeenCalled(); + }); + }); }); /** diff --git a/packages/controller-utils/src/create-service-policy.ts b/packages/controller-utils/src/create-service-policy.ts index dcac2149998..b7e5a91a142 100644 --- a/packages/controller-utils/src/create-service-policy.ts +++ b/packages/controller-utils/src/create-service-policy.ts @@ -203,8 +203,9 @@ const defaultIsServiceFailure = (error: unknown): boolean => { return error.httpStatus >= 500; } - // 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.) + // If the error is not an object, or doesn't have a numeric httpStatus + // property, consider it a service failure (e.g., network errors, timeouts, + // etc.) return true; }; diff --git a/packages/network-controller/src/rpc-service/rpc-service.test.ts b/packages/network-controller/src/rpc-service/rpc-service.test.ts index 9c4da444b26..309296cab77 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.test.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.test.ts @@ -344,6 +344,89 @@ describe('RpcService', () => { }); }); + describe('treating errors as service failures', () => { + const jsonRpcRequest = { + id: 1, + jsonrpc: '2.0' as const, + method: 'eth_chainId', + params: [], + }; + + describe('when the endpoint is an Infura URL', () => { + const endpointUrl = 'https://mainnet.infura.io'; + + it.each([400, 429])( + 'does not break the circuit when the endpoint responds with %d', + async (httpStatus) => { + nock(endpointUrl) + .post('/', jsonRpcRequest) + .times(3) + .reply(httpStatus); + const service = new RpcService({ + fetch, + btoa, + endpointUrl, + isOffline: (): boolean => false, + policyOptions: { maxConsecutiveFailures: 2 }, + }); + + // Make more requests than the max consecutive failures so that the + // circuit would open if these errors were treated as failures. + await ignoreRejection(service.request(jsonRpcRequest)); + await ignoreRejection(service.request(jsonRpcRequest)); + await ignoreRejection(service.request(jsonRpcRequest)); + + expect(service.getCircuitState()).toBe(CircuitState.Closed); + }, + ); + + it.each([401, 500])( + 'breaks the circuit when the endpoint responds with %d', + async (httpStatus) => { + nock(endpointUrl) + .post('/', jsonRpcRequest) + .times(2) + .reply(httpStatus); + const service = new RpcService({ + fetch, + btoa, + endpointUrl, + isOffline: (): boolean => false, + policyOptions: { maxConsecutiveFailures: 2 }, + }); + + await ignoreRejection(service.request(jsonRpcRequest)); + await ignoreRejection(service.request(jsonRpcRequest)); + + expect(service.getCircuitState()).toBe(CircuitState.Open); + }, + ); + }); + + describe('when the endpoint is not an Infura URL', () => { + const endpointUrl = 'https://rpc.example.chain'; + + it('does not break the circuit for a 4xx response that is not a server error', async () => { + nock(endpointUrl).post('/', jsonRpcRequest).times(3).reply(401); + const service = new RpcService({ + fetch, + btoa, + endpointUrl, + isOffline: (): boolean => false, + policyOptions: { maxConsecutiveFailures: 2 }, + }); + + // Make more requests than the max consecutive failures so that the + // circuit would open if these errors were treated as failures. + await ignoreRejection(service.request(jsonRpcRequest)); + await ignoreRejection(service.request(jsonRpcRequest)); + await ignoreRejection(service.request(jsonRpcRequest)); + + expect(service.getCircuitState()).toBe(CircuitState.Closed); + }); + }); + }); + describe('request', () => { // NOTE: Keep this list synced with CONNECTION_ERRORS describe.each([ diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index 482259bbb67..6e6cba3509e 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -306,8 +306,9 @@ function isServiceFailureInfura(error: unknown): boolean { return !INFURA_NON_FAILURE_HTTP_STATUS_CODES.includes(error.httpStatus); } - // 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.) + // If the error is not an object, or doesn't have a numeric httpStatus + // property, consider it a service failure (e.g., network errors, timeouts, + // etc.) return true; }