From 4072d0cd9d1ac773b1cf38280f180bd0b507f6c3 Mon Sep 17 00:00:00 2001
From: Max Kurapov <max@interledger.org>
Date: Sun, 25 Aug 2024 16:50:54 +0200
Subject: [PATCH] refactor(backend): update rate caching

---
 packages/backend/src/rates/service.test.ts | 29 +++++-----
 packages/backend/src/rates/service.ts      | 61 +++++++++++-----------
 2 files changed, 48 insertions(+), 42 deletions(-)

diff --git a/packages/backend/src/rates/service.test.ts b/packages/backend/src/rates/service.test.ts
index 5778a54fc6..8145a3cd56 100644
--- a/packages/backend/src/rates/service.test.ts
+++ b/packages/backend/src/rates/service.test.ts
@@ -6,6 +6,7 @@ import { initIocContainer } from '../'
 import { AppServices } from '../app'
 import { CacheDataStore } from '../middleware/cache/data-stores'
 import { mockRatesApi } from '../tests/rates'
+import { AxiosInstance } from 'axios'
 
 const nock = (global as unknown as { nock: typeof import('nock') }).nock
 
@@ -214,23 +215,27 @@ describe('Rates service', function () {
       expect(apiRequestCount).toBe(2)
     })
 
-    it('prefetches when the cached request is old', async () => {
+    it('returns new rates after cache expires', async () => {
       await expect(service.rates('USD')).resolves.toEqual(usdRates)
-      jest.advanceTimersByTime(exchangeRatesLifetime * 0.5 + 1)
-      // ... cache isn't expired yet, but it will be soon
-      await expect(service.rates('USD')).resolves.toEqual(usdRates)
-      expect(apiRequestCount).toBe(1)
-
-      // Invalidate the cache.
-      jest.advanceTimersByTime(exchangeRatesLifetime * 0.5 + 1)
+      jest.advanceTimersByTime(exchangeRatesLifetime + 1)
       await expect(service.rates('USD')).resolves.toEqual(usdRates)
-      // The prefetch response is promoted to the cache.
       expect(apiRequestCount).toBe(2)
     })
 
-    it('cannot use an expired cache', async () => {
-      await expect(service.rates('USD')).resolves.toEqual(usdRates)
-      jest.advanceTimersByTime(exchangeRatesLifetime + 1)
+    it('returns rates on second request (first one was error)', async () => {
+      jest
+        .spyOn(
+          (service as RatesService & { axios: AxiosInstance }).axios,
+          'get'
+        )
+        .mockImplementationOnce(() => {
+          apiRequestCount++
+          throw new Error()
+        })
+
+      await expect(service.rates('USD')).rejects.toThrow(
+        'Could not fetch rates'
+      )
       await expect(service.rates('USD')).resolves.toEqual(usdRates)
       expect(apiRequestCount).toBe(2)
     })
diff --git a/packages/backend/src/rates/service.ts b/packages/backend/src/rates/service.ts
index 5a6ed7322e..f4cae1f819 100644
--- a/packages/backend/src/rates/service.ts
+++ b/packages/backend/src/rates/service.ts
@@ -1,5 +1,5 @@
 import { BaseService } from '../shared/baseService'
-import Axios, { AxiosInstance } from 'axios'
+import Axios, { AxiosInstance, isAxiosError } from 'axios'
 import { convert, ConvertOptions } from './util'
 import { createInMemoryDataStore } from '../middleware/cache/data-stores/in-memory'
 import { CacheDataStore } from '../middleware/cache/data-stores'
@@ -74,29 +74,13 @@ class RatesServiceImpl implements RatesService {
   }
 
   private async getRates(baseAssetCode: string): Promise<Rates> {
-    const [cachedRate, cachedExpiry] = await Promise.all([
-      this.cachedRates.get(baseAssetCode),
-      this.cachedRates.getKeyExpiry(baseAssetCode)
-    ])
-
-    if (cachedRate && cachedExpiry) {
-      const isCloseToExpiry =
-        cachedExpiry.getTime() <
-        Date.now() + 0.5 * this.deps.exchangeRatesLifetime
-
-      if (isCloseToExpiry) {
-        this.fetchNewRatesAndCache(baseAssetCode) // don't await, just get new rates for later
-      }
+    const cachedRate = await this.cachedRates.get(baseAssetCode)
 
+    if (cachedRate) {
       return JSON.parse(cachedRate)
     }
 
-    try {
-      return await this.fetchNewRatesAndCache(baseAssetCode)
-    } catch (err) {
-      this.cachedRates.delete(baseAssetCode)
-      throw err
-    }
+    return await this.fetchNewRatesAndCache(baseAssetCode)
   }
 
   private async fetchNewRatesAndCache(baseAssetCode: string): Promise<Rates> {
@@ -106,12 +90,32 @@ class RatesServiceImpl implements RatesService {
       this.inProgressRequests[baseAssetCode] = this.fetchNewRates(baseAssetCode)
     }
 
-    const rates = await this.inProgressRequests[baseAssetCode]
+    try {
+      const rates = await this.inProgressRequests[baseAssetCode]
 
-    delete this.inProgressRequests[baseAssetCode]
+      await this.cachedRates.set(baseAssetCode, JSON.stringify(rates))
+      return rates
+    } catch (err) {
+      const errorMessage = 'Could not fetch rates'
+
+      this.deps.logger.error(
+        {
+          ...(isAxiosError(err)
+            ? {
+                errorMessage: err.message,
+                errorCode: err.code,
+                errorStatus: err.status
+              }
+            : { err }),
+          url: this.deps.exchangeRatesUrl
+        },
+        errorMessage
+      )
 
-    await this.cachedRates.set(baseAssetCode, JSON.stringify(rates))
-    return rates
+      throw new Error(errorMessage)
+    } finally {
+      delete this.inProgressRequests[baseAssetCode]
+    }
   }
 
   private async fetchNewRates(baseAssetCode: string): Promise<Rates> {
@@ -120,12 +124,9 @@ class RatesServiceImpl implements RatesService {
       return { base: baseAssetCode, rates: {} }
     }
 
-    const res = await this.axios
-      .get<Rates>(url, { params: { base: baseAssetCode } })
-      .catch((err) => {
-        this.deps.logger.warn({ err: err.message }, 'price request error')
-        throw err
-      })
+    const res = await this.axios.get<Rates>(url, {
+      params: { base: baseAssetCode }
+    })
 
     const { base, rates } = res.data
     this.checkBaseAsset(base)