From a811bf652adb3066d68d588232e0ce983dbf0fa2 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 21 Jan 2025 10:52:46 -0700 Subject: [PATCH 1/3] createServicePolicy: Convert callbacks to methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both RetryPolicy and CircuitBreakerPolicy from the Cockatiel library allow for listening for events using `on*` methods. Our "service policy" allows for listening to some of the same events as well as the "degraded" event. For parity with Cockatiel — and for easier use in general — this commit removes the callbacks that `createServicePolicy` takes and replaces them with `on*` methods. --- packages/controller-utils/CHANGELOG.md | 6 +- .../src/create-service-policy.test.ts | 529 +++++++++--------- .../src/create-service-policy.ts | 144 +++-- packages/controller-utils/src/index.ts | 5 +- 4 files changed, 373 insertions(+), 311 deletions(-) diff --git a/packages/controller-utils/CHANGELOG.md b/packages/controller-utils/CHANGELOG.md index 8ef13e469f5..ef8364c1754 100644 --- a/packages/controller-utils/CHANGELOG.md +++ b/packages/controller-utils/CHANGELOG.md @@ -9,8 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- Add `createServicePolicy` function to assist with reducing boilerplate for service classes ([#5154](https://github.com/MetaMask/core/pull/5154), [#5143](https://github.com/MetaMask/core/pull/5143), [#5149](https://github.com/MetaMask/core/pull/5149)) - - Also add `DEFAULT_CIRCUIT_BREAK_DURATION`, `DEFAULT_DEGRADED_THRESHOLD`, `DEFAULT_MAX_CONSECUTIVE_FAILURES`, and `DEFAULT_MAX_RETRIES` constants and `IServicePolicy` type +- Add utility function for reducing boilerplate for service classes ([#5154](https://github.com/MetaMask/core/pull/5154), [#5143](https://github.com/MetaMask/core/pull/5143), [#5149](https://github.com/MetaMask/core/pull/5149), [#5188](https://github.com/MetaMask/core/pull/5188)) + - Add function `createServicePolicy` + - Add constants `DEFAULT_CIRCUIT_BREAK_DURATION`, `DEFAULT_DEGRADED_THRESHOLD`, `DEFAULT_MAX_CONSECUTIVE_FAILURES`, and `DEFAULT_MAX_RETRIES` + - Add types `ServicePolicy` and `CreateServicePolicyOptions` ## [11.4.5] diff --git a/packages/controller-utils/src/create-service-policy.test.ts b/packages/controller-utils/src/create-service-policy.test.ts index 0474647cea4..0b5894dad8c 100644 --- a/packages/controller-utils/src/create-service-policy.test.ts +++ b/packages/controller-utils/src/create-service-policy.test.ts @@ -43,58 +43,62 @@ describe('createServicePolicy', () => { expect(mockService).toHaveBeenCalledTimes(1); }); - it('does not call the onBreak callback, since the circuit never opens', async () => { + it('does not call the listener passed to onBreak, since the circuit never opens', async () => { const mockService = jest.fn(() => ({ some: 'data' })); - const onBreak = jest.fn(); - const policy = createServicePolicy({ onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy(); + policy.onBreak(onBreakListener); await policy.execute(mockService); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { - it('does not call the onDegraded callback if the service execution time is below the threshold', async () => { + it('does not call the listener passed to onDegraded if the service execution time is below the threshold', async () => { const mockService = jest.fn(() => ({ some: 'data' })); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy(); + policy.onDegraded(onDegradedListener); await policy.execute(mockService); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); - it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { + it('calls the listener passed to onDegraded once if the service execution time is beyond the threshold', async () => { const delay = DEFAULT_DEGRADED_THRESHOLD + 1; const mockService = jest.fn(() => { return new Promise((resolve) => { setTimeout(() => resolve({ some: 'data' }), delay); }); }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy(); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); clock.tick(delay); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); describe('using a custom degraded threshold', () => { - it('does not call the onDegraded callback if the service execution time below the threshold', async () => { + it('does not call the listener passed to onDegraded if the service execution time below the threshold', async () => { const degradedThreshold = 2000; const mockService = jest.fn(() => ({ some: 'data' })); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ degradedThreshold, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ degradedThreshold }); + policy.onDegraded(onDegradedListener); await policy.execute(mockService); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); - it('calls the onDegraded callback once if the service execution time beyond the threshold', async () => { + it('calls the listener passed to onDegraded once if the service execution time beyond the threshold', async () => { const degradedThreshold = 2000; const delay = degradedThreshold + 1; const mockService = jest.fn(() => { @@ -102,14 +106,15 @@ describe('createServicePolicy', () => { setTimeout(() => resolve({ some: 'data' }), delay); }); }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ degradedThreshold, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ degradedThreshold }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); clock.tick(delay); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -149,37 +154,37 @@ describe('createServicePolicy', () => { expect(mockService).toHaveBeenCalledTimes(1); }); - it('does not call the onRetry callback', async () => { + it('does not call the listener passed to onRetry', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onRetry = jest.fn(); + const onRetryListener = jest.fn(); const policy = createServicePolicy({ retryFilterPolicy: handleWhen( (caughtError) => caughtError.message !== 'failure', ), - onRetry, }); + policy.onRetry(onRetryListener); const promise = policy.execute(mockService); await ignoreRejection(promise); - expect(onRetry).not.toHaveBeenCalled(); + expect(onRetryListener).not.toHaveBeenCalled(); }); - it('does not call the onBreak callback', async () => { + it('does not call the listener passed to onBreak', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ retryFilterPolicy: handleWhen( (caughtError) => caughtError.message !== 'failure', ), - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue @@ -188,21 +193,21 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); - it('does not call the onDegraded callback', async () => { + it('does not call the listener passed to onDegraded', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ retryFilterPolicy: handleWhen( (caughtError) => caughtError.message !== 'failure', ), - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue @@ -211,7 +216,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); }); @@ -243,13 +248,14 @@ describe('createServicePolicy', () => { expect(mockService).toHaveBeenCalledTimes(1 + DEFAULT_MAX_RETRIES); }); - it('calls the onRetry callback once per retry', async () => { + it('calls the listener passed to onRetry once per retry', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onRetry = jest.fn(); - const policy = createServicePolicy({ onRetry }); + const onRetryListener = jest.fn(); + const policy = createServicePolicy(); + policy.onRetry(onRetryListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue is @@ -258,7 +264,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onRetry).toHaveBeenCalledTimes(DEFAULT_MAX_RETRIES); + expect(onRetryListener).toHaveBeenCalledTimes(DEFAULT_MAX_RETRIES); }); describe(`using the default max number of consecutive failures (${DEFAULT_MAX_CONSECUTIVE_FAILURES})`, () => { @@ -278,13 +284,14 @@ describe('createServicePolicy', () => { await expect(promise).rejects.toThrow(error); }); - it('does not call the onBreak callback, since the max number of consecutive failures is never reached', async () => { + it('does not call the listener passed to onBreak, since the max number of consecutive failures is never reached', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); - const policy = createServicePolicy({ onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy(); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -293,16 +300,17 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); - it('calls the onDegraded callback once, since the circuit is still closed', async () => { + it('calls the listener passed to onDegraded once, since the circuit is still closed', async () => { const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy(); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -311,7 +319,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -323,10 +331,8 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); const promise = policy.execute(mockService); @@ -338,17 +344,17 @@ describe('createServicePolicy', () => { await expect(promise).rejects.toThrow(error); }); - it('does not call the onBreak callback', async () => { + it('does not call the listener passed to onBreak', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 2; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -357,20 +363,20 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); - it('calls the onDegraded callback once', async () => { + it('calls the listener passed to onDegraded once', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 2; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -379,7 +385,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -403,17 +409,17 @@ describe('createServicePolicy', () => { await expect(promise).rejects.toThrow(error); }); - it('calls the onBreak callback once with the error', async () => { + it('calls the listener passed to onBreak once with the error', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -422,21 +428,21 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); - it('never calls the onDegraded callback, since the circuit is open', async () => { + it('never calls the listener passed to onDegraded, since the circuit is open', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES + 1; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -445,7 +451,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('throws a BrokenCircuitError instead of whatever error the service produces if the service is executed again', async () => { @@ -498,17 +504,17 @@ describe('createServicePolicy', () => { ); }); - it('calls the onBreak callback once with the error', async () => { + it('calls the listener passed to onBreak once with the error', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -517,21 +523,21 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); - it('never calls the onDegraded callback, since the circuit is open', async () => { + it('never calls the listener passed to onDegraded, since the circuit is open', async () => { const maxConsecutiveFailures = DEFAULT_MAX_RETRIES; const error = new Error('failure'); const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -540,7 +546,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); }); }); @@ -582,11 +588,11 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onRetry = jest.fn(); + const onRetryListener = jest.fn(); const policy = createServicePolicy({ maxRetries, - onRetry, }); + policy.onRetry(onRetryListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue is @@ -595,7 +601,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onRetry).toHaveBeenCalledTimes(maxRetries); + expect(onRetryListener).toHaveBeenCalledTimes(maxRetries); }); describe(`using the default max number of consecutive failures (${DEFAULT_MAX_CONSECUTIVE_FAILURES})`, () => { @@ -623,8 +629,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -633,7 +640,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once', async () => { @@ -642,8 +649,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -652,7 +660,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -680,8 +688,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -690,8 +699,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('never calls the onDegraded callback, since the circuit is open', async () => { @@ -700,8 +709,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -710,7 +720,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('throws a BrokenCircuitError instead of whatever error the service produces if the policy is executed again', async () => { @@ -719,8 +729,7 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const policy = createServicePolicy({ maxRetries }); const firstExecution = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -765,8 +774,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -775,8 +785,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('never calls the onDegraded callback, since the circuit is open', async () => { @@ -785,8 +795,9 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -795,7 +806,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); }); }); @@ -830,12 +841,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -844,7 +855,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once', async () => { @@ -854,12 +865,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -868,7 +879,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -901,12 +912,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -915,8 +926,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('never calls the onDegraded callback, since the circuit is open', async () => { @@ -926,12 +937,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -940,7 +951,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('throws a BrokenCircuitError instead of whatever error the service produces if the policy is executed again', async () => { @@ -1004,12 +1015,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1018,8 +1029,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('never calls the onDegraded callback, since the circuit is open', async () => { @@ -1029,12 +1040,12 @@ describe('createServicePolicy', () => { const mockService = jest.fn(() => { throw error; }); - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1043,7 +1054,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); }); }); @@ -1096,8 +1107,9 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onBreak = jest.fn(); - const policy = createServicePolicy({ onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy(); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue @@ -1117,8 +1129,9 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onBreak = jest.fn(); - const policy = createServicePolicy({ onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy(); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise queue @@ -1127,7 +1140,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -1140,8 +1153,9 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy(); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1150,7 +1164,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1166,8 +1180,9 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy(); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1176,7 +1191,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -1191,11 +1206,11 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1204,7 +1219,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1221,11 +1236,11 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1234,7 +1249,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -1251,11 +1266,11 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1276,11 +1291,11 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1289,7 +1304,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -1303,11 +1318,11 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1316,7 +1331,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1333,11 +1348,11 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1346,7 +1361,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -1362,12 +1377,12 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1376,7 +1391,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1394,12 +1409,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1408,7 +1423,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -1424,11 +1439,11 @@ describe('createServicePolicy', () => { } throw new Error('failure'); }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1450,11 +1465,11 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1463,7 +1478,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -1478,11 +1493,11 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1491,7 +1506,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1508,11 +1523,11 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1521,7 +1536,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -1538,12 +1553,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1552,7 +1567,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1570,12 +1585,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1584,7 +1599,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -1601,11 +1616,11 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1630,11 +1645,11 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1643,8 +1658,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('does not call the onDegraded callback', async () => { @@ -1658,11 +1673,11 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1671,7 +1686,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); describe(`using the default circuit break duration (${DEFAULT_CIRCUIT_BREAK_DURATION})`, () => { @@ -1808,8 +1823,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1818,7 +1834,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -1833,8 +1849,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1843,7 +1860,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1860,8 +1877,9 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1870,7 +1888,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -1887,12 +1905,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1901,7 +1919,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -1919,12 +1937,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1933,7 +1951,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -1972,8 +1990,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -1982,7 +2001,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -1997,8 +2016,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2007,7 +2027,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2024,8 +2044,9 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2034,7 +2055,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -2051,12 +2072,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2065,7 +2086,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2083,12 +2104,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2097,7 +2118,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -2140,8 +2161,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); - const policy = createServicePolicy({ maxRetries, onBreak }); + const onBreakListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2150,8 +2172,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('does not call the onDegraded callback', async () => { @@ -2165,8 +2187,9 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); - const policy = createServicePolicy({ maxRetries, onDegraded }); + const onDegradedListener = jest.fn(); + const policy = createServicePolicy({ maxRetries }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2175,7 +2198,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); describe(`using the default circuit break duration (${DEFAULT_CIRCUIT_BREAK_DURATION})`, () => { @@ -2257,12 +2280,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2285,12 +2308,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2299,7 +2322,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -2315,12 +2338,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2329,7 +2352,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2347,12 +2370,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2361,7 +2384,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -2379,13 +2402,13 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2394,7 +2417,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2413,13 +2436,13 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2428,7 +2451,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -2472,12 +2495,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2486,7 +2509,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onBreak).not.toHaveBeenCalled(); + expect(onBreakListener).not.toHaveBeenCalled(); }); describe(`using the default degraded threshold (${DEFAULT_DEGRADED_THRESHOLD})`, () => { @@ -2502,12 +2525,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2516,7 +2539,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2534,12 +2557,12 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2548,7 +2571,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); @@ -2566,13 +2589,13 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2581,7 +2604,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); it('calls the onDegraded callback once if the service execution time is beyond the threshold', async () => { @@ -2600,13 +2623,13 @@ describe('createServicePolicy', () => { } }); }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, degradedThreshold, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2615,7 +2638,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await promise; - expect(onDegraded).toHaveBeenCalledTimes(1); + expect(onDegradedListener).toHaveBeenCalledTimes(1); }); }); }); @@ -2664,12 +2687,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onBreak = jest.fn(); + const onBreakListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onBreak, }); + policy.onBreak(onBreakListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2678,8 +2701,8 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onBreak).toHaveBeenCalledTimes(1); - expect(onBreak).toHaveBeenCalledWith({ error }); + expect(onBreakListener).toHaveBeenCalledTimes(1); + expect(onBreakListener).toHaveBeenCalledWith({ error }); }); it('does not call the onDegraded callback', async () => { @@ -2694,12 +2717,12 @@ describe('createServicePolicy', () => { } throw error; }; - const onDegraded = jest.fn(); + const onDegradedListener = jest.fn(); const policy = createServicePolicy({ maxRetries, maxConsecutiveFailures, - onDegraded, }); + policy.onDegraded(onDegradedListener); const promise = policy.execute(mockService); // It's safe not to await this promise; adding it to the promise @@ -2708,7 +2731,7 @@ describe('createServicePolicy', () => { clock.runAllAsync(); await ignoreRejection(promise); - expect(onDegraded).not.toHaveBeenCalled(); + expect(onDegradedListener).not.toHaveBeenCalled(); }); describe(`using the default circuit break duration (${DEFAULT_CIRCUIT_BREAK_DURATION})`, () => { diff --git a/packages/controller-utils/src/create-service-policy.ts b/packages/controller-utils/src/create-service-policy.ts index f60ed0f9cb8..971fae8f8cb 100644 --- a/packages/controller-utils/src/create-service-policy.ts +++ b/packages/controller-utils/src/create-service-policy.ts @@ -7,9 +7,70 @@ import { wrap, CircuitState, } from 'cockatiel'; -import type { IPolicy, Policy } from 'cockatiel'; +import type { + CircuitBreakerPolicy, + IPolicy, + Policy, + RetryPolicy, +} from 'cockatiel'; -export type { IPolicy as IServicePolicy }; +/** + * The options for `createServicePolicy`. + */ +export type CreateServicePolicyOptions = { + /** + * The length of time (in milliseconds) to pause retries of the action after + * the number of failures reaches `maxConsecutiveFailures`. + */ + circuitBreakDuration?: number; + /** + * The length of time (in milliseconds) that governs when the service is + * regarded as degraded (affecting when `onDegraded` is called). + */ + degradedThreshold?: number; + /** + * The maximum number of times that the service is allowed to fail before + * pausing further retries. + */ + maxConsecutiveFailures?: number; + /** + * The maximum number of times that a failing service should be re-invoked + * before giving up. + */ + maxRetries?: number; + /** + * The policy used to control when the service should be retried based on + * either the result of the service or an error that it throws. For instance, + * you could use this to retry only certain errors. See `handleWhen` and + * friends from Cockatiel for more. + */ + retryFilterPolicy?: Policy; +}; + +/** + * The service policy object. + */ +export type ServicePolicy = IPolicy & { + /** + * A function which is called when the number of times that the service fails + * in a row meets the set maximum number of consecutive failures. + */ + onBreak: CircuitBreakerPolicy['onBreak']; + /** + * A function which is called in two circumstances: 1) when the service + * succeeds before the maximum number of consecutive failures is reached, but + * takes more time than the `degradedThreshold` to run, or 2) if the service + * never succeeds before the retry policy gives up and before the maximum + * number of consecutive failures has been reached. + */ + onDegraded: (fn: () => void) => 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 + * primarily useful in tests where we are mocking timers. + */ + onRetry: RetryPolicy['onRetry']; +}; /** * The maximum number of times that a failing service should be re-run before @@ -19,7 +80,9 @@ export const DEFAULT_MAX_RETRIES = 3; /** * The maximum number of times that the service is allowed to fail before - * pausing further retries. + * pausing further retries. This is set to a value such that if given a + * service that continually fails, the policy needs to be executed 3 times + * before further retries are paused. */ export const DEFAULT_MAX_CONSECUTIVE_FAILURES = (1 + DEFAULT_MAX_RETRIES) * 3; @@ -65,14 +128,6 @@ export const DEFAULT_DEGRADED_THRESHOLD = 5_000; * @param options.degradedThreshold - The length of time (in milliseconds) that * governs when the service is regarded as degraded (affecting when `onDegraded` * is called). Defaults to 5 seconds. - * @param options.onBreak - A function which is called when the service fails - * too many times in a row (specifically, more than `maxConsecutiveFailures`). - * @param options.onDegraded - A function which is called when the service - * succeeds before `maxConsecutiveFailures` is reached, but takes more time than - * the `degradedThreshold` to run. - * @param options.onRetry - A function which will be called the moment the - * policy kicks off a timer to re-run the function passed to the policy. This is - * primarily useful in tests where we are mocking timers. * @returns The service policy. * @example * This function is designed to be used in the context of a service class like @@ -112,25 +167,7 @@ export function createServicePolicy({ maxConsecutiveFailures = DEFAULT_MAX_CONSECUTIVE_FAILURES, circuitBreakDuration = DEFAULT_CIRCUIT_BREAK_DURATION, degradedThreshold = DEFAULT_DEGRADED_THRESHOLD, - onBreak = () => { - // do nothing - }, - onDegraded = () => { - // do nothing - }, - onRetry = () => { - // do nothing - }, -}: { - maxRetries?: number; - retryFilterPolicy?: Policy; - maxConsecutiveFailures?: number; - circuitBreakDuration?: number; - degradedThreshold?: number; - onBreak?: () => void; - onDegraded?: () => void; - onRetry?: () => void; -} = {}): IPolicy { +}: CreateServicePolicyOptions = {}): ServicePolicy { const retryPolicy = retry(retryFilterPolicy, { // Note that although the option here is called "max attempts", it's really // maximum number of *retries* (attempts past the initial attempt). @@ -139,6 +176,7 @@ export function createServicePolicy({ // determined by a backoff formula. backoff: new ExponentialBackoff(), }); + const onRetry = retryPolicy.onRetry.bind(retryPolicy); const circuitBreakerPolicy = circuitBreaker(handleAll, { // While the circuit is open, any additional invocations of the service @@ -150,27 +188,14 @@ export function createServicePolicy({ halfOpenAfter: circuitBreakDuration, breaker: new ConsecutiveBreaker(maxConsecutiveFailures), }); + const onBreak = circuitBreakerPolicy.onBreak.bind(circuitBreakerPolicy); - // The `onBreak` callback will be called if the service consistently throws - // for as many times as exceeds the maximum consecutive number of failures. - // Combined with the retry policy, this can happen if: - // - `maxConsecutiveFailures` < the default max retries (3) and the policy is - // executed once - // - `maxConsecutiveFailures` >= the default max retries (3) but the policy is - // executed multiple times, enough for the total number of retries to exceed - // `maxConsecutiveFailures` - circuitBreakerPolicy.onBreak(onBreak); - - // The `onRetryPolicy` callback will be called each time the service is - // invoked (including retries). - retryPolicy.onRetry(onRetry); - + const onDegradedListeners: (() => void)[] = []; retryPolicy.onGiveUp(() => { if (circuitBreakerPolicy.state === CircuitState.Closed) { - // The `onDegraded` callback will be called if the number of retries is - // exceeded and the maximum number of consecutive failures has not been - // reached yet (whether the policy is called once or multiple times). - onDegraded(); + for (const listener of onDegradedListeners) { + listener(); + } } }); retryPolicy.onSuccess(({ duration }) => { @@ -178,14 +203,23 @@ export function createServicePolicy({ circuitBreakerPolicy.state === CircuitState.Closed && duration > degradedThreshold ) { - // The `onDegraded` callback will also be called if the service does not - // throw, but the time it takes for the service to run exceeds the - // `degradedThreshold`. - onDegraded(); + for (const listener of onDegradedListeners) { + listener(); + } } }); + const onDegraded = (listener: () => void) => { + onDegradedListeners.push(listener); + }; + + // Every time the retry policy makes an attempt, it executes the circuit + // breaker policy, which executes the service. + const policy = wrap(retryPolicy, circuitBreakerPolicy); - // The retry policy really retries the circuit breaker policy, which invokes - // the service. - return wrap(retryPolicy, circuitBreakerPolicy); + return { + ...policy, + onBreak, + onDegraded, + onRetry, + }; } diff --git a/packages/controller-utils/src/index.ts b/packages/controller-utils/src/index.ts index 446b9ce3823..67742049deb 100644 --- a/packages/controller-utils/src/index.ts +++ b/packages/controller-utils/src/index.ts @@ -5,7 +5,10 @@ export { DEFAULT_MAX_RETRIES, createServicePolicy, } from './create-service-policy'; -export type { IServicePolicy } from './create-service-policy'; +export type { + CreateServicePolicyOptions, + ServicePolicy, +} from './create-service-policy'; export * from './constants'; export type { NonEmptyArray } from './util'; export { From 2e29f94b58dbfbc63227387a6b45642829c0dcbb Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 7 Jan 2025 13:38:08 -0700 Subject: [PATCH 2/3] Refactor services to align with eventual RpcService In a future commit, we are adding an RpcService class to `network-controller` to handle automatic failovers. This class is different from the existing service classes in a couple of ways: - RpcService will make use of a new `createServicePolicy` function which encapsulates retry and circuit breaker logic. This will relieve the consuming module for RpcService from needing to test this logic. - RpcService will have `onBreak` and `onDegraded` methods instead of taking `onBreak` and `onDegraded` callbacks as constructor options. This is reflected not only in the class but also in the abstract RPC service interface. To these ends, this commit makes the following changes to `CodefiTokenPricesServiceV2` in `assets-controllers` and `ClientConfigApiService` in `remote-feature-flag-controller`: - Refactor service class to use `createServicePolicy` - Update the abstract service class interface to extend `ServicePolicy` from `controller-utils`, and prepend `I` to the type to indicate that it's not a superclass, but an interface - Deprecate the `onBreak` and `onDegraded` constructor options, and add methods instead --- eslint-warning-thresholds.json | 2 +- packages/assets-controllers/CHANGELOG.md | 11 + packages/assets-controllers/package.json | 1 - .../src/TokenRatesController.test.ts | 22 +- .../src/TokenRatesController.ts | 8 +- .../assets-controllers/src/assetsUtil.test.ts | 4 +- packages/assets-controllers/src/assetsUtil.ts | 6 +- .../abstract-token-prices-service.ts | 5 +- .../token-prices-service/codefi-v2.test.ts | 1164 ++++------------- .../src/token-prices-service/codefi-v2.ts | 151 ++- .../src/token-prices-service/index.ts | 2 +- .../CHANGELOG.md | 11 + .../package.json | 3 +- .../abstract-client-config-api-service.ts | 19 +- .../client-config-api-service.test.ts | 157 +-- .../client-config-api-service.ts | 163 ++- .../remote-feature-flag-controller.test.ts | 8 +- .../src/remote-feature-flag-controller.ts | 10 +- yarn.lock | 2 - 19 files changed, 564 insertions(+), 1185 deletions(-) diff --git a/eslint-warning-thresholds.json b/eslint-warning-thresholds.json index a55d4f88d87..baae7815cfd 100644 --- a/eslint-warning-thresholds.json +++ b/eslint-warning-thresholds.json @@ -5,7 +5,7 @@ "@typescript-eslint/no-unsafe-enum-comparison": 34, "@typescript-eslint/no-unused-vars": 41, "@typescript-eslint/prefer-promise-reject-errors": 33, - "@typescript-eslint/prefer-readonly": 147, + "@typescript-eslint/prefer-readonly": 143, "import-x/namespace": 189, "import-x/no-named-as-default": 1, "import-x/no-named-as-default-member": 8, diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 4a62573da5b..ada9f939fa5 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `onBreak` and `onDegraded` methods to `CodefiTokenPricesServiceV2` ([#5109](https://github.com/MetaMask/core/pull/5109)) + - These serve the same purpose as the `onBreak` and `onDegraded` constructor options, but align more closely with the Cockatiel policy API. + +### Changed + +- Deprecate `ClientConfigApiService` constructor options `onBreak` and `onDegraded` in favor of methods ([#5109](https://github.com/MetaMask/core/pull/5109)) +- Add `@metamask/controller-utils@^11.4.5` as a dependency ([#5109](https://github.com/MetaMask/core/pull/5109)) + - `cockatiel` should still be in the dependency tree because it's now a dependency of `@metamask/controller-utils` + ## [46.0.1] ### Changed diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index acde5f8d16a..0a70808053a 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -68,7 +68,6 @@ "async-mutex": "^0.5.0", "bitcoin-address-validation": "^2.2.3", "bn.js": "^5.2.1", - "cockatiel": "^3.1.2", "immer": "^9.0.6", "lodash": "^4.17.21", "multiformats": "^13.1.0", diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index db41deddd41..05378c15a80 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -30,7 +30,7 @@ import { } from '../../network-controller/tests/helpers'; import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import type { - AbstractTokenPricesService, + IAbstractTokenPricesService, TokenPrice, TokenPricesByTokenAddress, } from './token-prices-service/abstract-token-prices-service'; @@ -2093,11 +2093,9 @@ describe('TokenRatesController', () => { price: 0.002, }, }), - validateCurrencySupported: jest.fn().mockReturnValue( - false, - // Cast used because this method has an assertion in the return - // value that I don't know how to type properly with Jest's mock. - ) as unknown as AbstractTokenPricesService['validateCurrencySupported'], + validateCurrencySupported(_currency: unknown): _currency is string { + return false; + }, }); nock('https://min-api.cryptocompare.com') .get('/data/price') @@ -2288,11 +2286,9 @@ describe('TokenRatesController', () => { value: 0.002, }, }), - validateChainIdSupported: jest.fn().mockReturnValue( - false, - // Cast used because this method has an assertion in the return - // value that I don't know how to type properly with Jest's mock. - ) as unknown as AbstractTokenPricesService['validateChainIdSupported'], + validateChainIdSupported(_chainId: unknown): _chainId is Hex { + return false; + }, }); await withController( { @@ -2758,8 +2754,8 @@ async function callUpdateExchangeRatesMethod({ * @returns The built mock token prices service. */ function buildMockTokenPricesService( - overrides: Partial = {}, -): AbstractTokenPricesService { + overrides: Partial = {}, +): IAbstractTokenPricesService { return { async fetchTokenPrices() { return {}; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index f25702bbf82..5bca2809ea0 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -26,7 +26,7 @@ import { isEqual } from 'lodash'; import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service'; -import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; +import type { IAbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; import type { TokensControllerGetStateAction, @@ -238,7 +238,7 @@ export class TokenRatesController extends StaticIntervalPollingController> = {}; @@ -275,7 +275,7 @@ export class TokenRatesController extends StaticIntervalPollingController; }) { @@ -687,7 +687,7 @@ export class TokenRatesController extends StaticIntervalPollingController> + Awaited> >({ values: [...tokenAddresses].sort(), batchSize: TOKEN_PRICES_BATCH_SIZE, diff --git a/packages/assets-controllers/src/assetsUtil.test.ts b/packages/assets-controllers/src/assetsUtil.test.ts index 17dc36ebf93..a40eccfa542 100644 --- a/packages/assets-controllers/src/assetsUtil.test.ts +++ b/packages/assets-controllers/src/assetsUtil.test.ts @@ -10,7 +10,7 @@ import { add0x, type Hex } from '@metamask/utils'; import * as assetsUtil from './assetsUtil'; import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import type { Nft, NftMetadata } from './NftController'; -import type { AbstractTokenPricesService } from './token-prices-service'; +import type { IAbstractTokenPricesService } from './token-prices-service'; const DEFAULT_IPFS_URL_FORMAT = 'ipfs://'; const ALTERNATIVE_IPFS_URL_FORMAT = 'ipfs://ipfs/'; @@ -772,7 +772,7 @@ function buildAddress(number: number) { * * @returns The mocked functions of token prices service. */ -function createMockPriceService(): AbstractTokenPricesService { +function createMockPriceService(): IAbstractTokenPricesService { return { validateChainIdSupported(_chainId: unknown): _chainId is Hex { return true; diff --git a/packages/assets-controllers/src/assetsUtil.ts b/packages/assets-controllers/src/assetsUtil.ts index 48b0bcde927..52c0965246d 100644 --- a/packages/assets-controllers/src/assetsUtil.ts +++ b/packages/assets-controllers/src/assetsUtil.ts @@ -8,7 +8,7 @@ import { remove0x } from '@metamask/utils'; import BN from 'bn.js'; import type { Nft, NftMetadata } from './NftController'; -import type { AbstractTokenPricesService } from './token-prices-service'; +import type { IAbstractTokenPricesService } from './token-prices-service'; import { type ContractExchangeRates } from './TokenRatesController'; /** @@ -387,7 +387,7 @@ export async function fetchTokenContractExchangeRates({ tokenAddresses, chainId, }: { - tokenPricesService: AbstractTokenPricesService; + tokenPricesService: IAbstractTokenPricesService; nativeCurrency: string; tokenAddresses: Hex[]; chainId: Hex; @@ -403,7 +403,7 @@ export async function fetchTokenContractExchangeRates({ const tokenPricesByTokenAddress = await reduceInBatchesSerially< Hex, - Awaited> + Awaited> >({ values: [...tokenAddresses].sort(), batchSize: TOKEN_PRICES_BATCH_SIZE, diff --git a/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts b/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts index a3cd09a0586..bac95f74fec 100644 --- a/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts +++ b/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts @@ -1,3 +1,4 @@ +import type { ServicePolicy } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; /** @@ -49,11 +50,11 @@ export type TokenPricesByTokenAddress< * @template Currency - A type union of valid arguments for the `currency` * argument to `fetchTokenPrices`. */ -export type AbstractTokenPricesService< +export type IAbstractTokenPricesService< ChainId extends Hex = Hex, TokenAddress extends Hex = Hex, Currency extends string = string, -> = { +> = Partial> & { /** * Retrieves prices in the given currency for the tokens identified by the * given addresses which are expected to live on the given chain. diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts index e1efe858ec2..8f644e7e02e 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts @@ -14,6 +14,232 @@ import { const defaultMaxRetryDelay = 30_000; describe('CodefiTokenPricesServiceV2', () => { + describe('onBreak', () => { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = useFakeTimers({ now: Date.now() }); + }); + + afterEach(() => { + clock.restore(); + }); + + it('registers a listener that is called upon break', async () => { + const retries = 3; + // Max consencutive failures is set to match number of calls in three update attempts (including retries) + const maximumConsecutiveFailures = (1 + retries) * 3; + // Initial interceptor for failing requests + nock('https://price.api.cx.metamask.io') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: + '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + includeMarketData: 'true', + }) + .times(maximumConsecutiveFailures) + .replyWithError('Failed to fetch'); + // This interceptor should not be used + nock('https://price.api.cx.metamask.io') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: + '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + includeMarketData: 'true', + }) + .reply(200, { + '0x0000000000000000000000000000000000000000': { + price: 14, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + marketCap: 117219.99428314982, + allTimeHigh: 0.00060467892389492, + allTimeLow: 0.00002303954000865728, + totalVolume: 5155.094053542448, + high1d: 0.00008020715848194385, + low1d: 0.00007792083564549064, + circulatingSupply: 1494269733.9526057, + dilutedMarketCap: 117669.5125951733, + marketCapPercentChange1d: 0.76671, + pricePercentChange1h: -1.0736342953259423, + pricePercentChange7d: -7.351582573655089, + pricePercentChange14d: -1.0799098946709822, + pricePercentChange30d: -25.776321124365992, + pricePercentChange200d: 46.091571238599165, + pricePercentChange1y: -2.2992517267242754, + }, + '0xaaa': { + price: 148.17205755299946, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + marketCap: 117219.99428314982, + allTimeHigh: 0.00060467892389492, + allTimeLow: 0.00002303954000865728, + totalVolume: 5155.094053542448, + high1d: 0.00008020715848194385, + low1d: 0.00007792083564549064, + circulatingSupply: 1494269733.9526057, + dilutedMarketCap: 117669.5125951733, + marketCapPercentChange1d: 0.76671, + pricePercentChange1h: -1.0736342953259423, + pricePercentChange7d: -7.351582573655089, + pricePercentChange14d: -1.0799098946709822, + pricePercentChange30d: -25.776321124365992, + pricePercentChange200d: 46.091571238599165, + pricePercentChange1y: -2.2992517267242754, + }, + '0xbbb': { + price: 33689.98134554716, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + marketCap: 117219.99428314982, + allTimeHigh: 0.00060467892389492, + allTimeLow: 0.00002303954000865728, + totalVolume: 5155.094053542448, + high1d: 0.00008020715848194385, + low1d: 0.00007792083564549064, + circulatingSupply: 1494269733.9526057, + dilutedMarketCap: 117669.5125951733, + marketCapPercentChange1d: 0.76671, + pricePercentChange1h: -1.0736342953259423, + pricePercentChange7d: -7.351582573655089, + pricePercentChange14d: -1.0799098946709822, + pricePercentChange30d: -25.776321124365992, + pricePercentChange200d: 46.091571238599165, + pricePercentChange1y: -2.2992517267242754, + }, + '0xccc': { + price: 148.1344197578456, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + marketCap: 117219.99428314982, + allTimeHigh: 0.00060467892389492, + allTimeLow: 0.00002303954000865728, + totalVolume: 5155.094053542448, + high1d: 0.00008020715848194385, + low1d: 0.00007792083564549064, + circulatingSupply: 1494269733.9526057, + dilutedMarketCap: 117669.5125951733, + marketCapPercentChange1d: 0.76671, + pricePercentChange1h: -1.0736342953259423, + pricePercentChange7d: -7.351582573655089, + pricePercentChange14d: -1.0799098946709822, + pricePercentChange30d: -25.776321124365992, + pricePercentChange200d: 46.091571238599165, + pricePercentChange1y: -2.2992517267242754, + }, + }); + const onBreakHandler = jest.fn(); + const service = new CodefiTokenPricesServiceV2({ + retries, + maximumConsecutiveFailures, + // Ensure break duration is well over the max delay for a single request, so that the + // break doesn't end during a retry attempt + circuitBreakDuration: defaultMaxRetryDelay * 10, + }); + service.onBreak(onBreakHandler); + const fetchTokenPrices = () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }); + expect(onBreakHandler).not.toHaveBeenCalled(); + + // Initial three calls to exhaust maximum allowed failures + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for (const _retryAttempt of Array(retries).keys()) { + // eslint-disable-next-line no-loop-func + await expect(() => + fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices, + retries, + }), + ).rejects.toThrow('Failed to fetch'); + } + + expect(onBreakHandler).toHaveBeenCalledTimes(1); + }); + }); + + describe('onDegraded', () => { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = useFakeTimers({ now: Date.now() }); + }); + + afterEach(() => { + clock.restore(); + }); + + it('calls onDegraded when request is slower than threshold', async () => { + const degradedThreshold = 1000; + const retries = 0; + nock('https://price.api.cx.metamask.io') + .get('/v2/chains/1/spot-prices') + .query({ + tokenAddresses: + '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', + vsCurrency: 'ETH', + includeMarketData: 'true', + }) + .delay(degradedThreshold * 2) + .reply(200, { + '0x0000000000000000000000000000000000000000': { + price: 14, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + }, + '0xaaa': { + price: 148.17205755299946, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + }, + '0xbbb': { + price: 33689.98134554716, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + }, + '0xccc': { + price: 148.1344197578456, + currency: 'ETH', + pricePercentChange1d: 1, + priceChange1d: 1, + }, + }); + const onDegradedHandler = jest.fn(); + const service = new CodefiTokenPricesServiceV2({ + degradedThreshold, + retries, + }); + service.onDegraded(onDegradedHandler); + + await fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices: () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', + }), + retries, + }); + + expect(onDegradedHandler).toHaveBeenCalledTimes(1); + }); + }); + describe('fetchTokenPrices', () => { it('uses the /spot-prices endpoint of the Codefi Price API to gather prices for the given tokens', async () => { nock('https://price.api.cx.metamask.io') @@ -829,8 +1055,9 @@ describe('CodefiTokenPricesServiceV2', () => { clock.restore(); }); - it('does not call onDegraded when requests succeeds faster than threshold', async () => { + it('calls onDegraded when request is slower than threshold', async () => { const degradedThreshold = 1000; + const retries = 0; nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ @@ -839,326 +1066,38 @@ describe('CodefiTokenPricesServiceV2', () => { vsCurrency: 'ETH', includeMarketData: 'true', }) - .delay(degradedThreshold / 2) + .delay(degradedThreshold * 2) .reply(200, { '0x0000000000000000000000000000000000000000': { price: 14, currency: 'ETH', pricePercentChange1d: 1, priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, }, '0xaaa': { price: 148.17205755299946, currency: 'ETH', pricePercentChange1d: 1, priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, }, '0xbbb': { price: 33689.98134554716, currency: 'ETH', pricePercentChange1d: 1, priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, }, '0xccc': { price: 148.1344197578456, currency: 'ETH', pricePercentChange1d: 1, priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, }, }); const onDegradedHandler = jest.fn(); const service = new CodefiTokenPricesServiceV2({ degradedThreshold, onDegraded: onDegradedHandler, - }); - - await service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }); - - expect(onDegradedHandler).not.toHaveBeenCalled(); - }); - - it('does not call onDegraded when requests succeeds on retry faster than threshold', async () => { - // Set threshold above max retry delay to ensure the time is always under the threshold, - // even with random jitter - const degradedThreshold = defaultMaxRetryDelay + 1000; - const retries = 1; - // Initial interceptor for failing request - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .replyWithError('Failed to fetch'); - // Second interceptor for successful response - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .delay(500) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - }); - const onDegradedHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - degradedThreshold, - onDegraded: onDegradedHandler, - retries, - }); - - await fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices: () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }), - retries, - }); - - expect(onDegradedHandler).not.toHaveBeenCalled(); - }); - - it('calls onDegraded when request fails', async () => { - const retries = 0; - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .replyWithError('Failed to fetch'); - const onDegradedHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - onDegraded: onDegradedHandler, - retries, - }); - - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices: () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }), - retries, - }), - ).rejects.toThrow('Failed to fetch'); - - expect(onDegradedHandler).toHaveBeenCalledTimes(1); - }); - - it('calls onDegraded when request is slower than threshold', async () => { - const degradedThreshold = 1000; - const retries = 0; - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .delay(degradedThreshold * 2) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - }); - const onDegradedHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - degradedThreshold, - onDegraded: onDegradedHandler, - retries, - }); - - await fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices: () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }), - retries, - }); - - expect(onDegradedHandler).toHaveBeenCalledTimes(1); - }); - - it('calls onDegraded when request is slower than threshold after retry', async () => { - const degradedThreshold = 1000; - const retries = 1; - // Initial interceptor for failing request - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .replyWithError('Failed to fetch'); - // Second interceptor for successful response - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .delay(degradedThreshold * 2) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - }, - }); - const onDegradedHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - degradedThreshold, - onDegraded: onDegradedHandler, - retries, + retries, }); await fetchTokenPricesWithFakeTimers({ @@ -1187,7 +1126,7 @@ describe('CodefiTokenPricesServiceV2', () => { clock.restore(); }); - it('stops making fetch requests after too many consecutive failures', async () => { + it('calls onBreak handler upon break', async () => { const retries = 3; // Max consencutive failures is set to match number of calls in three update attempts (including retries) const maximumConsecutiveFailures = (1 + retries) * 3; @@ -1203,7 +1142,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(maximumConsecutiveFailures) .replyWithError('Failed to fetch'); // This interceptor should not be used - const successfullCallScope = nock('https://price.api.cx.metamask.io') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: @@ -1297,9 +1236,11 @@ describe('CodefiTokenPricesServiceV2', () => { pricePercentChange1y: -2.2992517267242754, }, }); + const onBreakHandler = jest.fn(); const service = new CodefiTokenPricesServiceV2({ retries, maximumConsecutiveFailures, + onBreak: onBreakHandler, // Ensure break duration is well over the max delay for a single request, so that the // break doesn't end during a retry attempt circuitBreakDuration: defaultMaxRetryDelay * 10, @@ -1310,6 +1251,8 @@ describe('CodefiTokenPricesServiceV2', () => { tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], currency: 'ETH', }); + expect(onBreakHandler).not.toHaveBeenCalled(); + // Initial three calls to exhaust maximum allowed failures // eslint-disable-next-line @typescript-eslint/no-unused-vars for (const _retryAttempt of Array(retries).keys()) { @@ -1323,642 +1266,7 @@ describe('CodefiTokenPricesServiceV2', () => { ).rejects.toThrow('Failed to fetch'); } - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - expect(successfullCallScope.isDone()).toBe(false); - }); - - it('calls onBreak handler upon break', async () => { - const retries = 3; - // Max consencutive failures is set to match number of calls in three update attempts (including retries) - const maximumConsecutiveFailures = (1 + retries) * 3; - // Initial interceptor for failing requests - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .times(maximumConsecutiveFailures) - .replyWithError('Failed to fetch'); - // This interceptor should not be used - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - }); - const onBreakHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - retries, - maximumConsecutiveFailures, - // Ensure break duration is well over the max delay for a single request, so that the - // break doesn't end during a retry attempt - onBreak: onBreakHandler, - circuitBreakDuration: defaultMaxRetryDelay * 10, - }); - const fetchTokenPrices = () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }); - expect(onBreakHandler).not.toHaveBeenCalled(); - - // Initial three calls to exhaust maximum allowed failures - // eslint-disable-next-line @typescript-eslint/no-unused-vars - for (const _retryAttempt of Array(retries).keys()) { - // eslint-disable-next-line no-loop-func - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow('Failed to fetch'); - } - - expect(onBreakHandler).toHaveBeenCalledTimes(1); - }); - - it('stops calling onDegraded after circuit break', async () => { - const retries = 3; - // Max consencutive failures is set to match number of calls in three update attempts (including retries) - const maximumConsecutiveFailures = (1 + retries) * 3; - // Initial interceptor for failing requests - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .times(maximumConsecutiveFailures) - .replyWithError('Failed to fetch'); - const onBreakHandler = jest.fn(); - const onDegradedHandler = jest.fn(); - const service = new CodefiTokenPricesServiceV2({ - retries, - maximumConsecutiveFailures, - // Ensure break duration is well over the max delay for a single request, so that the - // break doesn't end during a retry attempt - onBreak: onBreakHandler, - onDegraded: onDegradedHandler, - circuitBreakDuration: defaultMaxRetryDelay * 10, - }); - const fetchTokenPrices = () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }); - expect(onBreakHandler).not.toHaveBeenCalled(); - // Initial three calls to exhaust maximum allowed failures - // eslint-disable-next-line @typescript-eslint/no-unused-vars - for (const _retryAttempt of Array(retries).keys()) { - // eslint-disable-next-line no-loop-func - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow('Failed to fetch'); - } - // Confirm that circuit is broken expect(onBreakHandler).toHaveBeenCalledTimes(1); - // Should be called twice by now, once per update attempt prior to break - expect(onDegradedHandler).toHaveBeenCalledTimes(2); - - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - - expect(onDegradedHandler).toHaveBeenCalledTimes(2); - }); - - it('keeps circuit closed if first request fails when half-open', async () => { - const retries = 3; - // Max consencutive failures is set to match number of calls in three update attempts (including retries) - const maximumConsecutiveFailures = (1 + retries) * 3; - // Ensure break duration is well over the max delay for a single request, so that the - // break doesn't end during a retry attempt - const circuitBreakDuration = defaultMaxRetryDelay * 10; - // Initial interceptor for failing requests - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - // The +1 is for the additional request when the circuit is half-open - .times(maximumConsecutiveFailures + 1) - .replyWithError('Failed to fetch'); - // This interceptor should not be used - const successfullCallScope = nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: '0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - }) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - }); - const service = new CodefiTokenPricesServiceV2({ - retries, - maximumConsecutiveFailures, - circuitBreakDuration, - }); - const fetchTokenPrices = () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }); - // Initial three calls to exhaust maximum allowed failures - // eslint-disable-next-line @typescript-eslint/no-unused-vars - for (const _retryAttempt of Array(retries).keys()) { - // eslint-disable-next-line no-loop-func - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow('Failed to fetch'); - } - // Confirm that circuit has broken - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - // Wait for circuit to move to half-open - await clock.tickAsync(circuitBreakDuration); - - // The circuit should remain open after the first request fails - // The fetch error is replaced by the circuit break error due to the retries - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - - // Confirm that the circuit is still open - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - expect(successfullCallScope.isDone()).toBe(false); - }); - - it('recovers after circuit break', async () => { - const retries = 3; - // Max consencutive failures is set to match number of calls in three update attempts (including retries) - const maximumConsecutiveFailures = (1 + retries) * 3; - // Ensure break duration is well over the max delay for a single request, so that the - // break doesn't end during a retry attempt - const circuitBreakDuration = defaultMaxRetryDelay * 10; - // Initial interceptor for failing requests - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .times(maximumConsecutiveFailures) - .replyWithError('Failed to fetch'); - // Later interceptor for successfull request after recovery - nock('https://price.api.cx.metamask.io') - .get('/v2/chains/1/spot-prices') - .query({ - tokenAddresses: - '0x0000000000000000000000000000000000000000,0xAAA,0xBBB,0xCCC', - vsCurrency: 'ETH', - includeMarketData: 'true', - }) - .reply(200, { - '0x0000000000000000000000000000000000000000': { - price: 14, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xaaa': { - price: 148.17205755299946, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xbbb': { - price: 33689.98134554716, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xccc': { - price: 148.1344197578456, - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - }); - const service = new CodefiTokenPricesServiceV2({ - retries, - maximumConsecutiveFailures, - circuitBreakDuration, - }); - const fetchTokenPrices = () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }); - // Initial three calls to exhaust maximum allowed failures - // eslint-disable-next-line @typescript-eslint/no-unused-vars - for (const _retryAttempt of Array(retries).keys()) { - // eslint-disable-next-line no-loop-func - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow('Failed to fetch'); - } - // Confirm that circuit has broken - await expect(() => - fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - // Wait for circuit to move to half-open - await clock.tickAsync(circuitBreakDuration); - - const marketDataTokensByAddress = await fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices, - retries, - }); - - expect(marketDataTokensByAddress).toStrictEqual({ - '0x0000000000000000000000000000000000000000': { - tokenAddress: '0x0000000000000000000000000000000000000000', - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - price: 14, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xAAA': { - tokenAddress: '0xAAA', - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - price: 148.17205755299946, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xBBB': { - tokenAddress: '0xBBB', - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - price: 33689.98134554716, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - '0xCCC': { - tokenAddress: '0xCCC', - currency: 'ETH', - pricePercentChange1d: 1, - priceChange1d: 1, - marketCap: 117219.99428314982, - allTimeHigh: 0.00060467892389492, - allTimeLow: 0.00002303954000865728, - totalVolume: 5155.094053542448, - high1d: 0.00008020715848194385, - low1d: 0.00007792083564549064, - price: 148.1344197578456, - circulatingSupply: 1494269733.9526057, - dilutedMarketCap: 117669.5125951733, - marketCapPercentChange1d: 0.76671, - pricePercentChange1h: -1.0736342953259423, - pricePercentChange7d: -7.351582573655089, - pricePercentChange14d: -1.0799098946709822, - pricePercentChange30d: -25.776321124365992, - pricePercentChange200d: 46.091571238599165, - pricePercentChange1y: -2.2992517267242754, - }, - }); }); }); }); diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts index 4f163203e4d..cbc3f05b60f 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -1,19 +1,17 @@ -import { handleFetch } from '@metamask/controller-utils'; +import { + createServicePolicy, + DEFAULT_CIRCUIT_BREAK_DURATION, + DEFAULT_DEGRADED_THRESHOLD, + DEFAULT_MAX_CONSECUTIVE_FAILURES, + DEFAULT_MAX_RETRIES, + handleFetch, +} from '@metamask/controller-utils'; +import type { ServicePolicy } from '@metamask/controller-utils'; import type { Hex } from '@metamask/utils'; import { hexToNumber } from '@metamask/utils'; -import { - circuitBreaker, - ConsecutiveBreaker, - ExponentialBackoff, - handleAll, - type IPolicy, - retry, - wrap, - CircuitState, -} from 'cockatiel'; import type { - AbstractTokenPricesService, + IAbstractTokenPricesService, TokenPrice, TokenPricesByTokenAddress, } from './abstract-token-prices-service'; @@ -271,13 +269,6 @@ type SupportedChainId = (typeof SUPPORTED_CHAIN_IDS)[number]; */ const BASE_URL = 'https://price.api.cx.metamask.io/v2'; -const DEFAULT_TOKEN_PRICE_RETRIES = 3; -// Each update attempt will result (1 + retries) calls if the server is down -const DEFAULT_TOKEN_PRICE_MAX_CONSECUTIVE_FAILURES = - (1 + DEFAULT_TOKEN_PRICE_RETRIES) * 3; - -const DEFAULT_DEGRADED_THRESHOLD = 5_000; - /** * The shape of the data that the /spot-prices endpoint returns. */ @@ -363,33 +354,66 @@ type MarketDataByTokenAddress = { [address: Hex]: MarketData }; */ export class CodefiTokenPricesServiceV2 implements - AbstractTokenPricesService + IAbstractTokenPricesService { - #tokenPricePolicy: IPolicy; + readonly #policy: ServicePolicy; /** * Construct a Codefi Token Price Service. * - * @param options - Constructor options - * @param options.degradedThreshold - The threshold between "normal" and "degrated" service, - * in milliseconds. - * @param options.retries - Number of retry attempts for each token price update. - * @param options.maximumConsecutiveFailures - The maximum number of consecutive failures - * allowed before breaking the circuit and pausing further updates. - * @param options.onBreak - An event handler for when the circuit breaks, useful for capturing - * metrics about network failures. - * @param options.onDegraded - An event handler for when the circuit remains closed, but requests - * are failing or resolving too slowly (i.e. resolving more slowly than the `degradedThreshold). - * @param options.circuitBreakDuration - The amount of time to wait when the circuit breaks - * from too many consecutive failures. + * @param args - The arguments. + * @param args.degradedThreshold - The length of time (in milliseconds) + * that governs when the service is regarded as degraded (affecting when + * `onDegraded` is called). Defaults to 5 seconds. + * @param args.retries - Number of retry attempts for each fetch request. + * @param args.maximumConsecutiveFailures - The maximum number of consecutive + * failures allowed before breaking the circuit and pausing further updates. + * @param args.circuitBreakDuration - The amount of time to wait when the + * circuit breaks from too many consecutive failures. */ + constructor(args?: { + degradedThreshold?: number; + retries?: number; + maximumConsecutiveFailures?: number; + circuitBreakDuration?: number; + }); + + /** + * Construct a Codefi Token Price Service. + * + * @deprecated This signature is deprecated; please use the `onBreak` and + * `onDegraded` methods instead. + * @param args - The arguments. + * @param args.degradedThreshold - The length of time (in milliseconds) + * that governs when the service is regarded as degraded (affecting when + * `onDegraded` is called). Defaults to 5 seconds. + * @param args.retries - Number of retry attempts for each fetch request. + * @param args.maximumConsecutiveFailures - The maximum number of consecutive + * failures allowed before breaking the circuit and pausing further updates. + * @param args.onBreak - Callback for when the circuit breaks, useful + * for capturing metrics about network failures. + * @param args.onDegraded - Callback for when the API responds successfully + * but takes too long to respond (5 seconds or more). + * @param args.circuitBreakDuration - The amount of time to wait when the + * circuit breaks from too many consecutive failures. + */ + // eslint-disable-next-line @typescript-eslint/unified-signatures + constructor(args?: { + degradedThreshold?: number; + retries?: number; + maximumConsecutiveFailures?: number; + onBreak?: () => void; + onDegraded?: () => void; + circuitBreakDuration?: number; + }); + constructor({ degradedThreshold = DEFAULT_DEGRADED_THRESHOLD, - retries = DEFAULT_TOKEN_PRICE_RETRIES, - maximumConsecutiveFailures = DEFAULT_TOKEN_PRICE_MAX_CONSECUTIVE_FAILURES, + retries = DEFAULT_MAX_RETRIES, + maximumConsecutiveFailures = DEFAULT_MAX_CONSECUTIVE_FAILURES, onBreak, onDegraded, - circuitBreakDuration = 30 * 60 * 1000, + circuitBreakDuration = DEFAULT_CIRCUIT_BREAK_DURATION, }: { degradedThreshold?: number; retries?: number; @@ -398,35 +422,40 @@ export class CodefiTokenPricesServiceV2 onDegraded?: () => void; circuitBreakDuration?: number; } = {}) { - // Construct a policy that will retry each update, and halt further updates - // for a certain period after too many consecutive failures. - const retryPolicy = retry(handleAll, { - maxAttempts: retries, - backoff: new ExponentialBackoff(), - }); - const circuitBreakerPolicy = circuitBreaker(handleAll, { - halfOpenAfter: circuitBreakDuration, - breaker: new ConsecutiveBreaker(maximumConsecutiveFailures), + this.#policy = createServicePolicy({ + maxRetries: retries, + maxConsecutiveFailures: maximumConsecutiveFailures, + circuitBreakDuration, + degradedThreshold, }); if (onBreak) { - circuitBreakerPolicy.onBreak(onBreak); + this.#policy.onBreak(onBreak); } if (onDegraded) { - retryPolicy.onGiveUp(() => { - if (circuitBreakerPolicy.state === CircuitState.Closed) { - onDegraded(); - } - }); - retryPolicy.onSuccess(({ duration }) => { - if ( - circuitBreakerPolicy.state === CircuitState.Closed && - duration > degradedThreshold - ) { - onDegraded(); - } - }); + this.#policy.onDegraded(onDegraded); } - this.#tokenPricePolicy = wrap(retryPolicy, circuitBreakerPolicy); + } + + /** + * Listens for when the request to the API fails too many times in a row. + * + * @param args - The same arguments that {@link ServicePolicy.onBreak} + * takes. + * @returns What {@link ServicePolicy.onBreak} returns. + */ + onBreak(...args: Parameters) { + return this.#policy.onBreak(...args); + } + + /** + * Listens for when the API is degraded. + * + * @param args - The same arguments that {@link ServicePolicy.onDegraded} + * takes. + * @returns What {@link ServicePolicy.onDegraded} returns. + */ + onDegraded(...args: Parameters) { + return this.#policy.onDegraded(...args); } /** @@ -459,7 +488,7 @@ export class CodefiTokenPricesServiceV2 url.searchParams.append('includeMarketData', 'true'); const addressCryptoDataMap: MarketDataByTokenAddress = - await this.#tokenPricePolicy.execute(() => + await this.#policy.execute(() => handleFetch(url, { headers: { 'Cache-Control': 'no-cache' } }), ); diff --git a/packages/assets-controllers/src/token-prices-service/index.ts b/packages/assets-controllers/src/token-prices-service/index.ts index 509fc680055..2a69f5890e7 100644 --- a/packages/assets-controllers/src/token-prices-service/index.ts +++ b/packages/assets-controllers/src/token-prices-service/index.ts @@ -1,4 +1,4 @@ -export type { AbstractTokenPricesService } from './abstract-token-prices-service'; +export type { IAbstractTokenPricesService } from './abstract-token-prices-service'; export { CodefiTokenPricesServiceV2, SUPPORTED_CHAIN_IDS, diff --git a/packages/remote-feature-flag-controller/CHANGELOG.md b/packages/remote-feature-flag-controller/CHANGELOG.md index 20df3a0a4ca..b688177db93 100644 --- a/packages/remote-feature-flag-controller/CHANGELOG.md +++ b/packages/remote-feature-flag-controller/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `onBreak` and `onDegraded` methods to `ClientConfigApiService` ([#5109](https://github.com/MetaMask/core/pull/5109)) + - These serve the same purpose as the `onBreak` and `onDegraded` constructor options, but align more closely with the Cockatiel policy API. + +### Changed + +- Deprecate `ClientConfigApiService` constructor options `onBreak` and `onDegraded` in favor of methods ([#5109](https://github.com/MetaMask/core/pull/5109)) +- Add `@metamask/controller-utils@^11.4.5` as a dependency ([#5109](https://github.com/MetaMask/core/pull/5109)) + - `cockatiel` should still be in the dependency tree because it's now a dependency of `@metamask/controller-utils` + ## [1.3.0] ### Changed diff --git a/packages/remote-feature-flag-controller/package.json b/packages/remote-feature-flag-controller/package.json index 38128e1b2df..7184fd4a618 100644 --- a/packages/remote-feature-flag-controller/package.json +++ b/packages/remote-feature-flag-controller/package.json @@ -48,14 +48,13 @@ }, "dependencies": { "@metamask/base-controller": "^7.1.1", + "@metamask/controller-utils": "^11.4.5", "@metamask/utils": "^11.0.1", - "cockatiel": "^3.1.2", "uuid": "^8.3.2" }, "devDependencies": { "@lavamoat/allow-scripts": "^3.0.4", "@metamask/auto-changelog": "^3.4.4", - "@metamask/controller-utils": "^11.4.5", "@types/jest": "^27.4.1", "deepmerge": "^4.2.2", "jest": "^27.5.1", diff --git a/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts b/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts index a7def3bef56..ee7c6bf8787 100644 --- a/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts +++ b/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts @@ -1,9 +1,20 @@ -import type { PublicInterface } from '@metamask/utils'; +import type { ServicePolicy } from '@metamask/controller-utils'; -import type { ClientConfigApiService } from './client-config-api-service'; +import type { ServiceResponse } from '../remote-feature-flag-controller-types'; /** * A service object responsible for fetching feature flags. */ -export type AbstractClientConfigApiService = - PublicInterface; +export type IAbstractClientConfigApiService = Partial< + Pick +> & { + /** + * Fetches feature flags from the API with specific client, distribution, and + * environment parameters. Provides structured error handling, including + * fallback to cached data if available. + * + * @returns An object of feature flags and their boolean values or a + * structured error object. + */ + fetchRemoteFeatureFlags(): Promise; +}; diff --git a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts index 1baaaf72bf4..0cd8756d47c 100644 --- a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts +++ b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts @@ -20,9 +20,68 @@ const mockFeatureFlags: FeatureFlags = { feature2: { chrome: '<109' }, }; +jest.setTimeout(8000); + describe('ClientConfigApiService', () => { const networkError = new Error('Network error'); + describe('onBreak', () => { + it('should register a listener that is called when the circuit opens', async () => { + const onBreak = jest.fn(); + const mockFetch = createMockFetch({ error: networkError }); + + const clientConfigApiService = new ClientConfigApiService({ + fetch: mockFetch, + maximumConsecutiveFailures: 1, + config: { + client: ClientType.Extension, + distribution: DistributionType.Main, + environment: EnvironmentType.Production, + }, + }); + clientConfigApiService.onBreak(onBreak); + + await expect( + clientConfigApiService.fetchRemoteFeatureFlags(), + ).rejects.toThrow( + 'Execution prevented because the circuit breaker is open', + ); + + expect(onBreak).toHaveBeenCalled(); + }); + }); + + describe('onDegraded', () => { + it('should register a listener that is called when the request is slow', async () => { + const onDegraded = jest.fn(); + const slowFetchTime = 5500; // Exceed the DEFAULT_DEGRADED_THRESHOLD (5000ms) + // Mock fetch to take a long time + const mockSlowFetch = createMockFetch({ + response: { + ok: true, + status: 200, + json: async () => mockServerFeatureFlagsResponse, + }, + delay: slowFetchTime, + }); + + const clientConfigApiService = new ClientConfigApiService({ + fetch: mockSlowFetch, + config: { + client: ClientType.Extension, + distribution: DistributionType.Main, + environment: EnvironmentType.Production, + }, + }); + clientConfigApiService.onDegraded(onDegraded); + + await clientConfigApiService.fetchRemoteFeatureFlags(); + + // Verify the degraded callback was called + expect(onDegraded).toHaveBeenCalled(); + }, 7000); + }); + describe('fetchRemoteFeatureFlags', () => { it('fetches successfully and returns feature flags', async () => { const mockFetch = createMockFetch({ @@ -132,43 +191,8 @@ describe('ClientConfigApiService', () => { // Check that fetch was retried the correct number of times expect(mockFetch).toHaveBeenCalledTimes(maxRetries + 1); // Initial + retries }); - }); - - describe('circuit breaker', () => { - it('opens the circuit breaker after consecutive failures', async () => { - const mockFetch = createMockFetch({ error: networkError }); - const maxFailures = 3; - const clientConfigApiService = new ClientConfigApiService({ - fetch: mockFetch, - maximumConsecutiveFailures: maxFailures, - config: { - client: ClientType.Extension, - distribution: DistributionType.Main, - environment: EnvironmentType.Production, - }, - }); - - // Attempt requests until circuit breaker opens - for (let i = 0; i < maxFailures; i++) { - await expect( - clientConfigApiService.fetchRemoteFeatureFlags(), - ).rejects.toThrow( - /Network error|Execution prevented because the circuit breaker is open/u, - ); - } - - // Verify the circuit breaker is now open - await expect( - clientConfigApiService.fetchRemoteFeatureFlags(), - ).rejects.toThrow( - 'Execution prevented because the circuit breaker is open', - ); - - // Verify fetch was called the expected number of times - expect(mockFetch).toHaveBeenCalledTimes(maxFailures); - }); - it('should call onBreak when circuit breaker opens', async () => { + it('should call the onBreak callback when the circuit opens', async () => { const onBreak = jest.fn(); const mockFetch = createMockFetch({ error: networkError }); @@ -192,8 +216,7 @@ describe('ClientConfigApiService', () => { expect(onBreak).toHaveBeenCalled(); }); - it('should call the onDegraded callback when requests are slow', async () => { - jest.setTimeout(7000); + it('should call the onDegraded callback when the request is slow', async () => { const onDegraded = jest.fn(); const slowFetchTime = 5500; // Exceed the DEFAULT_DEGRADED_THRESHOLD (5000ms) // Mock fetch to take a long time @@ -221,64 +244,6 @@ describe('ClientConfigApiService', () => { // Verify the degraded callback was called expect(onDegraded).toHaveBeenCalled(); }, 7000); - - it('should succeed on a subsequent fetch attempt after retries', async () => { - const maxRetries = 2; - // Mock fetch to fail initially, then succeed - const mockFetch = jest - .fn() - .mockRejectedValueOnce(networkError) - .mockRejectedValueOnce(networkError) - .mockResolvedValueOnce({ - ok: true, - status: 200, - statusText: 'OK', - json: async () => mockServerFeatureFlagsResponse, - }); - - const clientConfigApiService = new ClientConfigApiService({ - fetch: mockFetch, - retries: maxRetries, - config: { - client: ClientType.Extension, - distribution: DistributionType.Main, - environment: EnvironmentType.Production, - }, - }); - - const result = await clientConfigApiService.fetchRemoteFeatureFlags(); - - expect(result).toStrictEqual({ - remoteFeatureFlags: mockFeatureFlags, - cacheTimestamp: expect.any(Number), - }); - - expect(mockFetch).toHaveBeenCalledTimes(maxRetries + 1); - }); - - it('calls onDegraded when retries are exhausted and circuit is closed', async () => { - const onDegraded = jest.fn(); - const mockFetch = jest.fn(); - mockFetch.mockRejectedValue(new Error('Network error')); - - const service = new ClientConfigApiService({ - fetch: mockFetch, - retries: 2, - onDegraded, - config: { - client: ClientType.Extension, - distribution: DistributionType.Main, - environment: EnvironmentType.Production, - }, - }); - - await expect(service.fetchRemoteFeatureFlags()).rejects.toThrow( - 'Network error', - ); - - // Should be called once after all retries are exhausted - expect(onDegraded).toHaveBeenCalledTimes(1); - }); }); }); diff --git a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts index 7cd9177e0b9..ef525e76954 100644 --- a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts +++ b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts @@ -1,14 +1,12 @@ import { - circuitBreaker, - ConsecutiveBreaker, - ExponentialBackoff, - handleAll, - type IPolicy, - retry, - wrap, - CircuitState, -} from 'cockatiel'; - + createServicePolicy, + DEFAULT_CIRCUIT_BREAK_DURATION, + DEFAULT_MAX_CONSECUTIVE_FAILURES, + DEFAULT_MAX_RETRIES, +} from '@metamask/controller-utils'; +import type { ServicePolicy } from '@metamask/controller-utils'; + +import type { IAbstractClientConfigApiService } from './abstract-client-config-api-service'; import { BASE_URL } from '../constants'; import type { FeatureFlags, @@ -19,19 +17,13 @@ import type { ApiDataResponse, } from '../remote-feature-flag-controller-types'; -const DEFAULT_FETCH_RETRIES = 3; -// Each update attempt will result (1 + retries) calls if the server is down -const DEFAULT_MAX_CONSECUTIVE_FAILURES = (1 + DEFAULT_FETCH_RETRIES) * 3; - -export const DEFAULT_DEGRADED_THRESHOLD = 5000; - /** * This service is responsible for fetching feature flags from the ClientConfig API. */ -export class ClientConfigApiService { +export class ClientConfigApiService implements IAbstractClientConfigApiService { #fetch: typeof fetch; - #policy: IPolicy; + readonly #policy: ServicePolicy; #client: ClientType; @@ -44,23 +36,83 @@ export class ClientConfigApiService { * * @param args - The arguments. * @param args.fetch - A function that can be used to make an HTTP request. + * If your JavaScript environment supports `fetch` natively, you'll probably + * want to pass that; otherwise you can pass an equivalent (such as `fetch` + * via `node-fetch`). * @param args.retries - Number of retry attempts for each fetch request. - * @param args.maximumConsecutiveFailures - The maximum number of consecutive failures - * allowed before breaking the circuit and pausing further fetch attempts. - * @param args.circuitBreakDuration - The duration for which the circuit remains open after - * too many consecutive failures. - * @param args.onBreak - Callback invoked when the circuit breaks. - * @param args.onDegraded - Callback invoked when the service is degraded (requests resolving too slowly). - * @param args.config - The configuration object, includes client, distribution, and environment. + * @param args.maximumConsecutiveFailures - The maximum number of consecutive + * failures allowed before breaking the circuit and pausing further fetch + * attempts. + * @param args.circuitBreakDuration - The amount of time to wait when the + * circuit breaks from too many consecutive failures. + * @param args.config - The configuration object, includes client, + * distribution, and environment. * @param args.config.client - The client type (e.g., 'extension', 'mobile'). - * @param args.config.distribution - The distribution type (e.g., 'main', 'flask'). - * @param args.config.environment - The environment type (e.g., 'prod', 'rc', 'dev'). + * @param args.config.distribution - The distribution type (e.g., 'main', + * 'flask'). + * @param args.config.environment - The environment type (e.g., 'prod', 'rc', + * 'dev'). */ + constructor(args: { + fetch: typeof fetch; + retries?: number; + maximumConsecutiveFailures?: number; + circuitBreakDuration?: number; + config: { + client: ClientType; + distribution: DistributionType; + environment: EnvironmentType; + }; + }); + + /** + * Constructs a new ClientConfigApiService object. + * + * @deprecated This signature is deprecated; please use the `onBreak` and + * `onDegraded` methods instead. + * @param args - The arguments. + * @param args.fetch - A function that can be used to make an HTTP request. + * If your JavaScript environment supports `fetch` natively, you'll probably + * want to pass that; otherwise you can pass an equivalent (such as `fetch` + * via `node-fetch`). + * @param args.retries - Number of retry attempts for each fetch request. + * @param args.maximumConsecutiveFailures - The maximum number of consecutive + * failures allowed before breaking the circuit and pausing further fetch + * attempts. + * @param args.circuitBreakDuration - The amount of time to wait when the + * circuit breaks from too many consecutive failures. + * @param args.onBreak - Callback for when the circuit breaks, useful + * for capturing metrics about network failures. + * @param args.onDegraded - Callback for when the API responds successfully + * but takes too long to respond (5 seconds or more). + * @param args.config - The configuration object, includes client, + * distribution, and environment. + * @param args.config.client - The client type (e.g., 'extension', 'mobile'). + * @param args.config.distribution - The distribution type (e.g., 'main', + * 'flask'). + * @param args.config.environment - The environment type (e.g., 'prod', 'rc', + * 'dev'). + */ + // eslint-disable-next-line @typescript-eslint/unified-signatures + constructor(args: { + fetch: typeof fetch; + retries?: number; + maximumConsecutiveFailures?: number; + circuitBreakDuration?: number; + onBreak?: () => void; + onDegraded?: () => void; + config: { + client: ClientType; + distribution: DistributionType; + environment: EnvironmentType; + }; + }); + constructor({ fetch: fetchFunction, - retries = DEFAULT_FETCH_RETRIES, + retries = DEFAULT_MAX_RETRIES, maximumConsecutiveFailures = DEFAULT_MAX_CONSECUTIVE_FAILURES, - circuitBreakDuration = 30 * 60 * 1000, + circuitBreakDuration = DEFAULT_CIRCUIT_BREAK_DURATION, onBreak, onDegraded, config, @@ -82,38 +134,39 @@ export class ClientConfigApiService { this.#distribution = config.distribution; this.#environment = config.environment; - const retryPolicy = retry(handleAll, { - maxAttempts: retries, - backoff: new ExponentialBackoff(), - }); - - const circuitBreakerPolicy = circuitBreaker(handleAll, { - halfOpenAfter: circuitBreakDuration, - breaker: new ConsecutiveBreaker(maximumConsecutiveFailures), + this.#policy = createServicePolicy({ + maxRetries: retries, + maxConsecutiveFailures: maximumConsecutiveFailures, + circuitBreakDuration, }); - if (onBreak) { - circuitBreakerPolicy.onBreak(onBreak); + this.#policy.onBreak(onBreak); } - if (onDegraded) { - retryPolicy.onGiveUp(() => { - if (circuitBreakerPolicy.state === CircuitState.Closed) { - onDegraded(); - } - }); - - retryPolicy.onSuccess(({ duration }) => { - if ( - circuitBreakerPolicy.state === CircuitState.Closed && - duration > DEFAULT_DEGRADED_THRESHOLD // Default degraded threshold - ) { - onDegraded(); - } - }); + this.#policy.onDegraded(onDegraded); } + } - this.#policy = wrap(retryPolicy, circuitBreakerPolicy); + /** + * Listens for when the request to the API fails too many times in a row. + * + * @param args - The same arguments that {@link ServicePolicy.onBreak} + * takes. + * @returns What {@link ServicePolicy.onBreak} returns. + */ + onBreak(...args: Parameters) { + return this.#policy.onBreak(...args); + } + + /** + * Listens for when the API is degraded. + * + * @param args - The same arguments that {@link ServicePolicy.onDegraded} + * takes. + * @returns What {@link ServicePolicy.onDegraded} returns. + */ + onDegraded(...args: Parameters) { + return this.#policy.onDegraded(...args); } /** diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts index 5def1e901a9..84af924058c 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts @@ -1,6 +1,6 @@ import { ControllerMessenger } from '@metamask/base-controller'; -import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; +import type { IAbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import { RemoteFeatureFlagController, controllerName, @@ -55,7 +55,7 @@ function createController( options: Partial<{ messenger: RemoteFeatureFlagControllerMessenger; state: Partial; - clientConfigApiService: AbstractClientConfigApiService; + clientConfigApiService: IAbstractClientConfigApiService; disabled: boolean; getMetaMetricsId: () => string; }> = {}, @@ -218,7 +218,7 @@ describe('RemoteFeatureFlagController', () => { const clientConfigApiService = { fetchRemoteFeatureFlags: fetchSpy, - } as AbstractClientConfigApiService; + } as IAbstractClientConfigApiService; const controller = createController({ clientConfigApiService, @@ -387,7 +387,7 @@ function buildClientConfigApiService({ remoteFeatureFlags?: FeatureFlags; cacheTimestamp?: number; error?: Error; -} = {}): AbstractClientConfigApiService { +} = {}): IAbstractClientConfigApiService { return { fetchRemoteFeatureFlags: jest.fn().mockImplementation(() => { if (error) { diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts index 9e370998baa..2664ff1ad90 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts @@ -5,7 +5,7 @@ import type { } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; +import type { IAbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import type { FeatureFlags, ServiceResponse, @@ -98,7 +98,7 @@ export class RemoteFeatureFlagController extends BaseController< #disabled: boolean; - #clientConfigApiService: AbstractClientConfigApiService; + readonly #clientConfigApiService: IAbstractClientConfigApiService; #inProgressFlagUpdate?: Promise; @@ -125,7 +125,7 @@ export class RemoteFeatureFlagController extends BaseController< }: { messenger: RemoteFeatureFlagControllerMessenger; state?: Partial; - clientConfigApiService: AbstractClientConfigApiService; + clientConfigApiService: IAbstractClientConfigApiService; getMetaMetricsId: () => string; fetchInterval?: number; disabled?: boolean; @@ -193,9 +193,7 @@ export class RemoteFeatureFlagController extends BaseController< * @private */ async #updateCache(remoteFeatureFlags: FeatureFlags) { - const processedRemoteFeatureFlags = await this.#processRemoteFeatureFlags( - remoteFeatureFlags, - ); + const processedRemoteFeatureFlags = await this.#processRemoteFeatureFlags(remoteFeatureFlags); this.update(() => { return { remoteFeatureFlags: processedRemoteFeatureFlags, diff --git a/yarn.lock b/yarn.lock index c48d3f85a7d..bf510aad60a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2447,7 +2447,6 @@ __metadata: async-mutex: "npm:^0.5.0" bitcoin-address-validation: "npm:^2.2.3" bn.js: "npm:^5.2.1" - cockatiel: "npm:^3.1.2" deepmerge: "npm:^4.2.2" immer: "npm:^9.0.6" jest: "npm:^27.5.1" @@ -3799,7 +3798,6 @@ __metadata: "@metamask/controller-utils": "npm:^11.4.5" "@metamask/utils": "npm:^11.0.1" "@types/jest": "npm:^27.4.1" - cockatiel: "npm:^3.1.2" deepmerge: "npm:^4.2.2" jest: "npm:^27.5.1" nock: "npm:^13.3.1" From cda581fa2528d2a9695c645091cd2a28e481813e Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 23 Jan 2025 16:54:13 -0700 Subject: [PATCH 3/3] Remove 'I' prefix from interfaces --- .../assets-controllers/src/TokenRatesController.test.ts | 6 +++--- packages/assets-controllers/src/TokenRatesController.ts | 8 ++++---- packages/assets-controllers/src/assetsUtil.test.ts | 4 ++-- packages/assets-controllers/src/assetsUtil.ts | 6 +++--- .../token-prices-service/abstract-token-prices-service.ts | 2 +- .../src/token-prices-service/codefi-v2.ts | 4 ++-- .../assets-controllers/src/token-prices-service/index.ts | 2 +- .../abstract-client-config-api-service.ts | 2 +- .../client-config-api-service.ts | 4 ++-- .../src/remote-feature-flag-controller.test.ts | 8 ++++---- .../src/remote-feature-flag-controller.ts | 6 +++--- 11 files changed, 26 insertions(+), 26 deletions(-) diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index 05378c15a80..ee01c088e3d 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -30,7 +30,7 @@ import { } from '../../network-controller/tests/helpers'; import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import type { - IAbstractTokenPricesService, + AbstractTokenPricesService, TokenPrice, TokenPricesByTokenAddress, } from './token-prices-service/abstract-token-prices-service'; @@ -2754,8 +2754,8 @@ async function callUpdateExchangeRatesMethod({ * @returns The built mock token prices service. */ function buildMockTokenPricesService( - overrides: Partial = {}, -): IAbstractTokenPricesService { + overrides: Partial = {}, +): AbstractTokenPricesService { return { async fetchTokenPrices() { return {}; diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index 5bca2809ea0..61f569ee963 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -26,7 +26,7 @@ import { isEqual } from 'lodash'; import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service'; -import type { IAbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; +import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import { getNativeTokenAddress } from './token-prices-service/codefi-v2'; import type { TokensControllerGetStateAction, @@ -238,7 +238,7 @@ export class TokenRatesController extends StaticIntervalPollingController> = {}; @@ -275,7 +275,7 @@ export class TokenRatesController extends StaticIntervalPollingController; }) { @@ -687,7 +687,7 @@ export class TokenRatesController extends StaticIntervalPollingController> + Awaited> >({ values: [...tokenAddresses].sort(), batchSize: TOKEN_PRICES_BATCH_SIZE, diff --git a/packages/assets-controllers/src/assetsUtil.test.ts b/packages/assets-controllers/src/assetsUtil.test.ts index a40eccfa542..17dc36ebf93 100644 --- a/packages/assets-controllers/src/assetsUtil.test.ts +++ b/packages/assets-controllers/src/assetsUtil.test.ts @@ -10,7 +10,7 @@ import { add0x, type Hex } from '@metamask/utils'; import * as assetsUtil from './assetsUtil'; import { TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; import type { Nft, NftMetadata } from './NftController'; -import type { IAbstractTokenPricesService } from './token-prices-service'; +import type { AbstractTokenPricesService } from './token-prices-service'; const DEFAULT_IPFS_URL_FORMAT = 'ipfs://'; const ALTERNATIVE_IPFS_URL_FORMAT = 'ipfs://ipfs/'; @@ -772,7 +772,7 @@ function buildAddress(number: number) { * * @returns The mocked functions of token prices service. */ -function createMockPriceService(): IAbstractTokenPricesService { +function createMockPriceService(): AbstractTokenPricesService { return { validateChainIdSupported(_chainId: unknown): _chainId is Hex { return true; diff --git a/packages/assets-controllers/src/assetsUtil.ts b/packages/assets-controllers/src/assetsUtil.ts index 52c0965246d..48b0bcde927 100644 --- a/packages/assets-controllers/src/assetsUtil.ts +++ b/packages/assets-controllers/src/assetsUtil.ts @@ -8,7 +8,7 @@ import { remove0x } from '@metamask/utils'; import BN from 'bn.js'; import type { Nft, NftMetadata } from './NftController'; -import type { IAbstractTokenPricesService } from './token-prices-service'; +import type { AbstractTokenPricesService } from './token-prices-service'; import { type ContractExchangeRates } from './TokenRatesController'; /** @@ -387,7 +387,7 @@ export async function fetchTokenContractExchangeRates({ tokenAddresses, chainId, }: { - tokenPricesService: IAbstractTokenPricesService; + tokenPricesService: AbstractTokenPricesService; nativeCurrency: string; tokenAddresses: Hex[]; chainId: Hex; @@ -403,7 +403,7 @@ export async function fetchTokenContractExchangeRates({ const tokenPricesByTokenAddress = await reduceInBatchesSerially< Hex, - Awaited> + Awaited> >({ values: [...tokenAddresses].sort(), batchSize: TOKEN_PRICES_BATCH_SIZE, diff --git a/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts b/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts index bac95f74fec..7e705a33ab6 100644 --- a/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts +++ b/packages/assets-controllers/src/token-prices-service/abstract-token-prices-service.ts @@ -50,7 +50,7 @@ export type TokenPricesByTokenAddress< * @template Currency - A type union of valid arguments for the `currency` * argument to `fetchTokenPrices`. */ -export type IAbstractTokenPricesService< +export type AbstractTokenPricesService< ChainId extends Hex = Hex, TokenAddress extends Hex = Hex, Currency extends string = string, diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts index cbc3f05b60f..901d18f7245 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -11,7 +11,7 @@ import type { Hex } from '@metamask/utils'; import { hexToNumber } from '@metamask/utils'; import type { - IAbstractTokenPricesService, + AbstractTokenPricesService, TokenPrice, TokenPricesByTokenAddress, } from './abstract-token-prices-service'; @@ -354,7 +354,7 @@ type MarketDataByTokenAddress = { [address: Hex]: MarketData }; */ export class CodefiTokenPricesServiceV2 implements - IAbstractTokenPricesService + AbstractTokenPricesService { readonly #policy: ServicePolicy; diff --git a/packages/assets-controllers/src/token-prices-service/index.ts b/packages/assets-controllers/src/token-prices-service/index.ts index 2a69f5890e7..509fc680055 100644 --- a/packages/assets-controllers/src/token-prices-service/index.ts +++ b/packages/assets-controllers/src/token-prices-service/index.ts @@ -1,4 +1,4 @@ -export type { IAbstractTokenPricesService } from './abstract-token-prices-service'; +export type { AbstractTokenPricesService } from './abstract-token-prices-service'; export { CodefiTokenPricesServiceV2, SUPPORTED_CHAIN_IDS, diff --git a/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts b/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts index ee7c6bf8787..7a07ed2db9b 100644 --- a/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts +++ b/packages/remote-feature-flag-controller/src/client-config-api-service/abstract-client-config-api-service.ts @@ -5,7 +5,7 @@ import type { ServiceResponse } from '../remote-feature-flag-controller-types'; /** * A service object responsible for fetching feature flags. */ -export type IAbstractClientConfigApiService = Partial< +export type AbstractClientConfigApiService = Partial< Pick > & { /** diff --git a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts index ef525e76954..73e642526d8 100644 --- a/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts +++ b/packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.ts @@ -6,7 +6,7 @@ import { } from '@metamask/controller-utils'; import type { ServicePolicy } from '@metamask/controller-utils'; -import type { IAbstractClientConfigApiService } from './abstract-client-config-api-service'; +import type { AbstractClientConfigApiService } from './abstract-client-config-api-service'; import { BASE_URL } from '../constants'; import type { FeatureFlags, @@ -20,7 +20,7 @@ import type { /** * This service is responsible for fetching feature flags from the ClientConfig API. */ -export class ClientConfigApiService implements IAbstractClientConfigApiService { +export class ClientConfigApiService implements AbstractClientConfigApiService { #fetch: typeof fetch; readonly #policy: ServicePolicy; diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts index 84af924058c..5def1e901a9 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.test.ts @@ -1,6 +1,6 @@ import { ControllerMessenger } from '@metamask/base-controller'; -import type { IAbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; +import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import { RemoteFeatureFlagController, controllerName, @@ -55,7 +55,7 @@ function createController( options: Partial<{ messenger: RemoteFeatureFlagControllerMessenger; state: Partial; - clientConfigApiService: IAbstractClientConfigApiService; + clientConfigApiService: AbstractClientConfigApiService; disabled: boolean; getMetaMetricsId: () => string; }> = {}, @@ -218,7 +218,7 @@ describe('RemoteFeatureFlagController', () => { const clientConfigApiService = { fetchRemoteFeatureFlags: fetchSpy, - } as IAbstractClientConfigApiService; + } as AbstractClientConfigApiService; const controller = createController({ clientConfigApiService, @@ -387,7 +387,7 @@ function buildClientConfigApiService({ remoteFeatureFlags?: FeatureFlags; cacheTimestamp?: number; error?: Error; -} = {}): IAbstractClientConfigApiService { +} = {}): AbstractClientConfigApiService { return { fetchRemoteFeatureFlags: jest.fn().mockImplementation(() => { if (error) { diff --git a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts index 2664ff1ad90..da206fc1219 100644 --- a/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts +++ b/packages/remote-feature-flag-controller/src/remote-feature-flag-controller.ts @@ -5,7 +5,7 @@ import type { } from '@metamask/base-controller'; import { BaseController } from '@metamask/base-controller'; -import type { IAbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; +import type { AbstractClientConfigApiService } from './client-config-api-service/abstract-client-config-api-service'; import type { FeatureFlags, ServiceResponse, @@ -98,7 +98,7 @@ export class RemoteFeatureFlagController extends BaseController< #disabled: boolean; - readonly #clientConfigApiService: IAbstractClientConfigApiService; + readonly #clientConfigApiService: AbstractClientConfigApiService; #inProgressFlagUpdate?: Promise; @@ -125,7 +125,7 @@ export class RemoteFeatureFlagController extends BaseController< }: { messenger: RemoteFeatureFlagControllerMessenger; state?: Partial; - clientConfigApiService: IAbstractClientConfigApiService; + clientConfigApiService: AbstractClientConfigApiService; getMetaMetricsId: () => string; fetchInterval?: number; disabled?: boolean;