Skip to content

Commit 3fb3df4

Browse files
cryptodev-2scryptodev-2s
authored
fix(rpc-service): improve error handling for HTTP status codes (#5843)
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> Improves error handling in the RPC service by making it more specific and consistent. The changes include: - Clarifies error handling for different HTTP status codes: - 401: Unauthorized error - 402/404/5xx: Resource unavailable error - 405/501: Method not found error - 429: Rate limiting error - Other 4xx: Invalid request error - Invalid JSON: Parse error ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> Fixes #5844 ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 0e40a5b commit 3fb3df4

File tree

8 files changed

+506
-241
lines changed

8 files changed

+506
-241
lines changed

‎packages/network-controller/CHANGELOG.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Improved error handling in RPC service with more specific error types ([#5843](https://github.com/MetaMask/core/pull/5843)):
13+
- 401 responses now throw an "Unauthorized" error
14+
- 402/404/5xx responses now throw a "Resource Unavailable" error
15+
- 429 responses now throw a "Rate Limiting" error
16+
- Other 4xx responses now throw a generic HTTP client error
17+
- Invalid JSON responses now throw a "Parse" error
18+
1019
## [23.5.0]
1120

1221
### Changed

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

Lines changed: 148 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -193,22 +193,46 @@ describe('RpcServiceChain', () => {
193193
};
194194
// Retry the first endpoint until max retries is hit.
195195
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
196-
'Gateway timeout',
196+
expect.objectContaining({
197+
code: -32002,
198+
message: 'RPC endpoint not found or unavailable.',
199+
data: {
200+
httpStatus: 503,
201+
},
202+
}),
197203
);
198204
// Retry the first endpoint again, until max retries is hit.
199205
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
200-
'Gateway timeout',
206+
expect.objectContaining({
207+
code: -32002,
208+
message: 'RPC endpoint not found or unavailable.',
209+
data: {
210+
httpStatus: 503,
211+
},
212+
}),
201213
);
202214
// Retry the first endpoint for a third time, until max retries is hit.
203215
// The circuit will break on the last time, and the second endpoint will
204216
// be retried, until max retries is hit.
205217
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
206-
'Gateway timeout',
218+
expect.objectContaining({
219+
code: -32002,
220+
message: 'RPC endpoint not found or unavailable.',
221+
data: {
222+
httpStatus: 503,
223+
},
224+
}),
207225
);
208226
// Try the first endpoint, see that the circuit is broken, and retry the
209227
// second endpoint, until max retries is hit.
210228
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
211-
'Gateway timeout',
229+
expect.objectContaining({
230+
code: -32002,
231+
message: 'RPC endpoint not found or unavailable.',
232+
data: {
233+
httpStatus: 503,
234+
},
235+
}),
212236
);
213237
// Try the first endpoint, see that the circuit is broken, and retry the
214238
// second endpoint, until max retries is hit.
@@ -315,22 +339,54 @@ describe('RpcServiceChain', () => {
315339
// Retry the first endpoint until max retries is hit.
316340
await expect(
317341
rpcServiceChain.request(jsonRpcRequest, fetchOptions),
318-
).rejects.toThrow('Gateway timeout');
342+
).rejects.toThrow(
343+
expect.objectContaining({
344+
code: -32002,
345+
message: 'RPC endpoint not found or unavailable.',
346+
data: {
347+
httpStatus: 503,
348+
},
349+
}),
350+
);
319351
// Retry the first endpoint again, until max retries is hit.
320352
await expect(
321353
rpcServiceChain.request(jsonRpcRequest, fetchOptions),
322-
).rejects.toThrow('Gateway timeout');
354+
).rejects.toThrow(
355+
expect.objectContaining({
356+
code: -32002,
357+
message: 'RPC endpoint not found or unavailable.',
358+
data: {
359+
httpStatus: 503,
360+
},
361+
}),
362+
);
323363
// Retry the first endpoint for a third time, until max retries is hit.
324364
// The circuit will break on the last time, and the second endpoint will
325365
// be retried, until max retries is hit.
326366
await expect(
327367
rpcServiceChain.request(jsonRpcRequest, fetchOptions),
328-
).rejects.toThrow('Gateway timeout');
368+
).rejects.toThrow(
369+
expect.objectContaining({
370+
code: -32002,
371+
message: 'RPC endpoint not found or unavailable.',
372+
data: {
373+
httpStatus: 503,
374+
},
375+
}),
376+
);
329377
// Try the first endpoint, see that the circuit is broken, and retry the
330378
// second endpoint, until max retries is hit.
331379
await expect(
332380
rpcServiceChain.request(jsonRpcRequest, fetchOptions),
333-
).rejects.toThrow('Gateway timeout');
381+
).rejects.toThrow(
382+
expect.objectContaining({
383+
code: -32002,
384+
message: 'RPC endpoint not found or unavailable.',
385+
data: {
386+
httpStatus: 503,
387+
},
388+
}),
389+
);
334390
// Try the first endpoint, see that the circuit is broken, and retry the
335391
// second endpoint, until max retries is hit.
336392
// The circuit will break on the last time, and the third endpoint will
@@ -415,22 +471,46 @@ describe('RpcServiceChain', () => {
415471
};
416472
// Retry the first endpoint until max retries is hit.
417473
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
418-
'Gateway timeout',
474+
expect.objectContaining({
475+
code: -32002,
476+
message: 'RPC endpoint not found or unavailable.',
477+
data: {
478+
httpStatus: 503,
479+
},
480+
}),
419481
);
420482
// Retry the first endpoint again, until max retries is hit.
421483
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
422-
'Gateway timeout',
484+
expect.objectContaining({
485+
code: -32002,
486+
message: 'RPC endpoint not found or unavailable.',
487+
data: {
488+
httpStatus: 503,
489+
},
490+
}),
423491
);
424492
// Retry the first endpoint for a third time, until max retries is hit.
425493
// The circuit will break on the last time, and the second endpoint will
426494
// be retried, until max retries is hit.
427495
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
428-
'Gateway timeout',
496+
expect.objectContaining({
497+
code: -32002,
498+
message: 'RPC endpoint not found or unavailable.',
499+
data: {
500+
httpStatus: 503,
501+
},
502+
}),
429503
);
430504
// Try the first endpoint, see that the circuit is broken, and retry the
431505
// second endpoint, until max retries is hit.
432506
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
433-
'Gateway timeout',
507+
expect.objectContaining({
508+
code: -32002,
509+
message: 'RPC endpoint not found or unavailable.',
510+
data: {
511+
httpStatus: 503,
512+
},
513+
}),
434514
);
435515
// Try the first endpoint, see that the circuit is broken, and retry the
436516
// second endpoint, until max retries is hit.
@@ -530,22 +610,46 @@ describe('RpcServiceChain', () => {
530610
};
531611
// Retry the first endpoint until max retries is hit.
532612
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
533-
'Gateway timeout',
613+
expect.objectContaining({
614+
code: -32002,
615+
message: 'RPC endpoint not found or unavailable.',
616+
data: {
617+
httpStatus: 503,
618+
},
619+
}),
534620
);
535621
// Retry the first endpoint again, until max retries is hit.
536622
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
537-
'Gateway timeout',
623+
expect.objectContaining({
624+
code: -32002,
625+
message: 'RPC endpoint not found or unavailable.',
626+
data: {
627+
httpStatus: 503,
628+
},
629+
}),
538630
);
539631
// Retry the first endpoint for a third time, until max retries is hit.
540632
// The circuit will break on the last time, and the second endpoint will
541633
// be retried, until max retries is hit.
542634
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
543-
'Gateway timeout',
635+
expect.objectContaining({
636+
code: -32002,
637+
message: 'RPC endpoint not found or unavailable.',
638+
data: {
639+
httpStatus: 503,
640+
},
641+
}),
544642
);
545643
// Try the first endpoint, see that the circuit is broken, and retry the
546644
// second endpoint, until max retries is hit.
547645
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
548-
'Gateway timeout',
646+
expect.objectContaining({
647+
code: -32002,
648+
message: 'RPC endpoint not found or unavailable.',
649+
data: {
650+
httpStatus: 503,
651+
},
652+
}),
549653
);
550654
// Try the first endpoint, see that the circuit is broken, and retry the
551655
// second endpoint, until max retries is hit.
@@ -645,22 +749,46 @@ describe('RpcServiceChain', () => {
645749
};
646750
// Retry the first endpoint until max retries is hit.
647751
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
648-
'Gateway timeout',
752+
expect.objectContaining({
753+
code: -32002,
754+
message: 'RPC endpoint not found or unavailable.',
755+
data: {
756+
httpStatus: 503,
757+
},
758+
}),
649759
);
650760
// Retry the first endpoint again, until max retries is hit.
651761
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
652-
'Gateway timeout',
762+
expect.objectContaining({
763+
code: -32002,
764+
message: 'RPC endpoint not found or unavailable.',
765+
data: {
766+
httpStatus: 503,
767+
},
768+
}),
653769
);
654770
// Retry the first endpoint for a third time, until max retries is hit.
655771
// The circuit will break on the last time, and the second endpoint will
656772
// be retried, until max retries is hit.
657773
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
658-
'Gateway timeout',
774+
expect.objectContaining({
775+
code: -32002,
776+
message: 'RPC endpoint not found or unavailable.',
777+
data: {
778+
httpStatus: 503,
779+
},
780+
}),
659781
);
660782
// Try the first endpoint, see that the circuit is broken, and retry the
661783
// second endpoint, until max retries is hit.
662784
await expect(rpcServiceChain.request(jsonRpcRequest)).rejects.toThrow(
663-
'Gateway timeout',
785+
expect.objectContaining({
786+
code: -32002,
787+
message: 'RPC endpoint not found or unavailable.',
788+
data: {
789+
httpStatus: 503,
790+
},
791+
}),
664792
);
665793
// Try the first endpoint, see that the circuit is broken, and retry the
666794
// second endpoint, until max retries is hit.

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ export class RpcServiceChain implements RpcServiceRequestable {
9999
* @param fetchOptions - An options bag for {@link fetch} which further
100100
* specifies the request.
101101
* @returns The decoded JSON-RPC response from the endpoint.
102-
* @throws A "method not found" error if the response status is 405.
103-
* @throws A rate limiting error if the response HTTP status is 429.
104-
* @throws A timeout error if the response HTTP status is 503 or 504.
105-
* @throws A generic error if the response HTTP status is not 2xx but also not
106-
* 405, 429, 503, or 504.
102+
* @throws A 401 error if the response status is 401.
103+
* @throws A "rate limiting" error if the response HTTP status is 429.
104+
* @throws A "resource unavailable" error if the response status is 402, 404, or any 5xx.
105+
* @throws A generic HTTP client error (-32100) for any other 4xx status codes.
106+
* @throws A "parse" error if the response is not valid JSON.
107107
*/
108108
async request<Params extends JsonRpcParams, Result extends Json>(
109109
jsonRpcRequest: JsonRpcRequest<Params> & { method: 'eth_getBlockByNumber' },
@@ -122,11 +122,11 @@ export class RpcServiceChain implements RpcServiceRequestable {
122122
* @param fetchOptions - An options bag for {@link fetch} which further
123123
* specifies the request.
124124
* @returns The decoded JSON-RPC response from the endpoint.
125-
* @throws A "method not found" error if the response status is 405.
126-
* @throws A rate limiting error if the response HTTP status is 429.
127-
* @throws A timeout error if the response HTTP status is 503 or 504.
128-
* @throws A generic error if the response HTTP status is not 2xx but also not
129-
* 405, 429, 503, or 504.
125+
* @throws A 401 error if the response status is 401.
126+
* @throws A "rate limiting" error if the response HTTP status is 429.
127+
* @throws A "resource unavailable" error if the response status is 402, 404, or any 5xx.
128+
* @throws A generic HTTP client error (-32100) for any other 4xx status codes.
129+
* @throws A "parse" error if the response is not valid JSON.
130130
*/
131131
async request<Params extends JsonRpcParams, Result extends Json>(
132132
jsonRpcRequest: JsonRpcRequest<Params>,

0 commit comments

Comments
 (0)