diff --git a/packages/twenty-server/package.json b/packages/twenty-server/package.json index f40cd04a51..ac1874ef3d 100644 --- a/packages/twenty-server/package.json +++ b/packages/twenty-server/package.json @@ -85,6 +85,7 @@ "apollo-server-core": "3.13.0", "archiver": "7.0.1", "axios": "^1.13.5", + "axios-retry": "^4.5.0", "babel-plugin-module-resolver": "5.0.2", "bcrypt": "5.1.1", "bullmq": "5.40.0", diff --git a/packages/twenty-server/src/engine/core-modules/file/file-core-picture/services/file-core-picture.service.ts b/packages/twenty-server/src/engine/core-modules/file/file-core-picture/services/file-core-picture.service.ts index 2c91b045c1..e4b3ec47a9 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file-core-picture/services/file-core-picture.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file-core-picture/services/file-core-picture.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { buffer as streamToBuffer } from 'node:stream/consumers'; @@ -24,6 +24,8 @@ import { getImageBufferFromUrl } from 'src/utils/image'; @Injectable() export class FileCorePictureService { + private readonly logger = new Logger(FileCorePictureService.name); + constructor( private readonly fileStorageService: FileStorageService, private readonly applicationService: ApplicationService, @@ -186,7 +188,11 @@ export class FileCorePictureService { imageUrl: string, ): Promise<{ buffer: Buffer; extension: string } | undefined> { try { - const httpClient = this.secureHttpClientService.getHttpClient(); + const httpClient = this.secureHttpClientService.getHttpClient({ + retries: 2, + shouldResetTimeout: true, + }); + const buffer = await getImageBufferFromUrl(imageUrl, httpClient); const type = await FileType.fromBuffer(buffer); @@ -196,7 +202,11 @@ export class FileCorePictureService { } return { buffer, extension: type.ext }; - } catch { + } catch (error) { + this.logger.warn( + `Failed to fetch image from URL: ${imageUrl} — ${error instanceof Error ? error.message : String(error)}`, + ); + return undefined; } } diff --git a/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts b/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts index 9dd65f1fbb..5ac85c315a 100644 --- a/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts +++ b/packages/twenty-server/src/engine/core-modules/file/file-upload/services/file-upload.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@nestjs/common'; +import { Injectable, Logger } from '@nestjs/common'; import FileType from 'file-type'; import sharp from 'sharp'; @@ -23,6 +23,8 @@ export type SignedFilesResult = { @Injectable() export class FileUploadService { + private readonly logger = new Logger(FileUploadService.name); + constructor( private readonly fileStorage: FileStorageService, private readonly fileService: FileService, @@ -95,33 +97,54 @@ export class FileUploadService { fileFolder: FileFolder; workspaceId: string; }) { - const httpClient = this.secureHttpClientService.getHttpClient(); + const imageData = await this.fetchImageBufferFromUrl(imageUrl).catch( + (error) => { + this.logger.warn( + `Failed to fetch image from URL: ${imageUrl} — ${error instanceof Error ? error.message : String(error)}`, + ); - const buffer = await getImageBufferFromUrl(imageUrl, httpClient); + return null; + }, + ); - const type = await FileType.fromBuffer(buffer); - - if (!type || !type.ext || !type.mime) { - throw new Error( - 'Unable to detect image type from buffer. The file may not be a valid image format.', - ); - } - - if (!type.mime.startsWith('image/')) { - throw new Error( - `Detected file type is not an image: ${type.mime}. Please provide a valid image URL.`, - ); + if (!imageData) { + return { name: '', mimeType: undefined, files: [] }; } return await this.uploadImage({ - file: buffer, - filename: `${v4()}.${type.ext}`, - mimeType: type.mime, + file: imageData.buffer, + filename: `${v4()}.${imageData.extension}`, + mimeType: imageData.mimeType, fileFolder, workspaceId, }); } + private async fetchImageBufferFromUrl(imageUrl: string): Promise<{ + buffer: Buffer; + extension: string; + mimeType: string; + } | null> { + const httpClient = this.secureHttpClientService.getHttpClient({ + retries: 2, + shouldResetTimeout: true, + }); + + const buffer = await getImageBufferFromUrl(imageUrl, httpClient); + + if (!buffer || buffer.length === 0) { + return null; + } + + const type = await FileType.fromBuffer(buffer); + + if (!type || !type.ext || !type.mime || !type.mime.startsWith('image/')) { + throw new Error(`Invalid image type for URL: ${imageUrl}`); + } + + return { buffer, extension: type.ext, mimeType: type.mime }; + } + async uploadImage({ file, filename, diff --git a/packages/twenty-server/src/engine/core-modules/secure-http-client/__tests__/secure-http-client.service.spec.ts b/packages/twenty-server/src/engine/core-modules/secure-http-client/__tests__/secure-http-client.service.spec.ts index e1373daaf9..5bbe6b039f 100644 --- a/packages/twenty-server/src/engine/core-modules/secure-http-client/__tests__/secure-http-client.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/secure-http-client/__tests__/secure-http-client.service.spec.ts @@ -1,9 +1,16 @@ import * as http from 'http'; import * as https from 'https'; +import axiosRetry from 'axios-retry'; + import { SecureHttpClientService } from 'src/engine/core-modules/secure-http-client/secure-http-client.service'; import { type TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; +jest.mock('axios-retry', () => ({ + __esModule: true, + default: jest.fn(), +})); + const createMockConfigService = ( overrides: Record = {}, ): TwentyConfigService => { @@ -81,6 +88,43 @@ describe('SecureHttpClientService', () => { expect(client.defaults.baseURL).toBe('https://example.com/api'); }); + + it('should configure axios-retry when retries is greater than 0', () => { + jest.mocked(axiosRetry).mockClear(); + const service = new SecureHttpClientService(createMockConfigService()); + const client = service.getHttpClient({ + retries: 2, + shouldResetTimeout: true, + }); + + expect(axiosRetry).toHaveBeenCalledWith(client, { + retries: 2, + shouldResetTimeout: true, + retryCondition: expect.any(Function), + }); + }); + + it('should not configure axios-retry when retries is 0', () => { + jest.mocked(axiosRetry).mockClear(); + const service = new SecureHttpClientService(createMockConfigService()); + + service.getHttpClient({ retries: 0 }); + + expect(axiosRetry).not.toHaveBeenCalled(); + }); + + it('should not leak retry config into axios defaults', () => { + const service = new SecureHttpClientService(createMockConfigService()); + const client = service.getHttpClient({ + retries: 2, + shouldResetTimeout: true, + baseURL: 'https://example.com', + }); + + expect(client.defaults.baseURL).toBe('https://example.com'); + expect(client.defaults).not.toHaveProperty('retries'); + expect(client.defaults).not.toHaveProperty('shouldResetTimeout'); + }); }); describe('getInternalHttpClient', () => { diff --git a/packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts b/packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts index 38728dd30f..638a7f8e7c 100644 --- a/packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts +++ b/packages/twenty-server/src/engine/core-modules/secure-http-client/secure-http-client.service.ts @@ -1,6 +1,8 @@ import { Injectable, Logger } from '@nestjs/common'; import axios, { type AxiosInstance, type CreateAxiosDefaults } from 'axios'; +import axiosRetry from 'axios-retry'; +import { isDefined } from 'twenty-shared/utils'; import { createSsrfSafeAgent } from 'src/engine/core-modules/secure-http-client/utils/create-ssrf-safe-agent.util'; import { resolveAndValidateHostname } from 'src/engine/core-modules/secure-http-client/utils/resolve-and-validate-hostname.util'; @@ -10,6 +12,11 @@ import { type OutboundRequestContext } from './outbound-request-context.type'; const MAX_REDIRECTS = 5; +type SecureHttpClientConfig = CreateAxiosDefaults & { + retries?: number; + shouldResetTimeout?: boolean; +}; + @Injectable() export class SecureHttpClientService { private readonly logger = new Logger(SecureHttpClientService.name); @@ -22,24 +29,37 @@ export class SecureHttpClientService { // When context is provided, outbound requests are logged with // workspace/user info for GuardDuty correlation. getHttpClient( - config?: CreateAxiosDefaults, + config?: SecureHttpClientConfig, context?: OutboundRequestContext, ): AxiosInstance { + const { retries, shouldResetTimeout, ...axiosConfig } = config ?? {}; + const isSafeModeEnabled = this.twentyConfigService.get( 'OUTBOUND_HTTP_SAFE_MODE_ENABLED', ); const client = isSafeModeEnabled ? axios.create({ - ...config, + ...axiosConfig, httpAgent: createSsrfSafeAgent('http'), httpsAgent: createSsrfSafeAgent('https'), maxRedirects: Math.min( - config?.maxRedirects ?? MAX_REDIRECTS, + axiosConfig.maxRedirects ?? MAX_REDIRECTS, MAX_REDIRECTS, ), }) - : axios.create(config); + : axios.create(axiosConfig); + + if (isDefined(retries) && retries > 0) { + axiosRetry(client, { + retries, + shouldResetTimeout, + retryCondition: (error) => + axiosRetry.isNetworkOrIdempotentRequestError(error) && + error.code !== 'ECONNABORTED' && + error.code !== 'ETIMEDOUT', + }); + } if (context) { client.interceptors.request.use((requestConfig) => { diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts index 74f2cc4296..cb1a0dfd05 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.spec.ts @@ -26,8 +26,8 @@ import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac import { DataSourceService } from 'src/engine/metadata-modules/data-source/data-source.service'; import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity'; import { PermissionsException } from 'src/engine/metadata-modules/permissions/permissions.exception'; -import { RoleValidationService } from 'src/engine/metadata-modules/role-validation/services/role-validation.service'; import { RoleTargetEntity } from 'src/engine/metadata-modules/role-target/role-target.entity'; +import { RoleValidationService } from 'src/engine/metadata-modules/role-validation/services/role-validation.service'; import { UserRoleService } from 'src/engine/metadata-modules/user-role/user-role.service'; import { GlobalWorkspaceOrmManager } from 'src/engine/twenty-orm/global-workspace-datasource/global-workspace-orm.manager'; import { type WorkspaceRepository } from 'src/engine/twenty-orm/repository/workspace.repository'; @@ -304,6 +304,50 @@ describe('UserWorkspaceService', () => { expect(userWorkspaceRepository.save).toHaveBeenCalledWith(userWorkspace); expect(result).toEqual(userWorkspace); }); + it('should create a user workspace without a default avatar url if image fetch fails', async () => { + const userId = 'user-id'; + const workspaceId = 'workspace-id'; + const userWorkspace = { + userId, + workspaceId, + } as UserWorkspaceEntity; + + jest + .spyOn(userWorkspaceRepository, 'create') + .mockReturnValue(userWorkspace); + jest + .spyOn(userWorkspaceRepository, 'save') + .mockResolvedValue(userWorkspace); + + jest + .spyOn(fileUploadService, 'uploadImageFromUrl') + .mockRejectedValue( + new Error( + 'Failed to fetch image from https://lh3.googleusercontent.com/a/invalid: Request failed with status code 404', + ), + ); + + const result = await service.create({ + userId, + workspaceId, + isExistingUser: false, + pictureUrl: 'https://lh3.googleusercontent.com/a/invalid', + }); + + expect(fileUploadService.uploadImageFromUrl).toHaveBeenCalledTimes(1); + expect(fileUploadService.uploadImageFromUrl).toHaveBeenCalledWith({ + imageUrl: 'https://lh3.googleusercontent.com/a/invalid', + fileFolder: FileFolder.ProfilePicture, + workspaceId, + }); + expect(userWorkspaceRepository.create).toHaveBeenCalledWith({ + userId, + workspaceId, + defaultAvatarUrl: undefined, + }); + expect(userWorkspaceRepository.save).toHaveBeenCalledWith(userWorkspace); + expect(result).toEqual(userWorkspace); + }); it("should create a user workspace without a default avatar url if it's a new user without a picture url", async () => { const userId = 'user-id'; const workspaceId = 'workspace-id'; diff --git a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts index e42ba54fa2..1ca67af181 100644 --- a/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts +++ b/packages/twenty-server/src/engine/core-modules/user-workspace/user-workspace.service.ts @@ -1,3 +1,4 @@ +import { Logger } from '@nestjs/common'; import { InjectRepository } from '@nestjs/typeorm'; import { TypeOrmQueryService } from '@ptc-org/nestjs-query-typeorm'; @@ -44,6 +45,8 @@ import { assert } from 'src/utils/assert'; import { getDomainNameByEmail } from 'src/utils/get-domain-name-by-email'; export class UserWorkspaceService extends TypeOrmQueryService { + private readonly logger = new Logger(UserWorkspaceService.name); + constructor( @InjectRepository(UserWorkspaceEntity) private readonly userWorkspaceRepository: Repository, @@ -482,17 +485,25 @@ export class UserWorkspaceService extends TypeOrmQueryService status >= 200 && status < 300, maxRedirects: 5, - timeout: 30000, + timeout: 10000, }); if (!response.data) { @@ -60,34 +60,8 @@ export const getImageBufferFromUrl = async ( return Buffer.from(response.data, 'binary'); } catch (error) { - const axiosError = error as AxiosError; - const axiosResponse = axiosError.response; + const message = error instanceof Error ? error.message : 'Unknown error'; - if (axiosResponse) { - throw new Error( - `Failed to fetch image: HTTP ${axiosResponse.status} from ${url}`, - ); - } - - if (error instanceof Error) { - if ( - axiosError.code === 'ECONNABORTED' || - error.message.includes('timeout') - ) { - throw new Error( - `Request timeout while fetching image from URL: ${url}`, - ); - } - if ( - axiosError.code === 'ENOTFOUND' || - axiosError.code === 'ECONNREFUSED' || - error.message.includes('ENOTFOUND') || - error.message.includes('ECONNREFUSED') - ) { - throw new Error(`Failed to connect to image URL: ${url}`); - } - throw new Error(`Failed to fetch image from URL: ${error.message}`); - } - throw new Error(`Failed to fetch image from URL: ${url}`); + throw new Error(`Failed to fetch image from ${url}: ${message}`); } }; diff --git a/yarn.lock b/yarn.lock index a5280b4ac0..2cbb84ee4d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -29122,6 +29122,17 @@ __metadata: languageName: node linkType: hard +"axios-retry@npm:^4.5.0": + version: 4.5.0 + resolution: "axios-retry@npm:4.5.0" + dependencies: + is-retry-allowed: "npm:^2.2.0" + peerDependencies: + axios: 0.x || 1.x + checksum: 10c0/574e7b1bf24aad99b560042d232a932d51bfaa29b5a6d4612d748ed799a6f11a5afb2582792492c55d95842200cbdfbe3454027a8c1b9a2d3e895d13c3d03c10 + languageName: node + linkType: hard + "axios@npm:^1.12.0, axios@npm:^1.13.5, axios@npm:^1.6.1, axios@npm:^1.7.7, axios@npm:^1.8.3": version: 1.13.5 resolution: "axios@npm:1.13.5" @@ -41270,6 +41281,13 @@ __metadata: languageName: node linkType: hard +"is-retry-allowed@npm:^2.2.0": + version: 2.2.0 + resolution: "is-retry-allowed@npm:2.2.0" + checksum: 10c0/013be4f8a0a06a49ed1fe495242952e898325d496202a018f6f9fb3fb9ac8fe3b957a9bd62463d68299ae35dbbda680473c85a9bcefca731b49d500d3ccc08ff + languageName: node + linkType: hard + "is-scoped@npm:^2.1.0": version: 2.1.0 resolution: "is-scoped@npm:2.1.0" @@ -57907,6 +57925,7 @@ __metadata: apollo-server-core: "npm:3.13.0" archiver: "npm:7.0.1" axios: "npm:^1.13.5" + axios-retry: "npm:^4.5.0" babel-plugin-module-resolver: "npm:5.0.2" bcrypt: "npm:5.1.1" bullmq: "npm:5.40.0"