From c2af5518153510fdedaffc9d1dfa2a9d5e061336 Mon Sep 17 00:00:00 2001 From: Nicholas Griffin Date: Sun, 29 Sep 2024 16:05:01 +0100 Subject: [PATCH 1/4] feat: adding exponential backoff --- package.json | 2 +- src/lib/cloudflare.ts | 51 ++++++++++++-- test/unit/lib/cloudflare.test.ts | 110 +++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 8 deletions(-) create mode 100644 test/unit/lib/cloudflare.test.ts diff --git a/package.json b/package.json index 0aeef46..e4a663b 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "lint:fix": "eslint . --fix", "format": "prettier --log-level warn --write \"**/*.{js,json,jsx,md,ts,tsx,html}\"", "format:check": "prettier --check \"**/*.{js,json,jsx,md,ts,tsx,html}\"", - "test:unit": "node --import tsx --test ./test/unit/*.test.ts", + "test:unit": "node --import tsx --test ./test/unit/*.test.ts --test ./test/unit/**/*.test.ts", "test:unit:watch": "node --import tsx --test --watch ./test/unit/*.test.ts", "test": "pnpm run test:unit && pnpm run lint && pnpm run format:check", "lcov": "node --import tsx --test --experimental-test-coverage --test-reporter=lcov --test-reporter-destination=coverage/lcov.info ./test/unit/*.test.ts", diff --git a/src/lib/cloudflare.ts b/src/lib/cloudflare.ts index 73a317e..712eadf 100644 --- a/src/lib/cloudflare.ts +++ b/src/lib/cloudflare.ts @@ -2,6 +2,7 @@ import { ProviderError } from "../errors.js"; import { throwErrorIfResponseNotOk } from "./fetch.js"; const CLOUDFLARE_HOST = "https://api.cloudflare.com/client/v4"; +const MAX_RETRIES = 5; export function getCredentials() { const QUEUES_API_TOKEN = process.env.QUEUES_API_TOKEN; @@ -17,6 +18,27 @@ export function getCredentials() { }; } +async function exponentialBackoff( + fn: () => Promise, + maxRetries: number, +): Promise { + let attempt = 0; + while (attempt < maxRetries) { + try { + return await fn(); + } catch (error) { + if (error instanceof ProviderError && error.message.includes("429")) { + attempt++; + const delay = Math.pow(2, attempt) * 100 + Math.random() * 100; + await new Promise((resolve) => setTimeout(resolve, delay)); + } else { + throw error; + } + } + } + throw new ProviderError("Max retries reached"); +} + export async function queuesClient({ path, method, @@ -24,6 +46,13 @@ export async function queuesClient({ accountId, queueId, signal, +}: { + path: string; + method: string; + body?: Record; + accountId: string; + queueId: string; + signal?: AbortSignal; }): Promise { const { QUEUES_API_TOKEN } = getCredentials(); @@ -38,15 +67,23 @@ export async function queuesClient({ signal, }; - const response = await fetch(url, options); + async function fetchWithBackoff() { + const response = await fetch(url, options); - if (!response) { - throw new ProviderError("No response from Cloudflare Queues API"); - } + if (!response) { + throw new ProviderError("No response from Cloudflare Queues API"); + } - throwErrorIfResponseNotOk(response); + if (response.status === 429) { + throw new ProviderError("429 Too Many Requests"); + } - const data = (await response.json()) as T; + throwErrorIfResponseNotOk(response); + + const data = (await response.json()) as T; + + return data; + } - return data; + return exponentialBackoff(fetchWithBackoff, MAX_RETRIES); } diff --git a/test/unit/lib/cloudflare.test.ts b/test/unit/lib/cloudflare.test.ts new file mode 100644 index 0000000..a4f705f --- /dev/null +++ b/test/unit/lib/cloudflare.test.ts @@ -0,0 +1,110 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import { assert } from "chai"; +import nock from "nock"; +import sinon from "sinon"; + +import { queuesClient } from "../../../src/lib/cloudflare"; +import { ProviderError } from "../../../src/errors"; + +const CLOUDFLARE_HOST = "https://api.cloudflare.com/client/v4"; +const ACCOUNT_ID = "test-account-id"; +const QUEUE_ID = "test-queue-id"; +const QUEUES_API_TOKEN = "test-queues-api-token"; + +describe("queuesClient", () => { + let sandbox: sinon.SinonSandbox; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + process.env.QUEUES_API_TOKEN = QUEUES_API_TOKEN; + }); + + afterEach(() => { + sandbox.restore(); + delete process.env.QUEUES_API_TOKEN; + nock.cleanAll(); + }); + + it("should successfully fetch data from Cloudflare Queues API", async () => { + const path = "messages"; + const method = "GET"; + const responseBody = { success: true, result: [] }; + + nock(CLOUDFLARE_HOST) + .get(`/accounts/${ACCOUNT_ID}/queues/${QUEUE_ID}/${path}`) + .reply(200, responseBody); + + const result = await queuesClient({ + path, + method, + accountId: ACCOUNT_ID, + queueId: QUEUE_ID, + }); + + assert.deepEqual(result, responseBody); + }); + + it("should throw an error if the API token is missing", async () => { + delete process.env.QUEUES_API_TOKEN; + + try { + await queuesClient({ + path: "messages", + method: "GET", + accountId: ACCOUNT_ID, + queueId: QUEUE_ID, + }); + assert.fail("Expected error to be thrown"); + } catch (error) { + assert.instanceOf(error, Error); + assert.equal( + error.message, + "Missing Cloudflare credentials, please set a QUEUES_API_TOKEN in the environment variables.", + ); + } + }); + + it("should retry on 429 Too Many Requests", async () => { + const path = "messages"; + const method = "GET"; + const responseBody = { success: true, result: [] }; + + nock(CLOUDFLARE_HOST) + .get(`/accounts/${ACCOUNT_ID}/queues/${QUEUE_ID}/${path}`) + .reply(429, "Too Many Requests") + .get(`/accounts/${ACCOUNT_ID}/queues/${QUEUE_ID}/${path}`) + .reply(200, responseBody); + + const result = await queuesClient({ + path, + method, + accountId: ACCOUNT_ID, + queueId: QUEUE_ID, + }); + + assert.deepEqual(result, responseBody); + }); + + it("should throw ProviderError after max retries", async () => { + const path = "messages"; + const method = "GET"; + + nock(CLOUDFLARE_HOST) + .get(`/accounts/${ACCOUNT_ID}/queues/${QUEUE_ID}/${path}`) + .times(5) + .reply(429, "Too Many Requests"); + + try { + await queuesClient({ + path, + method, + accountId: ACCOUNT_ID, + queueId: QUEUE_ID, + }); + assert.fail("Expected error to be thrown"); + } catch (error) { + assert.instanceOf(error, ProviderError); + assert.equal(error.message, "Max retries reached"); + } + }); +}); From db8643028a01a4dbd964f46658d249135b7c7724 Mon Sep 17 00:00:00 2001 From: Nicholas Griffin Date: Sun, 29 Sep 2024 16:08:42 +0100 Subject: [PATCH 2/4] chore: reduce retry --- src/lib/cloudflare.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/lib/cloudflare.ts b/src/lib/cloudflare.ts index 712eadf..1fe12f0 100644 --- a/src/lib/cloudflare.ts +++ b/src/lib/cloudflare.ts @@ -18,6 +18,22 @@ export function getCredentials() { }; } +function calculateDelay(attempt: number): number { + return Math.pow(2, attempt) * 100 + Math.random() * 100; +} + +function shouldRetry( + error: unknown, + attempt: number, + maxRetries: number, +): boolean { + return ( + error instanceof ProviderError && + error.message.includes("429") && + attempt < maxRetries + ); +} + async function exponentialBackoff( fn: () => Promise, maxRetries: number, @@ -27,9 +43,9 @@ async function exponentialBackoff( try { return await fn(); } catch (error) { - if (error instanceof ProviderError && error.message.includes("429")) { + if (shouldRetry(error, attempt, maxRetries)) { attempt++; - const delay = Math.pow(2, attempt) * 100 + Math.random() * 100; + const delay = calculateDelay(attempt); await new Promise((resolve) => setTimeout(resolve, delay)); } else { throw error; From 39a18a77dab21abb73d9335081b032713af09445 Mon Sep 17 00:00:00 2001 From: Nicholas Griffin Date: Sun, 29 Sep 2024 16:11:44 +0100 Subject: [PATCH 3/4] chore: change node versions tested --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cfe9419..61210f5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [18.x, 20.x] + node-version: [20.x, 22.x] steps: - uses: actions/checkout@v4 From a8790041a13d8f4207503eb571a586cbfecbccea Mon Sep 17 00:00:00 2001 From: Nicholas Griffin Date: Sun, 29 Sep 2024 16:12:58 +0100 Subject: [PATCH 4/4] fix: just change to 22 --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 61210f5..36767eb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,7 +14,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [20.x, 22.x] + node-version: [22.x] steps: - uses: actions/checkout@v4