From 987ec772d48e0d92f62954968ea00f4a29d1fec1 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Tue, 7 Jan 2025 13:38:08 -0700 Subject: [PATCH] Refactor service classes to use createServicePolicy A previous commit added a `createServicePolicy` function to `controller-utils`. This allows us to refactor the following service classes which make use of a Cockatiel policy by replacing boilerplace code with a simple function call: - `CodefiV2` in `assets-controllers` - `ClientConfigApiService` in `remote-feature-flag-controller` Note that other service modules that do not yet make use of a Cockatiel policy have not been updated to use `createServicePolicy`. We would need to upgrade these areas to use the latest service object pattern we've established. --- packages/assets-controllers/package.json | 1 - .../token-prices-service/codefi-v2.test.ts | 996 +----------------- .../src/token-prices-service/codefi-v2.ts | 67 +- .../src/create-service-policy.ts | 2 + packages/controller-utils/src/index.ts | 8 +- .../package.json | 2 +- .../client-config-api-service.test.ts | 93 -- .../client-config-api-service.ts | 61 +- yarn.lock | 2 - 9 files changed, 80 insertions(+), 1152 deletions(-) diff --git a/packages/assets-controllers/package.json b/packages/assets-controllers/package.json index 2b16bdafd3..7ffffaee92 100644 --- a/packages/assets-controllers/package.json +++ b/packages/assets-controllers/package.json @@ -66,7 +66,6 @@ "@types/uuid": "^8.3.0", "async-mutex": "^0.5.0", "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/token-prices-service/codefi-v2.test.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts index e1efe858ec..046877a8f5 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 @@ -829,726 +829,9 @@ describe('CodefiTokenPricesServiceV2', () => { clock.restore(); }); - it('does not call onDegraded when requests succeeds faster than threshold', async () => { - const degradedThreshold = 1000; - 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, - 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, - }); - - await fetchTokenPricesWithFakeTimers({ - clock, - fetchTokenPrices: () => - service.fetchTokenPrices({ - chainId: '0x1', - tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], - currency: 'ETH', - }), - retries, - }); - - expect(onDegradedHandler).toHaveBeenCalledTimes(1); - }); - }); - - describe('after circuit break', () => { - let clock: sinon.SinonFakeTimers; - - beforeEach(() => { - clock = useFakeTimers({ now: Date.now() }); - }); - - afterEach(() => { - clock.restore(); - }); - - it('stops making fetch requests after too many consecutive failures', 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 - const successfullCallScope = 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, - // 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, - }); - 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'); - } - - 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 + 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({ @@ -1557,170 +840,70 @@ describe('CodefiTokenPricesServiceV2', () => { 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', - }) + .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, 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, + + await fetchTokenPricesWithFakeTimers({ + clock, + fetchTokenPrices: () => + service.fetchTokenPrices({ + chainId: '0x1', + tokenAddresses: ['0xAAA', '0xBBB', '0xCCC'], + currency: 'ETH', }), - ).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); + retries, + }); + + expect(onDegradedHandler).toHaveBeenCalledTimes(1); + }); + }); - // 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', - ); + describe('after circuit break', () => { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = useFakeTimers({ now: Date.now() }); + }); - // 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); + afterEach(() => { + clock.restore(); }); - it('recovers after circuit break', 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; - // 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') @@ -1732,7 +915,7 @@ describe('CodefiTokenPricesServiceV2', () => { }) .times(maximumConsecutiveFailures) .replyWithError('Failed to fetch'); - // Later interceptor for successfull request after recovery + // This interceptor should not be used nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ @@ -1827,10 +1010,14 @@ describe('CodefiTokenPricesServiceV2', () => { pricePercentChange1y: -2.2992517267242754, }, }); + const onBreakHandler = jest.fn(); const service = new CodefiTokenPricesServiceV2({ retries, maximumConsecutiveFailures, - circuitBreakDuration, + // 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({ @@ -1838,6 +1025,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()) { @@ -1850,115 +1039,8 @@ describe('CodefiTokenPricesServiceV2', () => { }), ).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, - }, - }); + expect(onBreakHandler).toHaveBeenCalledTimes(1); }); }); }); 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 4f163203e4..c75bd012ea 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -1,16 +1,13 @@ -import { handleFetch } from '@metamask/controller-utils'; +import { + createServicePolicy, + DEFAULT_DEGRADED_THRESHOLD, + DEFAULT_MAX_CONSECUTIVE_FAILURES, + DEFAULT_MAX_RETRIES, + handleFetch, +} from '@metamask/controller-utils'; +import type { IServicePolicy } 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, @@ -271,13 +268,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. */ @@ -365,7 +355,7 @@ export class CodefiTokenPricesServiceV2 implements AbstractTokenPricesService { - #tokenPricePolicy: IPolicy; + #tokenPricePolicy: IServicePolicy; /** * Construct a Codefi Token Price Service. @@ -385,8 +375,8 @@ export class CodefiTokenPricesServiceV2 */ 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, @@ -398,35 +388,14 @@ 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(), + this.#tokenPricePolicy = createServicePolicy({ + maxRetries: retries, + maxConsecutiveFailures: maximumConsecutiveFailures, + circuitBreakDuration, + degradedThreshold, + onBreak, + onDegraded, }); - const circuitBreakerPolicy = circuitBreaker(handleAll, { - halfOpenAfter: circuitBreakDuration, - breaker: new ConsecutiveBreaker(maximumConsecutiveFailures), - }); - if (onBreak) { - circuitBreakerPolicy.onBreak(onBreak); - } - if (onDegraded) { - retryPolicy.onGiveUp(() => { - if (circuitBreakerPolicy.state === CircuitState.Closed) { - onDegraded(); - } - }); - retryPolicy.onSuccess(({ duration }) => { - if ( - circuitBreakerPolicy.state === CircuitState.Closed && - duration > degradedThreshold - ) { - onDegraded(); - } - }); - } - this.#tokenPricePolicy = wrap(retryPolicy, circuitBreakerPolicy); } /** diff --git a/packages/controller-utils/src/create-service-policy.ts b/packages/controller-utils/src/create-service-policy.ts index 210516d124..9dd3daa0ba 100644 --- a/packages/controller-utils/src/create-service-policy.ts +++ b/packages/controller-utils/src/create-service-policy.ts @@ -9,6 +9,8 @@ import { } from 'cockatiel'; import type { IPolicy } from 'cockatiel'; +export type { IPolicy as IServicePolicy }; + /** * The maximum number of times that a failing action should be re-run before * giving up. diff --git a/packages/controller-utils/src/index.ts b/packages/controller-utils/src/index.ts index b3bd8821e1..a8f8ac3cfb 100644 --- a/packages/controller-utils/src/index.ts +++ b/packages/controller-utils/src/index.ts @@ -1,4 +1,10 @@ -export { createServicePolicy } from './create-service-policy'; +export { + DEFAULT_DEGRADED_THRESHOLD, + DEFAULT_MAX_CONSECUTIVE_FAILURES, + DEFAULT_MAX_RETRIES, + createServicePolicy, +} from './create-service-policy'; +export type { IServicePolicy } from './create-service-policy'; export * from './constants'; export type { NonEmptyArray } from './util'; export { diff --git a/packages/remote-feature-flag-controller/package.json b/packages/remote-feature-flag-controller/package.json index efbb16bbd6..89162b4f06 100644 --- a/packages/remote-feature-flag-controller/package.json +++ b/packages/remote-feature-flag-controller/package.json @@ -48,8 +48,8 @@ }, "dependencies": { "@metamask/base-controller": "^7.1.0", + "@metamask/controller-utils": "^11.4.4", "@metamask/utils": "^11.0.1", - "cockatiel": "^3.1.2", "uuid": "^8.3.2" }, "devDependencies": { 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 1baaaf72bf..774e3b5726 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 @@ -132,41 +132,6 @@ 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 () => { const onBreak = jest.fn(); @@ -221,64 +186,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 7cd9177e0b..f39d78c9a4 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,13 +1,9 @@ import { - circuitBreaker, - ConsecutiveBreaker, - ExponentialBackoff, - handleAll, - type IPolicy, - retry, - wrap, - CircuitState, -} from 'cockatiel'; + createServicePolicy, + DEFAULT_MAX_CONSECUTIVE_FAILURES, + DEFAULT_MAX_RETRIES, +} from '@metamask/controller-utils'; +import type { IServicePolicy } from '@metamask/controller-utils'; import { BASE_URL } from '../constants'; import type { @@ -19,19 +15,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 { #fetch: typeof fetch; - #policy: IPolicy; + #policy: IServicePolicy; #client: ClientType; @@ -58,7 +48,7 @@ export class ClientConfigApiService { */ constructor({ fetch: fetchFunction, - retries = DEFAULT_FETCH_RETRIES, + retries = DEFAULT_MAX_RETRIES, maximumConsecutiveFailures = DEFAULT_MAX_CONSECUTIVE_FAILURES, circuitBreakDuration = 30 * 60 * 1000, onBreak, @@ -82,38 +72,13 @@ export class ClientConfigApiService { this.#distribution = config.distribution; this.#environment = config.environment; - const retryPolicy = retry(handleAll, { - maxAttempts: retries, - backoff: new ExponentialBackoff(), + this.#policy = createServicePolicy({ + maxRetries: retries, + maxConsecutiveFailures: maximumConsecutiveFailures, + circuitBreakDuration, + onBreak, + onDegraded, }); - - const circuitBreakerPolicy = circuitBreaker(handleAll, { - halfOpenAfter: circuitBreakDuration, - breaker: new ConsecutiveBreaker(maximumConsecutiveFailures), - }); - - if (onBreak) { - circuitBreakerPolicy.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 = wrap(retryPolicy, circuitBreakerPolicy); } /** diff --git a/yarn.lock b/yarn.lock index 44af54cfa4..96ddf40964 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2338,7 +2338,6 @@ __metadata: "@types/uuid": "npm:^8.3.0" async-mutex: "npm:^0.5.0" 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" @@ -3678,7 +3677,6 @@ __metadata: "@metamask/controller-utils": "npm:^11.4.4" "@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"