From 32641901f879fb2493321e54f7a63c9dcd22f8ec Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 17 Jun 2026 14:16:48 +0200 Subject: [PATCH 1/2] Add tests for isServiceFailure and Infura failure classification Cover the new `isServiceFailure` override option in `createServicePolicy` (custom predicate is honored, called with the raw error, and the default still applies when omitted) and the Infura specific classification in `RpcService` (400 and 429 do not break the circuit, other errors like 401 and 500 do, and non Infura endpoints are unaffected). Also fix a stale comment in both predicates that referred to a "code" property where the check is actually on `httpStatus`. --- .../src/create-service-policy.test.ts | 112 ++++++++++++++++++ .../src/create-service-policy.ts | 5 +- .../src/rpc-service/rpc-service.test.ts | 77 ++++++++++++ .../src/rpc-service/rpc-service.ts | 5 +- 4 files changed, 195 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..a8b1636e249 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,83 @@ 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; } From 37eff5a5047c78201f14df5366fad7eaad232cb2 Mon Sep 17 00:00:00 2001 From: Salah-Eddine Saakoun Date: Wed, 17 Jun 2026 14:46:35 +0200 Subject: [PATCH 2/2] Fix formatting in rpc-service tests --- .../src/rpc-service/rpc-service.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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 a8b1636e249..309296cab77 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.test.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.test.ts @@ -358,7 +358,10 @@ describe('RpcService', () => { 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); + nock(endpointUrl) + .post('/', jsonRpcRequest) + .times(3) + .reply(httpStatus); const service = new RpcService({ fetch, btoa, @@ -380,7 +383,10 @@ describe('RpcService', () => { it.each([401, 500])( 'breaks the circuit when the endpoint responds with %d', async (httpStatus) => { - nock(endpointUrl).post('/', jsonRpcRequest).times(2).reply(httpStatus); + nock(endpointUrl) + .post('/', jsonRpcRequest) + .times(2) + .reply(httpStatus); const service = new RpcService({ fetch, btoa,