From 644faf4f37686bfd364ba17853694253f3c0bac1 Mon Sep 17 00:00:00 2001 From: Michael Kret <88898367+michael-radency@users.noreply.github.com> Date: Sat, 30 Aug 2025 04:06:42 +0300 Subject: [PATCH] fix: Add timeout to community node types request (#18545) Co-authored-by: Your Name --- .../community-node-types.service.test.ts | 136 ++++++++++++++++++ .../__tests__/strapi-utils.test.ts | 69 +++++++++ .../community-node-types.service.ts | 16 ++- .../community-packages/strapi-utils.ts | 3 + 4 files changed, 222 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/modules/community-packages/__tests__/community-node-types.service.test.ts b/packages/cli/src/modules/community-packages/__tests__/community-node-types.service.test.ts index 8d63c09d17..0b2672403d 100644 --- a/packages/cli/src/modules/community-packages/__tests__/community-node-types.service.test.ts +++ b/packages/cli/src/modules/community-packages/__tests__/community-node-types.service.test.ts @@ -12,6 +12,9 @@ jest.mock('../community-node-types-utils', () => ({ getCommunityNodeTypes: jest.fn().mockResolvedValue([]), })); +const mockDateNow = jest.spyOn(Date, 'now'); +const mockMathRandom = jest.spyOn(Math, 'random'); + describe('CommunityNodeTypesService', () => { let service: CommunityNodeTypesService; let configMock: any; @@ -30,9 +33,16 @@ describe('CommunityNodeTypesService', () => { }; communityPackagesServiceMock = {}; + if (mockDateNow.mockRestore) mockDateNow.mockRestore(); + if (mockMathRandom.mockRestore) mockMathRandom.mockRestore(); + service = new CommunityNodeTypesService(loggerMock, configMock, communityPackagesServiceMock); }); + afterEach(() => { + jest.restoreAllMocks(); + }); + describe('fetchNodeTypes', () => { it('should use staging environment when ENVIRONMENT=staging', async () => { process.env.ENVIRONMENT = 'staging'; @@ -59,4 +69,130 @@ describe('CommunityNodeTypesService', () => { expect(getCommunityNodeTypes).toHaveBeenCalledWith('staging'); }); }); + + describe('updateCommunityNodeTypes', () => { + beforeEach(() => { + jest.spyOn(Date, 'now').mockImplementation(() => 1000000); + + jest.spyOn(Math, 'random').mockImplementation(() => 0.5); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should call setTimestampForRetry when nodeTypes is empty array', () => { + const setTimestampForRetrySpy = jest.spyOn(service as any, 'setTimestampForRetry'); + + (service as any).updateCommunityNodeTypes([]); + + expect(setTimestampForRetrySpy).toHaveBeenCalledTimes(1); + }); + + it('should call setTimestampForRetry when nodeTypes is null', () => { + const setTimestampForRetrySpy = jest.spyOn(service as any, 'setTimestampForRetry'); + + (service as any).updateCommunityNodeTypes(null); + + expect(setTimestampForRetrySpy).toHaveBeenCalledTimes(1); + }); + + it('should call setTimestampForRetry when nodeTypes is undefined', () => { + const setTimestampForRetrySpy = jest.spyOn(service as any, 'setTimestampForRetry'); + + (service as any).updateCommunityNodeTypes(undefined); + + expect(setTimestampForRetrySpy).toHaveBeenCalledTimes(1); + }); + + it('should return early when nodeTypes is empty without updating communityNodeTypes', () => { + const resetCommunityNodeTypesSpy = jest.spyOn(service as any, 'resetCommunityNodeTypes'); + const initialNodeTypes = (service as any).communityNodeTypes; + + (service as any).updateCommunityNodeTypes([]); + + expect(resetCommunityNodeTypesSpy).not.toHaveBeenCalled(); + expect((service as any).communityNodeTypes).toBe(initialNodeTypes); + + expect((service as any).lastUpdateTimestamp).not.toBe(1000000); + }); + + it('should process nodeTypes normally when array has content', () => { + const mockNodeTypes = [ + { name: 'test-node-1', version: '1.0.0' }, + { name: 'test-node-2', version: '1.1.0' }, + ]; + const resetCommunityNodeTypesSpy = jest.spyOn(service as any, 'resetCommunityNodeTypes'); + + (service as any).updateCommunityNodeTypes(mockNodeTypes); + + expect(resetCommunityNodeTypesSpy).toHaveBeenCalledTimes(1); + expect((service as any).communityNodeTypes.size).toBe(2); + expect((service as any).communityNodeTypes.get('test-node-1')).toEqual(mockNodeTypes[0]); + expect((service as any).communityNodeTypes.get('test-node-2')).toEqual(mockNodeTypes[1]); + expect((service as any).lastUpdateTimestamp).toBe(1000000); + }); + }); + + describe('setTimestampForRetry', () => { + const UPDATE_INTERVAL = 8 * 60 * 60 * 1000; + const RETRY_INTERVAL = 5 * 60 * 1000; + + beforeEach(() => { + jest.spyOn(Date, 'now').mockImplementation(() => 1000000); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should set timestamp with jitter for retry', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0.5); + + (service as any).setTimestampForRetry(); + + const expectedTimestamp = 1000000 - (UPDATE_INTERVAL - RETRY_INTERVAL + 0); + expect((service as any).lastUpdateTimestamp).toBe(expectedTimestamp); + }); + + it('should set timestamp with negative jitter', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 0); + + (service as any).setTimestampForRetry(); + + const expectedJitter = -120000; + const expectedTimestamp = 1000000 - (UPDATE_INTERVAL - RETRY_INTERVAL + expectedJitter); + expect((service as any).lastUpdateTimestamp).toBe(expectedTimestamp); + }); + + it('should set timestamp with positive jitter', () => { + jest.spyOn(Math, 'random').mockImplementation(() => 1); + + (service as any).setTimestampForRetry(); + + const expectedJitter = 120000; + const expectedTimestamp = 1000000 - (UPDATE_INTERVAL - RETRY_INTERVAL + expectedJitter); + expect((service as any).lastUpdateTimestamp).toBe(expectedTimestamp); + }); + + it('should calculate jitter within expected range', () => { + const testCases = [0, 0.25, 0.5, 0.75, 1]; + + testCases.forEach((randomValue, index) => { + const testTimestamp = 2000000 + index * 1000; + jest.spyOn(Math, 'random').mockImplementation(() => randomValue); + jest.spyOn(Date, 'now').mockImplementation(() => testTimestamp); + + (service as any).setTimestampForRetry(); + + const expectedJitter = Math.floor(randomValue * 4 * 60 * 1000) - 2 * 60 * 1000; + const expectedTimestamp = + testTimestamp - (UPDATE_INTERVAL - RETRY_INTERVAL + expectedJitter); + + expect((service as any).lastUpdateTimestamp).toBe(expectedTimestamp); + expect(expectedJitter).toBeGreaterThanOrEqual(-120000); + expect(expectedJitter).toBeLessThanOrEqual(120000); + }); + }); + }); }); diff --git a/packages/cli/src/modules/community-packages/__tests__/strapi-utils.test.ts b/packages/cli/src/modules/community-packages/__tests__/strapi-utils.test.ts index 1f76575d6b..e1470d6421 100644 --- a/packages/cli/src/modules/community-packages/__tests__/strapi-utils.test.ts +++ b/packages/cli/src/modules/community-packages/__tests__/strapi-utils.test.ts @@ -1,4 +1,5 @@ import nock from 'nock'; +import axios from 'axios'; import { paginatedRequest } from '../strapi-utils'; @@ -124,5 +125,73 @@ describe('Strapi utils', () => { expect(result).toEqual([]); }); + + it('should apply the correct timeout value to axios requests', async () => { + const axiosGetSpy = jest.spyOn(axios, 'get'); + + nock('https://strapi.test') + .get('/api/nodes') + .query(true) + .reply(200, { + data: [], + meta: { + pagination: { + page: 1, + pageSize: 25, + pageCount: 0, + total: 0, + }, + }, + }); + + await paginatedRequest(baseUrl); + + expect(axiosGetSpy).toHaveBeenCalledWith( + baseUrl, + expect.objectContaining({ + timeout: 3000, + }), + ); + + axiosGetSpy.mockRestore(); + }); + + it('should handle timeout errors and return empty array', async () => { + const timeoutError = new Error('timeout of 3000ms exceeded'); + timeoutError.name = 'AxiosError'; + (timeoutError as any).code = 'ECONNABORTED'; + + nock('https://strapi.test') + .get('/api/nodes') + .query(true) + .delayConnection(4000) // Delay longer than timeout + .reply(200, { data: [] }); + + const result = await paginatedRequest(baseUrl); + + expect(result).toEqual([]); + }); + + it('should handle network timeout and continue gracefully', async () => { + // Mock axios to simulate timeout + const axiosGetSpy = jest.spyOn(axios, 'get').mockRejectedValueOnce( + Object.assign(new Error('timeout of 3000ms exceeded'), { + code: 'ECONNABORTED', + name: 'AxiosError', + }), + ); + + const result = await paginatedRequest(baseUrl); + + expect(result).toEqual([]); + expect(axiosGetSpy).toHaveBeenCalledWith( + baseUrl, + expect.objectContaining({ + timeout: 3000, + }), + ); + + axiosGetSpy.mockRestore(); + }); }); }); diff --git a/packages/cli/src/modules/community-packages/community-node-types.service.ts b/packages/cli/src/modules/community-packages/community-node-types.service.ts index b17a742ba4..e572d93d49 100644 --- a/packages/cli/src/modules/community-packages/community-node-types.service.ts +++ b/packages/cli/src/modules/community-packages/community-node-types.service.ts @@ -8,6 +8,7 @@ import { CommunityPackagesConfig } from './community-packages.config'; import { CommunityPackagesService } from './community-packages.service'; const UPDATE_INTERVAL = 8 * 60 * 60 * 1000; +const RETRY_INTERVAL = 5 * 60 * 1000; @Service() export class CommunityNodeTypesService { @@ -45,7 +46,13 @@ export class CommunityNodeTypesService { } private updateCommunityNodeTypes(nodeTypes: StrapiCommunityNodeType[]) { - if (!nodeTypes?.length) return; + if (!nodeTypes?.length) { + // When we get empty data, don't wait the full UPDATE_INTERVAL to try again. + // Instead, set the timestamp to retry after RETRY_INTERVAL with some + // random jitter to avoid all instances retrying at once + this.setTimestampForRetry(); + return; + } this.resetCommunityNodeTypes(); @@ -63,6 +70,11 @@ export class CommunityNodeTypesService { return Date.now() - this.lastUpdateTimestamp > UPDATE_INTERVAL; } + private setTimestampForRetry() { + const jitter = Math.floor(Math.random() * 4 * 60 * 1000) - 2 * 60 * 1000; + this.lastUpdateTimestamp = Date.now() - (UPDATE_INTERVAL - RETRY_INTERVAL + jitter); + } + private async createIsInstalled() { const installedPackages = (await this.communityPackagesService.getAllInstalledPackages()) ?? []; const installedPackageNames = new Set(installedPackages.map((p) => p.packageName)); @@ -71,7 +83,7 @@ export class CommunityNodeTypesService { } async getCommunityNodeTypes(): Promise { - if (this.updateRequired() || !this.communityNodeTypes.size) { + if (this.updateRequired()) { await this.fetchNodeTypes(); } diff --git a/packages/cli/src/modules/community-packages/strapi-utils.ts b/packages/cli/src/modules/community-packages/strapi-utils.ts index 72eb39fdc2..99fbb1f982 100644 --- a/packages/cli/src/modules/community-packages/strapi-utils.ts +++ b/packages/cli/src/modules/community-packages/strapi-utils.ts @@ -24,6 +24,8 @@ interface Pagination { total: number; } +const REQUEST_TIMEOUT_MS = 3000; + export async function paginatedRequest(url: string): Promise { let returnData: T[] = []; let responseData: T[] | undefined = []; @@ -41,6 +43,7 @@ export async function paginatedRequest(url: string): Promise { response = await axios.get>(url, { headers: { 'Content-Type': 'application/json' }, params, + timeout: REQUEST_TIMEOUT_MS, }); } catch (error) { Container.get(ErrorReporter).error(error, {