From 716fafd99df91cda3f970caf863f228eb50f10aa Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 26 Sep 2024 14:27:24 +0200 Subject: [PATCH 1/6] test: Ensure FS tests run on iOS --- packages/cache/src/__tests__/constants.spec.ts | 5 ++++- packages/rest/src/__tests__/client.spec.ts | 15 ++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/cache/src/__tests__/constants.spec.ts b/packages/cache/src/__tests__/constants.spec.ts index 26bb231..bc3373d 100644 --- a/packages/cache/src/__tests__/constants.spec.ts +++ b/packages/cache/src/__tests__/constants.spec.ts @@ -1,3 +1,6 @@ +import os from 'node:os'; +import path from 'node:path'; + import * as constants from '../constants'; describe('@figmarine/cache - constants', () => { @@ -6,7 +9,7 @@ describe('@figmarine/cache - constants', () => { expect(constants).toHaveProperty('DEFAULT_CACHE_PATH'); }); it('scopes cache content with the package name', () => { - expect(constants.DEFAULT_CACHE_PATH.startsWith('/tmp/@figmarine')).toBeTruthy(); + expect(constants.DEFAULT_CACHE_PATH.startsWith(path.join(os.tmpdir()))).toBeTruthy(); }); }); describe('YEAR_IN_SECONDS', () => { diff --git a/packages/rest/src/__tests__/client.spec.ts b/packages/rest/src/__tests__/client.spec.ts index 08e5e64..77ee1f8 100644 --- a/packages/rest/src/__tests__/client.spec.ts +++ b/packages/rest/src/__tests__/client.spec.ts @@ -1,3 +1,6 @@ +import os from 'node:os'; +import path from 'node:path'; + import * as cacheModule from '@figmarine/cache'; import * as loggerModule from '@figmarine/logger'; import { test as base } from 'vitest'; @@ -35,12 +38,14 @@ const it = base.extend({ }, }); +const tmpDir = os.tmpdir(); + // TODO mock oas serv and write extra tests describe('@figmarine/rest - client', () => { beforeEach(() => { vol.reset(); - vol.fromJSON({ '/tmp': null }); + vol.fromJSON({ [tmpDir]: null }); }); afterEach(() => { vi.restoreAllMocks(); @@ -54,7 +59,7 @@ describe('@figmarine/rest - client', () => { await Client(); - const folder = vol.lstatSync('/tmp/@figmarine/cache'); + const folder = vol.lstatSync(path.join(tmpDir, '@figmarine/cache')); expect(folder.isDirectory).toBeTruthy(); }); @@ -63,7 +68,7 @@ describe('@figmarine/rest - client', () => { mockedEnv(); await Client(); - const hasFolder = vol.existsSync('/tmp/@figmarine/cache'); + const hasFolder = vol.existsSync(path.join(tmpDir, '@figmarine/cache')); expect(hasFolder).toBeFalsy(); }); @@ -75,7 +80,7 @@ describe('@figmarine/rest - client', () => { await Client({ cache: false }); - const hasFolder = vol.existsSync('/tmp/@figmarine/cache'); + const hasFolder = vol.existsSync(path.join(tmpDir, '@figmarine/cache')); expect(hasFolder).toBeFalsy(); }); @@ -97,7 +102,7 @@ describe('@figmarine/rest - client', () => { const hasFolderRelativeToCwd = vol.existsSync(location); const hasFolderRelativeToDefaultLocation = vol.existsSync( - `/tmp/@figmarine/cache/${location}`, + path.join(tmpDir, `@figmarine/cache/${location}`), ); expect(hasFolderRelativeToCwd).toBeFalsy(); From b8f1efcc00da4af44896f448f6214226e7caa5ba Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 26 Sep 2024 14:36:56 +0200 Subject: [PATCH 2/6] chore(config-vitest): Split unit and integ and add TS decls --- packages/config-vitest/index.js | 23 ----------------------- packages/config-vitest/integration.js | 13 +++++++++++++ packages/config-vitest/package.json | 20 +++++++++++++++++++- packages/config-vitest/shared.js | 16 ++++++++++++++++ packages/config-vitest/tsconfig.json | 11 +++++++++++ packages/config-vitest/unit.js | 10 ++++++++++ 6 files changed, 69 insertions(+), 24 deletions(-) delete mode 100644 packages/config-vitest/index.js create mode 100644 packages/config-vitest/integration.js create mode 100644 packages/config-vitest/shared.js create mode 100644 packages/config-vitest/tsconfig.json create mode 100644 packages/config-vitest/unit.js diff --git a/packages/config-vitest/index.js b/packages/config-vitest/index.js deleted file mode 100644 index 13f1959..0000000 --- a/packages/config-vitest/index.js +++ /dev/null @@ -1,23 +0,0 @@ -import { defineConfig } from 'vitest/config'; - -export default defineConfig({ - test: { - globals: true, - coverage: { - include: [ - '**/*.{mjs,mjsx,js,jsx,ts,tsx}', - '!*.config.{js,ts,cjs}', - '!**/coverage/**', - '!**/scripts/**', - '!**/__fixtures__/**', - '!**/__mocks__/**', - '!**/__tests__/**', - '!**/__generated__/**', - '!**/node_modules/**', - '!**/dist/**', - '!**/src/debug.ts', - ], - provider: 'istanbul', - }, - }, -}); diff --git a/packages/config-vitest/integration.js b/packages/config-vitest/integration.js new file mode 100644 index 0000000..782dbe6 --- /dev/null +++ b/packages/config-vitest/integration.js @@ -0,0 +1,13 @@ +import { defineConfig } from 'vitest/config'; + +import { coverage } from './shared.js'; + +export default defineConfig({ + test: { + globals: true, + threads: false, + setupFiles: ['./src/__tests__/setupIntegration.ts'], + include: ['**/__tests__/**/*.integration.ts',], + coverage, + }, +}); diff --git a/packages/config-vitest/package.json b/packages/config-vitest/package.json index eb47be7..e439873 100644 --- a/packages/config-vitest/package.json +++ b/packages/config-vitest/package.json @@ -10,6 +10,9 @@ "url": "https://github.com/Sidnioulz/figmarine", "directory": "packages/config-vitest" }, + "scripts": { + "build": "tsc" + }, "bugs": { "url": "https://github.com/Sidnioulz/figmarine/issues" }, @@ -20,9 +23,24 @@ "author": "Steve Dodier-Lazaro", "homepage": "https://github.com/Sidnioulz/figmarine", "files": [ - "index.js" + "integration.js", + "unit.js" ], + "exports": { + ".": { + "types": "./dist/unit.d.ts", + "default": "./unit.js" + }, + "./*": { + "types": "./dist/*.d.ts", + "default": "./*.js" + } + }, "dependencies": { "vitest": "^2.0.5" + }, + "devDependencies": { + "typescript": "^5.6.2", + "vite": "^5.4.5" } } \ No newline at end of file diff --git a/packages/config-vitest/shared.js b/packages/config-vitest/shared.js new file mode 100644 index 0000000..29d6444 --- /dev/null +++ b/packages/config-vitest/shared.js @@ -0,0 +1,16 @@ +export const coverage = { + include: [ + '**/*.{mjs,mjsx,js,jsx,ts,tsx}', + '!*.config.{js,ts,cjs}', + '!**/coverage/**', + '!**/scripts/**', + '!**/__fixtures__/**', + '!**/__mocks__/**', + '!**/__tests__/**', + '!**/__generated__/**', + '!**/node_modules/**', + '!**/dist/**', + '!**/src/debug.ts', + ], + provider: 'istanbul', +}; diff --git a/packages/config-vitest/tsconfig.json b/packages/config-vitest/tsconfig.json new file mode 100644 index 0000000..99c53fb --- /dev/null +++ b/packages/config-vitest/tsconfig.json @@ -0,0 +1,11 @@ +{ + "compilerOptions": { + "outDir": "dist", + "allowJs": true, + "declaration": true, + "emitDeclarationOnly": true, + "skipLibCheck": true, + "types": ["vitest/globals"] + }, + "include": ["integration.js", "unit.js"], +} \ No newline at end of file diff --git a/packages/config-vitest/unit.js b/packages/config-vitest/unit.js new file mode 100644 index 0000000..2ec77fc --- /dev/null +++ b/packages/config-vitest/unit.js @@ -0,0 +1,10 @@ +import { defineConfig } from 'vitest/config'; + +import { coverage } from './shared.js'; + +export default defineConfig({ + test: { + globals: true, + coverage, + }, +}); From 685b181e95b31449c875efd8a1a71a29851afb6f Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 26 Sep 2024 14:37:44 +0200 Subject: [PATCH 3/6] chore(config-tsup): Add TS decls --- packages/config-tsup/package.json | 8 ++++++-- packages/config-tsup/tsconfig.json | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 packages/config-tsup/tsconfig.json diff --git a/packages/config-tsup/package.json b/packages/config-tsup/package.json index 1c91726..7fb7964 100644 --- a/packages/config-tsup/package.json +++ b/packages/config-tsup/package.json @@ -5,6 +5,9 @@ "license": "MIT", "type": "module", "description": "tsup config package for the Figmarine monorepo", + "scripts": { + "build": "tsc" + }, "repository": { "type": "git", "url": "https://github.com/Sidnioulz/figmarine", @@ -24,11 +27,12 @@ ], "exports": { ".": { - "types": "./index.d.ts", + "types": "./dist/index.d.ts", "default": "./index.js" } }, "devDependencies": { - "tsup": "^8.2.4" + "tsup": "^8.2.4", + "typescript": "^5.6.2" } } \ No newline at end of file diff --git a/packages/config-tsup/tsconfig.json b/packages/config-tsup/tsconfig.json new file mode 100644 index 0000000..e23eef5 --- /dev/null +++ b/packages/config-tsup/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "outDir": "dist", + "allowJs": true, + "declaration": true, + "emitDeclarationOnly": true, + "skipLibCheck": true + }, + "include": ["index.js"] +} \ No newline at end of file From 5a5b80e5e55e8d8ee2ca50fb2f0ba715c7530b9f Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 26 Sep 2024 15:09:17 +0200 Subject: [PATCH 4/6] chore(config-eslint): Add TS decls --- packages/config-eslint/package.json | 13 +++++++++++-- packages/config-eslint/tsconfig.json | 10 ++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 packages/config-eslint/tsconfig.json diff --git a/packages/config-eslint/package.json b/packages/config-eslint/package.json index 637d24c..c23eeee 100644 --- a/packages/config-eslint/package.json +++ b/packages/config-eslint/package.json @@ -10,6 +10,9 @@ "url": "https://github.com/Sidnioulz/figmarine", "directory": "packages/config-eslint" }, + "scripts": { + "build": "tsc" + }, "bugs": { "url": "https://github.com/Sidnioulz/figmarine/issues" }, @@ -23,8 +26,14 @@ "*.js" ], "exports": { - ".": "./index.js", - "./*": "./*.js" + ".": { + "types": "./dist/index.d.ts", + "default": "./index.js" + }, + "./*": { + "types": "./dist/*.d.ts", + "default": "./*.js" + } }, "dependencies": { "@eslint/json": "^0.4.0", diff --git a/packages/config-eslint/tsconfig.json b/packages/config-eslint/tsconfig.json new file mode 100644 index 0000000..e23eef5 --- /dev/null +++ b/packages/config-eslint/tsconfig.json @@ -0,0 +1,10 @@ +{ + "compilerOptions": { + "outDir": "dist", + "allowJs": true, + "declaration": true, + "emitDeclarationOnly": true, + "skipLibCheck": true + }, + "include": ["index.js"] +} \ No newline at end of file From f99d29478d0b4e17634935c478a660a41d0da8fa Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Thu, 26 Sep 2024 15:10:22 +0200 Subject: [PATCH 5/6] chore(vitest): Fix vitest workspace file name --- vitest.workspace => vitest.workspace.ts | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename vitest.workspace => vitest.workspace.ts (100%) diff --git a/vitest.workspace b/vitest.workspace.ts similarity index 100% rename from vitest.workspace rename to vitest.workspace.ts From e71710246d5ac78a3841ecaf6037c574a32a919a Mon Sep 17 00:00:00 2001 From: Steve Dodier-Lazaro Date: Sat, 28 Sep 2024 13:07:13 +0200 Subject: [PATCH 6/6] feat(rest): Add auto-retry on 429 --- packages/cache/package.json | 2 +- packages/cuttings/package.json | 2 +- packages/eslint-plugin-figma/package.json | 2 +- packages/logger/package.json | 2 +- packages/rest/package.json | 6 +- packages/rest/src/__tests__/client.spec.ts | 189 ++++++++++++++++++++- packages/rest/src/client.ts | 34 +++- packages/rest/src/rateLimit.config.ts | 24 +-- packages/rest/src/rateLimit.ts | 9 - pnpm-lock.yaml | 55 ++++++ 10 files changed, 294 insertions(+), 31 deletions(-) diff --git a/packages/cache/package.json b/packages/cache/package.json index d74a95d..cbeba88 100644 --- a/packages/cache/package.json +++ b/packages/cache/package.json @@ -25,7 +25,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "test": "vitest run", - "test:coverage": "vitest --coverage", + "test:coverage": "vitest --coverage run", "test:dev": "vitest dev", "typecheck": "tsc --noEmit" }, diff --git a/packages/cuttings/package.json b/packages/cuttings/package.json index 4d77ad0..3a2af0e 100644 --- a/packages/cuttings/package.json +++ b/packages/cuttings/package.json @@ -29,7 +29,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "test": "vitest run", - "test:coverage": "vitest --coverage", + "test:coverage": "vitest --coverage run", "test:dev": "vitest dev", "typecheck": "tsc --noEmit" }, diff --git a/packages/eslint-plugin-figma/package.json b/packages/eslint-plugin-figma/package.json index 8328626..39e94b0 100644 --- a/packages/eslint-plugin-figma/package.json +++ b/packages/eslint-plugin-figma/package.json @@ -28,7 +28,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "test": "vitest run", - "test:coverage": "vitest --coverage", + "test:coverage": "vitest --coverage run", "test:dev": "vitest dev", "typecheck": "tsc --noEmit" }, diff --git a/packages/logger/package.json b/packages/logger/package.json index 085145c..9384507 100644 --- a/packages/logger/package.json +++ b/packages/logger/package.json @@ -22,7 +22,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "test": "vitest run", - "test:coverage": "vitest --coverage", + "test:coverage": "vitest --coverage run", "test:dev": "vitest dev", "typecheck": "tsc --noEmit" }, diff --git a/packages/rest/package.json b/packages/rest/package.json index 4d67977..6c43aaa 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -33,7 +33,7 @@ "lint": "eslint .", "lint:fix": "eslint . --fix", "test": "vitest run", - "test:coverage": "vitest --coverage", + "test:coverage": "vitest --coverage run", "test:dev": "vitest dev", "typecheck": "tsc --noEmit" }, @@ -63,6 +63,7 @@ "cacheable": "^0.8.0", "memfs": "^4.11.1", "mocked-env": "^1.3.5", + "nock": "^13.5.5", "swagger-typescript-api": "^13.0.22", "tsc-watch": "^6.2.0", "tsup": "^8.2.4", @@ -73,6 +74,7 @@ "@figmarine/cache": "workspace:*", "@figmarine/logger": "workspace:*", "axios": "^1.7.7", - "axios-cache-interceptor": "^1.6.0" + "axios-cache-interceptor": "^1.6.0", + "axios-retry": "^4.5.0" } } \ No newline at end of file diff --git a/packages/rest/src/__tests__/client.spec.ts b/packages/rest/src/__tests__/client.spec.ts index 77ee1f8..4d3451f 100644 --- a/packages/rest/src/__tests__/client.spec.ts +++ b/packages/rest/src/__tests__/client.spec.ts @@ -3,17 +3,40 @@ import path from 'node:path'; import * as cacheModule from '@figmarine/cache'; import * as loggerModule from '@figmarine/logger'; +import axios from 'axios'; import { test as base } from 'vitest'; +import nock from 'nock'; import upstreamMockedEnv from 'mocked-env'; import { vol } from 'memfs'; import * as interceptorsModule from '../interceptors'; -import { Client } from '../client'; +import { Client, ClientInterface } from '../client'; + +/* Mock rate limit config. */ +const { mockedConfig, mocked429Config } = vi.hoisted(() => { + return { + mockedConfig: vi.fn(), + mocked429Config: vi.fn(), + }; +}); + +vi.mock(import('../rateLimit.config'), async (importOriginal) => { + const mod = await importOriginal(); + + return { + ...mod, + getConfig: mockedConfig, + get429Config: mocked429Config, + }; +}); /* FS mocks. */ vi.mock('node:fs'); vi.mock('node:fs/promises'); +/* Mock the 1 minute initial delay for 429 retries. We use a shorter delay to avoid timeouts. */ +const INITIAL_DELAY = 10; + /* Local context. */ interface ClientFixtures { mockedEnv: (args?: typeof process.env) => void; @@ -40,12 +63,36 @@ const it = base.extend({ const tmpDir = os.tmpdir(); -// TODO mock oas serv and write extra tests +/* Nock helpers. */ +const callNockedEndpoint = async (c: ClientInterface) => { + try { + const response = await c.v1.getFile('testKey'); + return { + status: response.status, + headers: response.headers as Record, + data: response.data, + }; + } catch (error) { + if (axios.isAxiosError(error) && error.response) { + return { + status: error.response.status, + headers: error.response.headers as Record, + }; + } + throw error; + } +}; describe('@figmarine/rest - client', () => { beforeEach(() => { vol.reset(); vol.fromJSON({ [tmpDir]: null }); + mockedConfig.mockReturnValue({ + reqLog: [], + WINDOW_BUDGET: 10, + WINDOW_LENGTH: 60, + }); + mocked429Config.mockReturnValue({ INITIAL_DELAY: 500 }); }); afterEach(() => { vi.restoreAllMocks(); @@ -283,4 +330,142 @@ describe('@figmarine/rest - client', () => { expect(rlSpy).toHaveBeenCalled(); }); }); + + describe('Environment - FIGMA_API_BASE_URL', () => { + afterEach(() => { + nock.cleanAll(); + }); + + it('allows customising the default URL for integration testing', async ({ mockedEnv }) => { + mockedEnv({ FIGMA_API_BASE_URL: 'https://localhost:4010' }); + const c = await Client({ rateLimit: true }); + const originalScole = nock('https://api.figma.com').get('/v1/files/testKey').reply(200, 'OK'); + const modifiedUrlScope = nock('https://localhost:4010') + .get('/v1/files/testKey') + .reply(200, 'OK'); + + const response = await callNockedEndpoint(c); + + expect(response.status).toBe(200); + expect(originalScole.isDone()).toBe(false); + expect(originalScole.activeMocks()).toHaveLength(1); + expect(modifiedUrlScope.isDone()).toBe(true); + expect(modifiedUrlScope.activeMocks()).toHaveLength(0); + }); + }); + + describe('Auto Retry - 429', () => { + beforeEach(() => { + mocked429Config.mockReturnValue({ INITIAL_DELAY }); + }); + + afterEach(() => { + nock.cleanAll(); + }); + + it('retries 429 errors but not other errors', async ({ mockedEnv }) => { + mockedEnv(); + + const c = await Client({ rateLimit: true }); + const scope = nock('https://api.figma.com').get('/v1/files/testKey').reply(403, 'Forbidden'); + + const response = await callNockedEndpoint(c); + + expect(response.status).toBe(403); + expect(scope.isDone()).toBe(true); + expect(scope.activeMocks()).toHaveLength(0); + }); + + it.for([1, 2, 3, 4, 5])( + 'retries %i minute(s) after equally many 429 errors without Retry-After', + async (numberOf429, { mockedEnv }) => { + mockedEnv(); + + const requestTimes: number[] = []; + let requestCount = 0; + + const c = await Client({ rateLimit: true }); + const scope = nock('https://api.figma.com'); + for (let i = 0; i < numberOf429; i++) { + scope.get('/v1/files/testKey').reply(429, 'Too Many Requests'); + } + scope.get('/v1/files/testKey').reply(200, 'OK'); + + scope.on('request', () => { + requestTimes.push(Date.now()); + requestCount++; + }); + + const response = await callNockedEndpoint(c); + + expect(response.status).toBe(200); + expect(scope.isDone()).toBe(true); + expect(scope.activeMocks()).toHaveLength(0); + expect(requestCount).toBe(numberOf429 + 1); + + const totalDelay = requestTimes[requestTimes.length - 1] - requestTimes[0]; + const expectedMinDelay = INITIAL_DELAY * (Math.pow(2, numberOf429 + 1) - 2); + // We know that axios-retry returns between 1 and 1.2 times the computed delay. + // We take that delay and add a 100ms buffer for JS operations. + const expectedMaxDelay = 1.2 * expectedMinDelay + 100; + + expect(totalDelay).toBeGreaterThanOrEqual(expectedMinDelay); + expect(totalDelay).toBeLessThanOrEqual(expectedMaxDelay); + }, + ); + + it('fails on the 7th consecutive retry', async ({ mockedEnv }) => { + mockedEnv(); + let requestCount = 0; + const c = await Client({ rateLimit: true }); + + const scope = nock('https://api.figma.com') + .get('/v1/files/testKey') + .reply(429, 'Too Many Requests') + .persist(); + + scope.on('request', () => { + requestCount++; + }); + + const response = await callNockedEndpoint(c); + + expect(response.status).toBe(429); + expect(scope.isDone()).toBe(true); + expect(scope.activeMocks()).toHaveLength(1); + expect(requestCount).toBe(7); + }); + + it('respects the Retry-After header delay if higher than our computed delay', async ({ + mockedEnv, + }) => { + mockedEnv(); + const headerDelay = 200; + const startTime = Date.now(); + let endTime = 0; + + const c = await Client({ rateLimit: true }); + + const scope = nock('https://api.figma.com') + .get('/v1/files/testKey') + .reply(429, 'Too Many Requests', { 'Retry-After': `${headerDelay / 1000}` }) + .get('/v1/files/testKey') + .reply(200, 'OK'); + + scope.on('replied', (req) => { + if (req.response.statusCode === 200) { + endTime = Date.now(); + } + }); + + const response = await callNockedEndpoint(c); + + expect(response.status).toBe(200); + expect(scope.isDone()).toBe(true); + expect(scope.activeMocks()).toHaveLength(0); + + const totalDelay = endTime - startTime; + expect(totalDelay).toBeGreaterThanOrEqual(headerDelay); + }); + }); }); diff --git a/packages/rest/src/client.ts b/packages/rest/src/client.ts index 338c015..0383584 100644 --- a/packages/rest/src/client.ts +++ b/packages/rest/src/client.ts @@ -1,8 +1,10 @@ +import axiosRetry, { exponentialDelay, isNetworkOrIdempotentRequestError } from 'axios-retry'; import { defaultKeyGenerator, setupCache } from 'axios-cache-interceptor'; import { makeCache, MakeCacheOptions } from '@figmarine/cache'; import { log } from '@figmarine/logger'; import { Api, type Api as ApiInterface } from './__generated__/figmaRestApi'; +import { get429Config } from './rateLimit.config'; import { rateLimitRequestInterceptor } from './interceptors'; import { securityWorker } from './securityWorker'; import { storage } from './storage'; @@ -93,7 +95,7 @@ export async function Client(opts: ClientOptions = {}): Promise * Ignore "don't cache" headers from Figma by default. * Let the cache package handle TTL for us, and cache * invalidation logic clear stale cache. - * @returns {number} A 999999999999 seconds TTL. + * @returns A 999999999999 seconds TTL. */ headerInterpreter: () => 999999999999, /** @@ -105,6 +107,12 @@ export async function Client(opts: ClientOptions = {}): Promise }); } + /* Support changing the base URL so this package can be tested + * against a mock server. */ + if (process.env.FIGMA_API_BASE_URL) { + api.instance.defaults.baseURL = process.env.FIGMA_API_BASE_URL; + } + // TODO: add endpoints to download arbitrary image URLs returned by // other endpoints, which account for the need to deduce calls from // the rate limit budget. @@ -118,9 +126,29 @@ export async function Client(opts: ClientOptions = {}): Promise api.instance.interceptors.request.use( rateLimitRequestInterceptor(defaultKeyGenerator, diskCache), ); - } - // TODO: add response interceptor for 429 handling. + // Add response interceptor for 429 handling. + const rlConfig = get429Config(); + axiosRetry(api.instance, { + onMaxRetryTimesExceeded: (error) => { + log( + `Client: 429 Too Many Requests on '${error.request.url}'. Aborting after too many retries.`, + ); + }, + onRetry: (retryCount, _, requestConfig) => { + log( + `Client: 429 Too Many Requests on '${requestConfig.url}'. Will retry automatically in ${2 ** retryCount / 2} seconds.`, + ); + }, + retries: 6, + retryCondition: (error) => + isNetworkOrIdempotentRequestError(error) || + error.status === 429 || + error.response?.status === 429, + retryDelay: (retryCount, error) => + exponentialDelay(retryCount, error, rlConfig.INITIAL_DELAY), + }); + } log(`Created Figma REST API client successfully.`); diff --git a/packages/rest/src/rateLimit.config.ts b/packages/rest/src/rateLimit.config.ts index 6dd4afe..9b4c9e4 100644 --- a/packages/rest/src/rateLimit.config.ts +++ b/packages/rest/src/rateLimit.config.ts @@ -17,21 +17,11 @@ type LogEntry = { export type Log = LogEntry[]; /* - * Single event queue for number of requests in the last minute. Note that the Plugin API - * rate limit seems to use two sliding windows: one per minute and one per day. - * TODO: find a good storage place for these. + * Single event queue for number of requests in the last minute. * TODO: ensure these logs are disk-cached and well restored across client runs. */ const reqLog: Log = []; -/** - * Log of previous Error 429 events. When we get a 429, we apply a multiplier to our budget - * calculations to slow future requests down for a while, in an attempt to limit the risk - * of further 429 errors. - */ -// TODO: implement -// const error429Log: Log = []; - /** * Returns singleton objects and constants for the rate limit algorithm. * Exposed this way for mockability in tests. @@ -46,3 +36,15 @@ export function getConfig() { RATE_DECREASE_LENGTH, }; } + +/** + * Returns singleton objects and constants for the exponential delay algorithm on 429 errors. + * Exposed this way for mockability in tests. + * @returns 429 retry algorithm config data + */ +export function get429Config() { + return { + // We want a one second initial delay, but axios-retry uses 2^count, so 2^1 * 500 = 1000ms for the initial retry. + INITIAL_DELAY: 500, + }; +} diff --git a/packages/rest/src/rateLimit.ts b/packages/rest/src/rateLimit.ts index efe5b5d..21f0422 100644 --- a/packages/rest/src/rateLimit.ts +++ b/packages/rest/src/rateLimit.ts @@ -92,8 +92,6 @@ export async function interceptRequest(requestCost: number): Promise { // budget we'll need to run the call. reqLog.push({ timestamp: runNextRequest, budget: requestCost }); - // TODO read the 429 log for additional delays. - // Now wait if necessary. if (timeDelta > 0) { log(`Waiting ${timeDelta} seconds to respect API rate limits.\n`); @@ -106,10 +104,3 @@ export async function interceptRequest(requestCost: number): Promise { log(`Rate limit: sending request immediately\n`); } } - -// export async function interceptResponse() { -// In case of unexpected 429, record it so we can delay the next requests. -// TODO -// Return the response to the caller. -// TODO -// } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9787c67..4855f91 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -117,6 +117,9 @@ importers: tsup: specifier: ^8.2.4 version: 8.2.4(postcss@8.4.45)(typescript@5.6.2)(yaml@2.5.1) + typescript: + specifier: ^5.6.2 + version: 5.6.2 packages/config-typedoc: dependencies: @@ -137,6 +140,13 @@ importers: vitest: specifier: ^2.0.5 version: 2.1.1(@types/node@22.5.5)(jsdom@20.0.3) + devDependencies: + typescript: + specifier: ^5.6.2 + version: 5.6.2 + vite: + specifier: ^5.4.5 + version: 5.4.5(@types/node@22.5.5) packages/cuttings: dependencies: @@ -301,6 +311,9 @@ importers: axios-cache-interceptor: specifier: ^1.6.0 version: 1.6.0(axios@1.7.7) + axios-retry: + specifier: ^4.5.0 + version: 4.5.0(axios@1.7.7) devDependencies: '@figma/rest-api-spec': specifier: ^0.16.0 @@ -338,6 +351,9 @@ importers: mocked-env: specifier: ^1.3.5 version: 1.3.5 + nock: + specifier: ^13.5.5 + version: 13.5.5 swagger-typescript-api: specifier: ^13.0.22 version: 13.0.22 @@ -1169,6 +1185,11 @@ packages: peerDependencies: axios: ^1 + axios-retry@4.5.0: + resolution: {integrity: sha512-aR99oXhpEDGo0UuAlYcn2iGRds30k366Zfa05XWScR9QaQD4JYiP3/1Qt1u7YlefUOK+cn0CcwoL1oefavQUlQ==} + peerDependencies: + axios: 0.x || 1.x + axios@1.7.7: resolution: {integrity: sha512-S4kL7XrjgBmvdGut0sN3yJxqYzrDOnivkBiN0OFs6hLiUam3UPvswUo0kqGyhqUZGEOytHyumEdXsAkgCOUf3Q==} @@ -1800,6 +1821,10 @@ packages: is-potential-custom-element-name@1.0.1: resolution: {integrity: sha512-bCYeRA2rVibKZd+s2625gGnGF/t7DSqDs4dP7CrLA1m7jKWz6pps0LpYLJN8Q64HtmPKJ1hrN3nzPNKFEKOUiQ==} + is-retry-allowed@2.2.0: + resolution: {integrity: sha512-XVm7LOeLpTW4jV19QSH38vkswxoLud8sQ57YwJVTPWdiaI9I8keEhGFpBlslyVsgdQy4Opg8QOLb8YRgsyZiQg==} + engines: {node: '>=10'} + is-stream@2.0.1: resolution: {integrity: sha512-hFoiJiTl63nn+kstHGBtewWSKnQLpyb155KHheA1l39uvtO9nWIop1p3udqPcUd/xbF1VLMO4n7OI6p7RbngDg==} engines: {node: '>=8'} @@ -1871,6 +1896,9 @@ packages: json-stable-stringify-without-jsonify@1.0.1: resolution: {integrity: sha512-Bdboy+l7tA3OGW6FjyFHWkP5LuByj1Tk33Ljyq0axyzdk9//JSi2u3fP1QSmd1KNwq6VOKYGlAu87CisVir6Pw==} + json-stringify-safe@5.0.1: + resolution: {integrity: sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==} + json5@2.2.3: resolution: {integrity: sha512-XmOWe7eyHYH14cLdVPoyg+GOH3rYX++KpzrylJwSW98t3Nk+U8XOl8FWKOgwtzdb8lXGf6zYwDUzeHMWfxasyg==} engines: {node: '>=6'} @@ -2036,6 +2064,10 @@ packages: natural-compare@1.4.0: resolution: {integrity: sha512-OWND8ei3VtNC9h7V60qff3SVobHr996CTwgxubgyQYEpg290h9J0buyECNNJexkFm5sOajh5G116RYA1c8ZMSw==} + nock@13.5.5: + resolution: {integrity: sha512-XKYnqUrCwXC8DGG1xX4YH5yNIrlh9c065uaMZZHUoeUUINTOyt+x/G+ezYk0Ft6ExSREVIs+qBJDK503viTfFA==} + engines: {node: '>= 10.13'} + node-cleanup@2.1.2: resolution: {integrity: sha512-qN8v/s2PAJwGUtr1/hYTpNKlD6Y9rc4p8KSmJXyGdYGZsDGKXrGThikLFP9OCHFeLeEpQzPwiAtdIvBLqm//Hw==} @@ -2204,6 +2236,10 @@ packages: engines: {node: '>=14'} hasBin: true + propagate@2.0.1: + resolution: {integrity: sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==} + engines: {node: '>= 8'} + property-information@6.5.0: resolution: {integrity: sha512-PgTgs/BlvHxOu8QuEN7wi5A0OmXaBcHpmCSTehcs6Uuu9IkDIEo13Hy7n898RHfrQ49vKCoGeWZSaAK01nwVig==} @@ -3596,6 +3632,11 @@ snapshots: fast-defer: 1.1.8 object-code: 1.3.3 + axios-retry@4.5.0(axios@1.7.7): + dependencies: + axios: 1.7.7 + is-retry-allowed: 2.2.0 + axios@1.7.7: dependencies: follow-redirects: 1.15.6 @@ -4270,6 +4311,8 @@ snapshots: is-potential-custom-element-name@1.0.1: optional: true + is-retry-allowed@2.2.0: {} + is-stream@2.0.1: {} isexe@2.0.0: {} @@ -4369,6 +4412,8 @@ snapshots: json-stable-stringify-without-jsonify@1.0.1: {} + json-stringify-safe@5.0.1: {} + json5@2.2.3: {} jsonfile@4.0.0: @@ -4545,6 +4590,14 @@ snapshots: natural-compare@1.4.0: {} + nock@13.5.5: + dependencies: + debug: 4.3.6 + json-stringify-safe: 5.0.1 + propagate: 2.0.1 + transitivePeerDependencies: + - supports-color + node-cleanup@2.1.2: {} node-fetch-h2@2.3.0: @@ -4701,6 +4754,8 @@ snapshots: prettier@3.3.3: {} + propagate@2.0.1: {} + property-information@6.5.0: {} proxy-from-env@1.1.0: {}