fix(rpc-service): handle 405 and 429 status codes without triggering circuit breaker

packages/network-controller/CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Improved handling of HTTP status codes to prevent unnecessary circuit breaker triggers ([#5767](https://github.com/MetaMask/core/pull/5767))
+  - 405 (Method Not Allowed) now returns a JSON-RPC error with code -32601 (Method not found)
+  - 429 (Too Many Requests) now returns a JSON-RPC error with code -32005 (Request rate limit exceeded)
+  - Added retry delay information in 429 error responses when available via Retry-After header
+  - These status codes no longer trigger the circuit breaker, preventing unnecessary failover to backup endpoints
+
 ## [23.4.0]
 
 ### Added

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

@@ -214,7 +214,9 @@ describe('RpcService', () => {
           method: 'eth_chainId',
           params: [],
         };
-        await ignoreRejection(service.request(jsonRpcRequest));
+        await expect(service.request(jsonRpcRequest)).rejects.toThrow(
+          'Nock: No match for request',
+        );
         expect(failoverService.request).not.toHaveBeenCalled();
       });
 
@@ -236,13 +238,14 @@ describe('RpcService', () => {
         });
         service.onBreak(onBreakListener);
 
-        const promise = service.request({
-          id: 1,
-          jsonrpc: '2.0',
-          method: 'eth_chainId',
-          params: [],
-        });
-        await ignoreRejection(promise);
+        await expect(
+          service.request({
+            id: 1,
+            jsonrpc: '2.0',
+            method: 'eth_chainId',
+            params: [],
+          }),
+        ).rejects.toThrow('Nock: No match for request');
         expect(onBreakListener).not.toHaveBeenCalled();
       });
     });
@@ -331,7 +334,7 @@ describe('RpcService', () => {
     );
 
     describe('if the endpoint has a 405 response', () => {
-      it('throws a non-existent method error without retrying the request', async () => {
+      it('returns a method not supported error response for 405 status', async () => {
         const endpointUrl = 'https://rpc.example.chain';
         nock(endpointUrl)
           .post('/', {
@@ -347,15 +350,21 @@ describe('RpcService', () => {
           endpointUrl,
         });
 
-        const promise = service.request({
+        const response = await service.request({
           id: 1,
           jsonrpc: '2.0',
           method: 'eth_unknownMethod',
           params: [],
         });
-        await expect(promise).rejects.toThrow(
-          'The method does not exist / is not available.',
-        );
+
+        expect(response).toStrictEqual({
+          id: 1,
+          jsonrpc: '2.0',
+          error: {
+            code: -32601,
+            message: 'The method does not exist / is not available.',
+          },
+        });
       });
 
       it('does not forward the request to a failover service if given one', async () => {
@@ -382,7 +391,7 @@ describe('RpcService', () => {
           method: 'eth_unknownMethod',
           params: [],
         };
-        await ignoreRejection(service.request(jsonRpcRequest));
+        await service.request(jsonRpcRequest);
         expect(failoverService.request).not.toHaveBeenCalled();
       });
 
@@ -404,19 +413,18 @@ describe('RpcService', () => {
         });
         service.onBreak(onBreakListener);
 
-        const promise = service.request({
+        await service.request({
           id: 1,
           jsonrpc: '2.0',
           method: 'eth_unknownMethod',
           params: [],
         });
-        await ignoreRejection(promise);
         expect(onBreakListener).not.toHaveBeenCalled();
       });
     });
 
     describe('if the endpoint has a 429 response', () => {
-      it('throws a rate-limiting error without retrying the request', async () => {
+      it('returns a rate limit error response for 429 status', async () => {
         const endpointUrl = 'https://rpc.example.chain';
         nock(endpointUrl)
           .post('/', {
@@ -432,13 +440,62 @@ describe('RpcService', () => {
           endpointUrl,
         });
 
-        const promise = service.request({
+        const response = await service.request({
+          id: 1,
+          jsonrpc: '2.0',
+          method: 'eth_chainId',
+          params: [],
+        });
+
+        expect(response).toStrictEqual({
+          id: 1,
+          jsonrpc: '2.0',
+          error: {
+            code: -32005,
+            message: 'Request is being rate limited.',
+            data: {
+              retryAfter: 1000,
+            },
+          },
+        });
+      });
+
+      it('includes retry delay from Retry-After header when available', async () => {
+        const endpointUrl = 'https://rpc.example.chain';
+        nock(endpointUrl)
+          .post('/', {
+            id: 1,
+            jsonrpc: '2.0',
+            method: 'eth_chainId',
+            params: [],
+          })
+          .reply(429, '', {
+            'Retry-After': '5',
+          });
+        const service = new RpcService({
+          fetch,
+          btoa,
+          endpointUrl,
+        });
+
+        const response = await service.request({
           id: 1,
           jsonrpc: '2.0',
           method: 'eth_chainId',
           params: [],
         });
-        await expect(promise).rejects.toThrow('Request is being rate limited.');
+
+        expect(response).toStrictEqual({
+          id: 1,
+          jsonrpc: '2.0',
+          error: {
+            code: -32005,
+            message: 'Request is being rate limited.',
+            data: {
+              retryAfter: 5000,
+            },
+          },
+        });
       });
 
       it('does not forward the request to a failover service if given one', async () => {
@@ -465,7 +522,7 @@ describe('RpcService', () => {
           method: 'eth_chainId',
           params: [],
         };
-        await ignoreRejection(service.request(jsonRpcRequest));
+        await service.request(jsonRpcRequest);
         expect(failoverService.request).not.toHaveBeenCalled();
       });
 
@@ -487,13 +544,12 @@ describe('RpcService', () => {
         });
         service.onBreak(onBreakListener);
 
-        const promise = service.request({
+        await service.request({
           id: 1,
           jsonrpc: '2.0',
           method: 'eth_chainId',
           params: [],
         });
-        await ignoreRejection(promise);
         expect(onBreakListener).not.toHaveBeenCalled();
       });
     });

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

@@ -333,12 +333,10 @@ export class RpcService implements AbstractRpcService {
    * @param jsonRpcRequest - The JSON-RPC request to send to the endpoint.
    * @param fetchOptions - An options bag for {@link fetch} which further
    * specifies the request.
-   * @returns The decoded JSON-RPC response from the endpoint.
-   * @throws A "method not found" error if the response status is 405.
-   * @throws A rate limiting error if the response HTTP status is 429.
+   * @returns The decoded JSON-RPC response from the endpoint. For 405 status, returns a JSON-RPC error response with code -32601.
+   * For 429 status, returns a JSON-RPC error response with code -32005.
    * @throws A timeout error if the response HTTP status is 503 or 504.
-   * @throws A generic error if the response HTTP status is not 2xx but also not
-   * 405, 429, 503, or 504.
+   * @throws A generic error if the response HTTP status is not 2xx and not one of the specifically handled status codes (405, 429, 503, 504).
    */
   async request<Params extends JsonRpcParams, Result extends Json>(
     jsonRpcRequest: JsonRpcRequest<Params> & { method: 'eth_getBlockByNumber' },
@@ -357,12 +355,10 @@ export class RpcService implements AbstractRpcService {
    * @param jsonRpcRequest - The JSON-RPC request to send to the endpoint.
    * @param fetchOptions - An options bag for {@link fetch} which further
    * specifies the request.
-   * @returns The decoded JSON-RPC response from the endpoint.
-   * @throws A "method not found" error if the response status is 405.
-   * @throws A rate limiting error if the response HTTP status is 429.
+   * @returns The decoded JSON-RPC response from the endpoint. For 405 status, returns a JSON-RPC error response with code -32601.
+   * For 429 status, returns a JSON-RPC error response with code -32005.
    * @throws A timeout error if the response HTTP status is 503 or 504.
-   * @throws A generic error if the response HTTP status is not 2xx but also not
-   * 405, 429, 503, or 504.
+   * @throws A generic error if the response HTTP status is not 2xx and not one of the specifically handled status codes (405, 429, 503, 504).
    */
   async request<Params extends JsonRpcParams, Result extends Json>(
     jsonRpcRequest: JsonRpcRequest<Params>,
@@ -465,12 +461,10 @@ export class RpcService implements AbstractRpcService {
    * @param jsonRpcRequest - The JSON-RPC request to send to the endpoint.
    * @param fetchOptions - The options for `fetch`; will be combined with the
    * fetch options passed to the constructor
-   * @returns The decoded JSON-RPC response from the endpoint.
-   * @throws A "method not found" error if the response status is 405.
-   * @throws A rate limiting error if the response HTTP status is 429.
+   * @returns The decoded JSON-RPC response from the endpoint. For 405 status, returns a JSON-RPC error response with code -32601.
+   * For 429 status, returns a JSON-RPC error response with code -32005.
    * @throws A timeout error if the response HTTP status is 503 or 504.
-   * @throws A generic error if the response HTTP status is not 2xx but also not
-   * 405, 429, 503, or 504.
+   * @throws A generic error if the response HTTP status is not 2xx and not one of the specifically handled status codes (405, 429, 503, 504).
    */
   async #executePolicy<
     Params extends JsonRpcParams,
@@ -484,11 +478,31 @@ export class RpcService implements AbstractRpcService {
       const response = await this.#fetch(this.endpointUrl, fetchOptions);
 
       if (response.status === 405) {
-        throw rpcErrors.methodNotFound();
+        return {
+          id: jsonRpcRequest.id,
+          jsonrpc: jsonRpcRequest.jsonrpc,
+          error: {
+            code: -32601,
+            message: 'The method does not exist / is not available.',
+          },
+        };
       }
 
       if (response.status === 429) {
-        throw rpcErrors.internal({ message: 'Request is being rate limited.' });
+        const retryAfter = response.headers.get('Retry-After');
+        const retryDelay = retryAfter ? parseInt(retryAfter, 10) * 1000 : 1000;
+
+        return {
+          id: jsonRpcRequest.id,
+          jsonrpc: jsonRpcRequest.jsonrpc,
+          error: {
+            code: -32005,
+            message: 'Request is being rate limited.',
+            data: {
+              retryAfter: retryDelay,
+            },
+          },
+        };
       }
 
       if (response.status === 503 || response.status === 504) {