From 889b5df95bc8089c56dcc14737d68426e55fcbe2 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 24 Jul 2025 13:56:29 -0600 Subject: [PATCH 1/9] WIP - Add error to NetworkController:rpcEndpointDegraded --- .../src/create-service-policy.ts | 10 +++-- packages/network-controller/CHANGELOG.md | 4 ++ .../src/NetworkController.ts | 1 + .../src/create-network-client.ts | 10 ++++- .../src/rpc-service/rpc-service.ts | 4 +- .../src/rpc-service/shared.ts | 40 +++++++++++++++++-- 6 files changed, 58 insertions(+), 11 deletions(-) diff --git a/packages/controller-utils/src/create-service-policy.ts b/packages/controller-utils/src/create-service-policy.ts index e2028b42dce..fbce1ed88d8 100644 --- a/packages/controller-utils/src/create-service-policy.ts +++ b/packages/controller-utils/src/create-service-policy.ts @@ -14,6 +14,7 @@ import { import type { CircuitBreakerPolicy, Event as CockatielEvent, + FailureReason, IBackoffFactory, IPolicy, Policy, @@ -95,7 +96,7 @@ export type ServicePolicy = IPolicy & { * never succeeds before the retry policy gives up and before the maximum * number of consecutive failures has been reached. */ - onDegraded: CockatielEvent; + onDegraded: CockatielEvent | void>; /** * A function which will be called by the retry policy each time the service * fails and the policy kicks off a timer to re-run the service. This is @@ -229,10 +230,11 @@ export function createServicePolicy( }); const onBreak = circuitBreakerPolicy.onBreak.bind(circuitBreakerPolicy); - const onDegradedEventEmitter = new CockatielEventEmitter(); - retryPolicy.onGiveUp(() => { + const onDegradedEventEmitter = + new CockatielEventEmitter | void>(); + retryPolicy.onGiveUp((data) => { if (circuitBreakerPolicy.state === CircuitState.Closed) { - onDegradedEventEmitter.emit(); + onDegradedEventEmitter.emit(data); } }); retryPolicy.onSuccess(({ duration }) => { diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index 946c1f13a2f..29232b31138 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** `NetworkController:rpcEndpointDegraded` now includes an extra `error` property, which will be produced when exceeding the maximum number of retries for the request + ## [24.0.1] ### Changed diff --git a/packages/network-controller/src/NetworkController.ts b/packages/network-controller/src/NetworkController.ts index d23f6d55aae..ceaef81f810 100644 --- a/packages/network-controller/src/NetworkController.ts +++ b/packages/network-controller/src/NetworkController.ts @@ -470,6 +470,7 @@ export type NetworkControllerRpcEndpointDegradedEvent = { { chainId: Hex; endpointUrl: string; + error: unknown; }, ]; }; diff --git a/packages/network-controller/src/create-network-client.ts b/packages/network-controller/src/create-network-client.ts index c6388ae3c18..7ac7f920dc9 100644 --- a/packages/network-controller/src/create-network-client.ts +++ b/packages/network-controller/src/create-network-client.ts @@ -111,10 +111,18 @@ export function createNetworkClient({ error, }); }); - rpcServiceChain.onDegraded(({ endpointUrl }) => { + rpcServiceChain.onDegraded(({ endpointUrl, ...rest }) => { + let error: unknown; + if ('error' in rest) { + error = rest.error; + } else if ('value' in rest) { + error = rest.value; + } + messenger.publish('NetworkController:rpcEndpointDegraded', { chainId: configuration.chainId, endpointUrl, + error, }); }); rpcServiceChain.onRetry(({ endpointUrl, attempt }) => { diff --git a/packages/network-controller/src/rpc-service/rpc-service.ts b/packages/network-controller/src/rpc-service/rpc-service.ts index 383b98d3605..037429364ac 100644 --- a/packages/network-controller/src/rpc-service/rpc-service.ts +++ b/packages/network-controller/src/rpc-service/rpc-service.ts @@ -361,8 +361,8 @@ export class RpcService implements AbstractRpcService { { endpointUrl: string } >, ) { - return this.#policy.onDegraded(() => { - listener({ endpointUrl: this.endpointUrl.toString() }); + return this.#policy.onDegraded((data) => { + listener({ ...(data ?? {}), endpointUrl: this.endpointUrl.toString() }); }); } diff --git a/packages/network-controller/src/rpc-service/shared.ts b/packages/network-controller/src/rpc-service/shared.ts index 68e4c78b250..844788a85fc 100644 --- a/packages/network-controller/src/rpc-service/shared.ts +++ b/packages/network-controller/src/rpc-service/shared.ts @@ -7,10 +7,42 @@ export type FetchOptions = RequestInit; * Extends an event listener that Cockatiel uses so that when it is called, more * data can be supplied in the event object. */ +// If we have: +// +// AddToCockatielEventData< +// (data: void | { error: Error } | { value: unknown }) => void, +// { foo: 'bar' } +// > +// +// we want this to resolve as: +// +// (data: { foo: 'bar' } | ({ error: Error } & { foo: 'bar' }) | ({ value: unknown } & { foo: 'bar' })) => void +// +// To do this we need to get TypeScript to distribute +// `void | { error: Error } | { value: unknown }) => void` over the condition +// (`Data extends void ? ... : ...`). The key to doing this right is placing the +// condition right at the point where we need to know what `Data` is and not +// before (that is, before `(data ...) => void`). If we did this, the above +// would resolve as: +// +// (data: ((void | { error: Error } | { value: unknown }) & { foo: 'bar' }) => void +// +// which distributes to: +// +// (data: (void & { foo: 'bar' }) | ({ error: Error } & { foo: 'bar' }) | ({ value: unknown } & { foo: 'bar' })) => void +// +// (which is not correct because `void & { foo: 'bar' }` doesn't make sense). +// +// There is some information on distributive conditional types here: +// +// It is pretty technical, but the key insight is: "Distributive conditional +// types are automatically distributed over union types during instantiation." +// In this case, we need to place the condition so it becomes a part of the type +// that we want to "return" from this utility type and is not used to simply +// determine the return type. This way TypeScript evaluates the condition +// when that return type is used and not beforehand. +// export type AddToCockatielEventData = EventListener extends (data: infer Data) => void - ? // Prevent Data from being split if it's a type union - [Data] extends [void] - ? (data: AdditionalData) => void - : (data: Data & AdditionalData) => void + ? (data: Data extends void ? AdditionalData : Data & AdditionalData) => void : never; From 1e29c4420601b5fcccdcbed478bd9f3d51ebf2e8 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 24 Jul 2025 15:09:06 -0600 Subject: [PATCH 2/9] Update changelog --- packages/controller-utils/CHANGELOG.md | 5 +++++ packages/network-controller/CHANGELOG.md | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index c1b196e62af..3898f49419e 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([???](https://github.com/MetaMask/core/pull/6054)) + - This `error` property will be `undefined` if the degraded event represents a slow request + ## [11.11.0] ### Added diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index 29232b31138..b9fb1f6e2af 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **BREAKING:** `NetworkController:rpcEndpointDegraded` now includes an extra `error` property, which will be produced when exceeding the maximum number of retries for the request +- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([???](https://github.com/MetaMask/core/pull/6054)) ## [24.0.1] From 7dc0e905fe7cd6bfc2fce0796e6dd912098d1337 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 24 Jul 2025 15:09:55 -0600 Subject: [PATCH 3/9] Fix changelog --- packages/controller-utils/CHANGELOG.md | 2 +- packages/network-controller/CHANGELOG.md | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 3898f49419e..c691d8a6fe3 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([???](https://github.com/MetaMask/core/pull/6054)) +- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6168)) - This `error` property will be `undefined` if the degraded event represents a slow request ## [11.11.0] diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index b9fb1f6e2af..348d10478b7 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([???](https://github.com/MetaMask/core/pull/6054)) +- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6168)) + - This `error` property will be `undefined` if the degraded event represents a slow request ## [24.0.1] From 4693d479d43d8cdf1aef80a0b77e1e7c1f0160ee Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 24 Jul 2025 15:12:41 -0600 Subject: [PATCH 4/9] Recategorize under Added --- 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 348d10478b7..e9455972ead 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Changed +### Added - **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6168)) - This `error` property will be `undefined` if the degraded event represents a slow request From 54cf37f022370869b6f136bbd82aa114c1b36aae Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 6 Aug 2025 13:43:53 -0600 Subject: [PATCH 5/9] Fix changelog --- packages/controller-utils/CHANGELOG.md | 2 +- packages/network-controller/CHANGELOG.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index c691d8a6fe3..c886e8d12cc 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6168)) +- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6188)) - This `error` property will be `undefined` if the degraded event represents a slow request ## [11.11.0] diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index e9455972ead..4614ff59a2d 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6168)) +- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6188)) - This `error` property will be `undefined` if the degraded event represents a slow request ## [24.0.1] From 0b19e79dc6c857ed7442070ad4bd3f329e6c2455 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 6 Aug 2025 13:54:07 -0600 Subject: [PATCH 6/9] Fix changelog again --- 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 4614ff59a2d..45e5a095b0c 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6188)) +- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) - This `error` property will be `undefined` if the degraded event represents a slow request ## [24.0.1] From 611dc37c691a0d1fbcab59aa023080e0c5157c78 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 6 Aug 2025 15:35:50 -0600 Subject: [PATCH 7/9] Really really fix changelog --- packages/controller-utils/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index c886e8d12cc..0d37391654e 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6168](https://github.com/MetaMask/core/pull/6188)) +- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) - This `error` property will be `undefined` if the degraded event represents a slow request ## [11.11.0] From 25772fcb386f609c6fe2844eda0d0d30f8c94955 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 6 Aug 2025 16:17:14 -0600 Subject: [PATCH 8/9] Downgrade the changes to non-breaking --- packages/controller-utils/CHANGELOG.md | 4 ++-- packages/network-controller/CHANGELOG.md | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 0d37391654e..daa01a7c5e4 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) - - This `error` property will be `undefined` if the degraded event represents a slow request +- Update `onDegraded` in `createServicePolicy` so that its event payload now includes an `error` property which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) + - This `error` property will be `undefined` if the degraded event merely represents a slow request ## [11.11.0] diff --git a/packages/network-controller/CHANGELOG.md b/packages/network-controller/CHANGELOG.md index 45e5a095b0c..af5aa545780 100644 --- a/packages/network-controller/CHANGELOG.md +++ b/packages/network-controller/CHANGELOG.md @@ -9,8 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **BREAKING:** The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) - - This `error` property will be `undefined` if the degraded event represents a slow request +- The object in the `NetworkController:rpcEndpointDegraded` event payload now includes an `error` property, which can be used to access the error produced by the last request when the maximum number of retries is exceeded ([#6188](https://github.com/MetaMask/core/pull/6188)) + - This `error` property will be `undefined` if the degraded event merely represents a slow request ## [24.0.1] From 19968e3c9fce8ce8e3cf2e4cb5f8d91cebabe119 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Mon, 11 Aug 2025 13:27:51 -0600 Subject: [PATCH 9/9] Remove comment --- .../src/rpc-service/shared.ts | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/packages/network-controller/src/rpc-service/shared.ts b/packages/network-controller/src/rpc-service/shared.ts index 844788a85fc..e33ae6129ad 100644 --- a/packages/network-controller/src/rpc-service/shared.ts +++ b/packages/network-controller/src/rpc-service/shared.ts @@ -7,41 +7,6 @@ export type FetchOptions = RequestInit; * Extends an event listener that Cockatiel uses so that when it is called, more * data can be supplied in the event object. */ -// If we have: -// -// AddToCockatielEventData< -// (data: void | { error: Error } | { value: unknown }) => void, -// { foo: 'bar' } -// > -// -// we want this to resolve as: -// -// (data: { foo: 'bar' } | ({ error: Error } & { foo: 'bar' }) | ({ value: unknown } & { foo: 'bar' })) => void -// -// To do this we need to get TypeScript to distribute -// `void | { error: Error } | { value: unknown }) => void` over the condition -// (`Data extends void ? ... : ...`). The key to doing this right is placing the -// condition right at the point where we need to know what `Data` is and not -// before (that is, before `(data ...) => void`). If we did this, the above -// would resolve as: -// -// (data: ((void | { error: Error } | { value: unknown }) & { foo: 'bar' }) => void -// -// which distributes to: -// -// (data: (void & { foo: 'bar' }) | ({ error: Error } & { foo: 'bar' }) | ({ value: unknown } & { foo: 'bar' })) => void -// -// (which is not correct because `void & { foo: 'bar' }` doesn't make sense). -// -// There is some information on distributive conditional types here: -// -// It is pretty technical, but the key insight is: "Distributive conditional -// types are automatically distributed over union types during instantiation." -// In this case, we need to place the condition so it becomes a part of the type -// that we want to "return" from this utility type and is not used to simply -// determine the return type. This way TypeScript evaluates the condition -// when that return type is used and not beforehand. -// export type AddToCockatielEventData = EventListener extends (data: infer Data) => void ? (data: Data extends void ? AdditionalData : Data & AdditionalData) => void